-
Notifications
You must be signed in to change notification settings - Fork 470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement support for NetworkService #2622
Comments
Issue #2497 is also related. |
@shagkur Can you share any progress on this issue? I'm planning to pick this up shortly as well, given the approaching old path removal deadline. |
Generalizing this issue to cover all NetworkService-related changes. |
|
Key methods for NetworkService initialization:
Request handling/interception with NetworkService: Custom schemes are handled using URLLoaderFactory (for example, ExtensionURLLoaderFactory). These objects are created via:
Existing URLLoaderFactory interfaces can be intercepted using WillCreateURLLoaderFactory. Arbitrary requests can be intercepted using URLLoaderRequestInterceptor (for example, OfflinePageURLLoaderRequestInterceptor). These objects are created via WillCreateURLLoaderRequestInterceptors. |
CefURLRequest can be implemented using a custom URLLoader (likely starting with a fork of SimpleURLLoader). |
Current status:
Summary of changes (see commit log for a full list of changes):
Other related enhancements (not blocking NetworkService by default):
|
@shagkur if you would like to work on the CefURLRequest support feel free to use my PR as a starting point -- we can merge all of the commits at some point in the future. |
Original comment by Mike Wiedenbauer (Bitbucket: shagkur, GitHub: shagkur). @magreenblatt Sorry for not being much responsive these days, but i'm currently at an on-site visit for my company and can't do much on this topic. Next week looks better and i'll start on it on your branch (already had a quick look at it today) |
@shagkur sounds good, thanks! :) |
Cookies can be accessed/modified from the browser process using StoragePartition::GetCookieManagerForBrowserProcess, but that doesn't provide the same proxy capabilities that we currently offer via CefRequestContextHandler::GetCookieHandler. We might need to register some handlers in the network service/process directly. Call stack for creation of the NetworkContext:
CefContentBrowserClient::CreateNetworkContext calls Profile::CreateNetworkContext which results in a call to ProfileNetworkContextService::CreateNetworkContextParams which sets CookieManagerParams values. The NetworkContext is uniquely identified by these parameters:
The |relative_partition_path| value, if non-empty, will be appended to the BrowserContext's cache path. If we use |
It's probably time that we move to the intended Chromium model (see discussion here) and get rid of the special CefRequestContextHandler::GetCookieHandler handling (which currently requires all kinds of hacks in Chromium code to implement SiteInstance proxying). Just create your CefBrowser with a different CefRequestContext and cache_path if you want separate cookie storage. We can then re-implement CefCookieManager to use StoragePartition::GetCookieManagerForBrowserProcess for access to the underlying cookie storage. |
Add initial NetworkService support (see issue #2622). This change adds an --enable-network-service command-line flag to run with To test: Run There should be no functional change when running without the NetworkService → <<cset 6b2c1fe9693f (bb)>> |
Remove methods that modify cookie storage at runtime (see issue #2622). This change removes cookie and request handler functionality that will not The following methods have been removed:
The following methods have been renamed:
This change substantially simplifies the network implementation in CEF because To test: Verify that → <<cset a23e8452443d (bb)>> |
Move the frame/handler association to CefResourceContext (see issue #2622). A CefBrowserHostImpl is created using a CefRequestContextImpl that may have a This change forwards frame create/delete notifications from CefBrowserHostImpl To test: Verify that all ceftests pass with NetworkService disabled. → <<cset ea27dff33839 (bb)>> |
Remove Chromium patches that are no longer required (see issue #2622). → <<cset 9b43d265c38d (bb)>> |
Enforce cache_path requirements for NetworkService (see issue #2622). This change adds a new CefSettings.root_cache_path value that must be either To test: Run cefclient with a combination of the following flags: --cache-path=c:\temp\cache --request-context-per-browser --enable-network-service --disable-extensions Known issues:
→ <<cset b65f336f8126 (bb)>> |
Original comment by Mike Wiedenbauer (Bitbucket: shagkur, GitHub: shagkur). @magreenblatt Is it still valid to retrieve the CefRequestContext(Impl) and hence the CefBrowserContext via the "GetRequestContextOnUIThread" function in CefURLRequest? Because URLLoader and SimpleURLLoader need a URLLoaderFactory which, if i understand CefBrowserContext correctly, is provided by that class? |
@shagkur The logic in GetRequestContextOnUIThread for retrieving the CefRequestContextImpl and CefBrowserContext looks fine. You should then be able to retrieve the URLLoaderFactory via |
Add CookieManagerImpl for NetworkService (see issue #2622). To test: Run There should be no functional change when running without the NetworkService Known issues:
→ <<cset 85c34c4dcf43 (bb)>> |
Move CookieManager callbacks to the UI thread (see issue #2622). The Chromium content layer (which also exposes the NetworkService interface) To test: Run ceftests. They should pass as expected. → <<cset b949d86c40f8 (bb)>> |
|
The NetworkService process is created with a command line like:
You can debug the NetworkService by running a CEF-based application with A ContentUtilityClient::RegisterNetworkBinders method will also be called on startup. There does not appear to be a capability for configuring the NetworkService directly from inside that process. Instead, |
We need to call CookieMonster::SetCookieableSchemes in the NetworkProcess after the CookieMonster is created [1] and before CookieMonster::MarkCookieStoreAsInitialized is called for the first time [2]. Simply adding a CookieManager::SetCookieableSchemes method (in cookie_manager.h and cookie_manager.mojom) is not sufficient because URLLoaderFactory::CreateLoaderAndStart will always be called first. CookieManagerParams (discussed above) has the following members:
These are checked via CookieSettings::GetCookieSetting but not currently considered by NetworkContext::ApplyContextParamsToBuilder when creating the CookieMonster (where they could be accessed via The best option is probably to add a new value in CookieManagerParams which would be set in ProfileNetworkContextService::CreateNetworkContextParams. On the NetworkService side this value would be passed to CookieMonster::SetCookieableSchemes immediately after the CookieMonster is created in NetworkContext::ApplyContextParamsToBuilder. [1] Creation call stack:
[2] MarkCookieStoreAsInitialized call stack:
|
Original comment by Mike Wiedenbauer (Bitbucket: shagkur, GitHub: shagkur). @magreenblatt I guess i then just can call CefBrowserContext::GetURLLoaderFactory(), as it seems it's exactly doing what you described i should do?:) I'll create a new class "CefURLLoader" which will be, implementation wise, based on SimpleURLLoader. |
@shagkur Sounds good, I didn't realize we had that method :D |
Fix crash on shutdown with extensions enabled (see issue #2622). We don't support the WebRequestAPI yet, so we shouldn't be creating any objects To test: Run → <<cset 9ce29e8ec5cc (bb)>> |
Fix CHECK on startup when running with --trace-startup=mojom (see issue #2622). → <<cset 941d53ebfd57 (bb)>> |
Mojo messages related to the NetworkService can be debugged as follows:
You'll see a view like this (go here for general information on using the tracing tool): |
Original comment by Mike Wiedenbauer (Bitbucket: shagkur, GitHub: shagkur). @magreenblatt I've created the follwing PR: https://bitbucket.org/chromiumembedded/cef/pull-requests/227 |
…hromiumembedded#2699, see issue chromiumembedded#2622). Modifying the URL in OnBeforeResourceLoad causes an internal redirect response. In cases where the request is cross-origin and credentials mode is 'include' the redirect response must include the "Access-Control-Allow-Credentials" header, otherwise the request will be blocked.
… (fixes issue chromiumembedded#2709, see issue chromiumembedded#2622) Requests from the PDF viewer are not associated with a CefBrowser. Consequently, the InterceptedRequestHandler for those requests will register as an observer of CefContext destruction. When the browser is closed the InterceptedRequestHandler is destroyed and an async task is posted to remove/delete the observer on the UI thread. If CefShutdown is then called the task may execute after shutdown has started, in which case CONTEXT_STATE_VALID() will return false. We still need to remove the observer in this case to avoid a use-after-free in FinishShutdownOnUIThread.
The `--disable-features=NetworkService` flag is no longer supported.
…mbedded#2622) The implementation of this option was removed in commit 67b61c4. Certificate transparency is disabled by default for Chromium embedders. Details at: https://chromium.googlesource.com/chromium/src/+/master/net/docs/certificate-transparency.md#Supporting-Certificate-Transparency-for-Embedders
…hromiumembedded#2622) Limited test coverage for the old API is still available by passing the `--test-old-resource-api` command-line flag to ceftests.
…iumembedded#2622). This is a speculative fix for a crash where |on_disconnect_| appears to be null in ProxyURLLoaderFactory::MaybeDestroySelf. The hypothesis here is that OnURLLoaderClientError is being called while the proxy object is still in-flight to ResourceContextData::AddProxy (e.g. before SetDisconnectCallback has been called for the proxy object). Additonally, this change protects against MaybeDestroySelf attempting to execute |on_disconnect_| multiple times.
…chromiumembedded#2622). Initialization of request objects requires asynchronous hops between the UI and IO threads. In some cases the browser may be destroyed, the mojo connection may be aborted, or the ProxyURLLoaderFactory object may be deleted while initialization is still in progress. This change fixes crashes and adds unit tests that try to reproduce these conditions. To test: Run `ceftests --gtest_repeat=50 --gtest_filter=ResourceRequestHandlerTest.Basic*Abort*`
…hromiumembedded#2622). This is a speculative fix for a crash where the pending ResourceRequest appears to be invalid after the request is continued from SetInitialized.
…xes issue chromiumembedded#2685, see issue chromiumembedded#2622). Determine external request status based on the current URL instead of the request initiator.
…miumembedded#2695, see issue chromiumembedded#2622). Modifying the URL in OnBeforeResourceLoad causes an internal redirect response. In cases where the request is cross-origin (containing a non-null "Origin" header) the redirect response must include the "Access-Control-Allow-Origin" header, otherwise the request will be blocked. This change also fixes a problem where existing request headers would be discarded if the request was modified in OnBeforeResourceLoad.
…, see issue chromiumembedded#2622). For 303 redirects all request methods except HEAD are converted to GET as per the latest http draft. For historical reasons the draft also allows POST requests to be converted to GETs when following 301/302 redirects. Most major browsers do this and so shall we. When a request is converted to GET any POST data should also be removed. Use 307 redirects instead if you want the request to be repeated using the same method and POST data.
… see issue chromiumembedded#2622). When NetworkService is enabled requests created using CefFrame::CreateURLRequest will call CefRequestHandler::GetAuthCredentials for the associated browser after calling CefURLRequestClient::GetAuthCredentials if that call returns false.
…hromiumembedded#2699, see issue chromiumembedded#2622). Modifying the URL in OnBeforeResourceLoad causes an internal redirect response. In cases where the request is cross-origin and credentials mode is 'include' the redirect response must include the "Access-Control-Allow-Credentials" header, otherwise the request will be blocked.
… (fixes issue chromiumembedded#2709, see issue chromiumembedded#2622) Requests from the PDF viewer are not associated with a CefBrowser. Consequently, the InterceptedRequestHandler for those requests will register as an observer of CefContext destruction. When the browser is closed the InterceptedRequestHandler is destroyed and an async task is posted to remove/delete the observer on the UI thread. If CefShutdown is then called the task may execute after shutdown has started, in which case CONTEXT_STATE_VALID() will return false. We still need to remove the observer in this case to avoid a use-after-free in FinishShutdownOnUIThread.
… (fixes issue chromiumembedded#2709, see issue chromiumembedded#2622) Requests from the PDF viewer are not associated with a CefBrowser. Consequently, the InterceptedRequestHandler for those requests will register as an observer of CefContext destruction. When the browser is closed the InterceptedRequestHandler is destroyed and an async task is posted to remove/delete the observer on the UI thread. If CefShutdown is then called the task may execute after shutdown has started, in which case CONTEXT_STATE_VALID() will return false. We still need to remove the observer in this case to avoid a use-after-free in FinishShutdownOnUIThread.
…hromiumembedded#2622) Limited test coverage for the old API is still available by passing the `--test-old-resource-api` command-line flag to ceftests.
…mbedded#2622) The implementation of this option was removed in commit 67b61c4. Certificate transparency is disabled by default for Chromium embedders. Details at: https://chromium.googlesource.com/chromium/src/+/master/net/docs/certificate-transparency.md#Supporting-Certificate-Transparency-for-Embedders
Original report by Mike Wiedenbauer (Bitbucket: shagkur, GitHub: shagkur).
Chromium is moving from an in-process network stack implementation to a separate NetworkService process.
Tracking issue and design docs at https://bugs.chromium.org/p/chromium/issues/detail?id=598073
Related embedder-dev@ thread: https://groups.google.com/a/chromium.org/d/msg/embedder-dev/DYE94KHLMx4/g_vhK2T4CAAJ
Chromium NetworkService status updates are being sent to the services-dev@ mailing list. Here's the most recent update: https://groups.google.com/a/chromium.org/d/msg/services-dev/bhiYEx0d10I/c-VgO4scBgAJ. Based on this update I'm guessing the old code path will be removed after M75 branches (mid-April).
The text was updated successfully, but these errors were encountered: