chromiumembedded / cef

Chromium Embedded Framework (CEF). A simple framework for embedding Chromium-based browsers in other applications.
https://bitbucket.org/chromiumembedded/cef/
Other
3.23k stars 456 forks source link

chrome: Add support for GetResourceRequestHandler callback with active extension #3444

Closed magreenblatt closed 1 year ago

magreenblatt commented 1 year ago

Original report by Bob Miller (Bitbucket: DTBob, GitHub: DTBob).


(See support topic https://magpcss.org/ceforum/viewtopic.php?f=6&t=19352&p=53182#p53182)

When a CEF app is run with --enable-chrome-runtime, and a Chrome extension is enabled (via chrome://extensions in the CEF app), when a page is loaded a call is never made CefClientHandler::GetResourceRequestHandler. This prevents using CefResourceRequestHandler callbacks.

WebRequestProxyingURLLoaderFactory::CreateLoaderAndStart is called instead of ProxyURLLoaderFactory::CreateLoaderAndStart, and so a call is never made CefClientHandler::GetResourceRequestHandler.

  1. What steps will reproduce the problem?
    • Run cefclient in debugger, with --enable-chrome-runtime.
    • Place a breakpoint in client_handler.cc inside the ClientHandler::GetResourceRequestHandler method (around line 940).
    • If no extensions are enabled, the breakpoint should be hit as content is loaded.
    • Disable the breakpoint temporarily
    • Navigate to chrome://extensions. If using views (default) a separate cefclient will run with the Chrome Extensions available listed. Enable an extension and close the window.
    • Re-enable the breakpoint.
    • Refresh the page or load a new one. The breakpoint set in ClientHandler::GetResourceRequestHandler is no longer hit.
  2. What is the expected output? What do you see instead?
    When extensions are enabled, ClientHandler::GetResourceRequestHandler should still be available.
  3. What version of the product are you using? On what operating system?
    CEF 107.1.12+g65b79a6+chromium-107.0.5304.122
    Chromium 107.0.5304.122
  4. Does the problem reproduce with the cefclient or cefsimple sample application at the same version? How about with a newer or older version?
    Yes in cefclient. Not tried in cefsimple. Not tried earlier versions.
  5. Does the problem reproduce with Google Chrome at the same version? How about with a newer or older version?
    N/A

magreenblatt commented 1 year ago

We’ll need to figure out how to chain proxies in ChromeContentBrowserClientCef::WillCreateURLLoaderFactory when |use_proxy| is true.

magreenblatt commented 1 year ago

To test in cefclient:

  1. Install the AdBlock extension.
  2. Create a non-Incognito window.
  3. Load an internally handled URL like http://tests/other_tests

Currently, the URL load will fail. Instead, it should succeed.

magreenblatt commented 1 year ago

Looks like devtools_instrumentation::WillCreateURLLoaderFactory tries to do something similar to what we need here.

magreenblatt commented 1 year ago

Original comment by Dmitry Azaraev (Bitbucket: Mystic Artifact).


@{557058:2f2a2aee-b500-4023-9734-037e9897c3ab} take into account what devtools solution is racy in sense of handler ordering. One case is just install extension and devtools handler probably first, and second - running existing profile with already installed extension, and handlers are called in reverse order (probably actual order is reverse of my words, but it varied nor specified). So, i guess CEF may do it better, and should not be issue, but mention just in case for such potential ambiguity. Also i can imagine cases when we want install interception before or after extension, but GetResourceRequestHandler probably should go always first.

magreenblatt commented 1 year ago

To verify that this issue is fixed, after installing the AdBlock extension:

  1. In a non-Incognito window, load an internally handled URL like http://tests/other_tests. That URL should load successfully.
  2. In a non-Incognito window, load this URL for testing AdBlock. It should report same as Google Chrome (currently 33% blocked at M111, vs 9% with AdBlock not installed).

magreenblatt commented 1 year ago

@{557058:009b194f-c853-4192-bde1-f762de326b1f} Thanks for the warning. I’ve verified that CEF’s InterceptedRequest::OnReceiveResponse and OnComplete are always called before Chrome’s WebRequestProxyingURLLoaderFactory::InProgressRequest::OnReceiveResponse and OnComplete. Beyond the few manual tests mentioned above, I have not verified that chrome.webRequest extension APIs work as expected in combination with CEF’s request interception.

In particular, there’s a remaining TODO for proxying TrustedURLLoaderHeaderClient callbacks (and it’s unclear whether this will be required).

magreenblatt commented 1 year ago

chrome: Make primary user profile the default global context (see issue #3444)

Chrome is always loading the primary user profile by default, so with this change the CEF behavior more accurately reflects reality. Incognito contexts can still be created explicitly via CefRequestContext::CreateContext.

Prior to this change, the default for the global context was an Incognito profile based on the primary user profile. That caused request interception to be bypassed in WillCreateURLLoaderFactory for profiles associated with the "New window" and "New incognito window" commands. Those profiles, while also being (or based on) the primary user profile, did not match the specific Incognito profile assigned to the default global context.

After this change, the "New window" and "New incognito window" commands will match the default global context when executed on a browser that was created using the primary user profile.

→ <<cset 73082fd2ced9 (bb)>>

magreenblatt commented 1 year ago

chrome: Fix request interception with active extension (fixes issue #3444)

Support chaining of proxies in WillCreateURLLoaderFactory.

→ <<cset 80cfb3c97cb9 (bb)>>

magreenblatt commented 1 year ago

chrome: cefclient: Add default handler for request tests (see issue #3444)

Support loading of request tests (e.g. http://tests/other_tests) inside default browsers created via "New window" and "New incognito window" commands.

→ <<cset 13f228275488 (bb)>>

magreenblatt commented 1 year ago

chrome: Make primary user profile the default global context (see issue #3444)

Chrome is always loading the primary user profile by default, so with this change the CEF behavior more accurately reflects reality. Incognito contexts can still be created explicitly via CefRequestContext::CreateContext.

Prior to this change, the default for the global context was an Incognito profile based on the primary user profile. That caused request interception to be bypassed in WillCreateURLLoaderFactory for profiles associated with the "New window" and "New incognito window" commands. Those profiles, while also being (or based on) the primary user profile, did not match the specific Incognito profile assigned to the default global context.

After this change, the "New window" and "New incognito window" commands will match the default global context when executed on a browser that was created using the primary user profile.

→ <<cset b7ba0b9a66f7 (bb)>>

magreenblatt commented 1 year ago

chrome: Fix request interception with active extension (fixes issue #3444)

Support chaining of proxies in WillCreateURLLoaderFactory.

→ <<cset d4c8104ca8e9 (bb)>>

magreenblatt commented 1 year ago

chrome: cefclient: Add default handler for request tests (see issue #3444)

Support loading of request tests (e.g. http://tests/other_tests) inside default browsers created via "New window" and "New incognito window" commands.

→ <<cset 565ad7bb99fa (bb)>>

magreenblatt commented 1 year ago

Original comment by Dmitry Azaraev (Bitbucket: Mystic Artifact).


@{557058:2f2a2aee-b500-4023-9734-037e9897c3ab}

Chrome is always loading the primary user profile by default, so with this change the CEF behavior more accurately reflects reality. Incognito contexts can still be created explicitly via CefRequestContext::CreateContext.

I’m did not inspect code, but wording is absolutely wrong: chrome is always load last active profiles, or it might open system profile (profile, which implements profile chooser), unless you did not specify which profile it should open. last active profiles is tracked by Local State. I’m even not sure what it is exist “primary” profile in it, except what it gravitate to profile named “Default”, but this is not strictly necessary.

PS: And did very trust to ProfileManager: GetLastUsedProfile actually returns everything, except what it should return. It always returns expected profile in normal browser scenario, but it is still just points to profile in prefs, not actually last used profile. :) (I’m guess it was named before actual behavior established)

magreenblatt commented 1 year ago

@{557058:009b194f-c853-4192-bde1-f762de326b1f} The primary profile selection is now more explicit (always “Default”) with recent CEF versions, and the profile picker is disabled (see issue #3440).

magreenblatt commented 1 year ago

Original comment by Dmitry Azaraev (Bitbucket: Mystic Artifact).


@{557058:2f2a2aee-b500-4023-9734-037e9897c3ab} this doesn’t meant what last used profile will not be used. Default chrome behavior is loading all profiles which had been opened before close. Pointed code describes what it is primary profile but it did not points when it used. I’m just mention, what on normal code path it is not used (except as fallback case).

magreenblatt commented 1 year ago

Original comment by Dmitry Azaraev (Bitbucket: Mystic Artifact).


@{557058:2f2a2aee-b500-4023-9734-037e9897c3ab} code below seems like explicitly set which profile to pick (as you say above), so I’m sure what CEF API works right. But calls to ProfileManager::GetLastUsedProfile will end in random pick (from API usage perspective).

magreenblatt commented 1 year ago

Original changes by Bob Miller (Bitbucket: DTBob, GitHub: DTBob).


magreenblatt commented 1 year ago
magreenblatt commented 1 year ago