commit | f0285cde6e11a1178dbdb30a3503ae6603526c88 | [log] [tgz] |
---|---|---|
author | Antonio Gomes <tonikitoo@igalia.com> | Mon Jun 18 21:51:38 2018 |
committer | Commit Bot <commit-bot@chromium.org> | Mon Jun 18 21:51:38 2018 |
tree | f9a08b749fbd6bf3dd15b727ecf9d7e4ae871cf3 | |
parent | f988ce79e2d72cd00817583def5a7b46177bc79a [diff] |
Reland "Migrate ContentHashFetcher to SimpleURLLoader" This is a reland of 27f8891c7cd9a86b2c92508cf6818726df4058d6 The main difference from the original CL is that instead of passing raw network::mojom::URLLoaderFactory pointers, this CL takes a more conservative approach where a thread-agnostic network::mojom::URLLoaderFactoryPtrInfo instance is passed through ContentHash::FetchParams, which allows us to "bind" the network::URLLoaderFactoryPtr instance only when it is actually going to be used (in ContentHashFetcher::Start). Also, differently than the original/reverted CL, this CL does not change the threads ContentHash methods run on. Last, in order to adapt ContentVerifierHashTest, we: 1) Replace the use of net::TestURLRequestInterceptor by content::URLLoaderInterceptor. 2) Install the interceptor at the beginning of each test execution flow, so that it is able to intercept all relevant URLLoaderFactory creation requests. Some other minor differences, were because of some preparation CLs already landed, eg https://crrev.com/c/1089273. Original change's description: > Migrate ContentHashFetcher to SimpleURLLoader > > This CL migrates ContentHashFetcher away from URLRequestFetcher to > SimpleURLLoader. > > In order to accomplish this, a few other related changes are also performed: > > 1) The regular flow of the code is ContentVerifier -> ContentHash -> > ContentHashFetcher. In ContentVerifier, for instance, UI, IO and > "File tasks" threads take place. > Previously, the URLRequestFetcher logic residing in ContentHashFetcher > executed in the "file tasks" thread. > > As part of the migration from URLRequestFetcher to SimpleURLLoader > machinery, this CL also changes the ContentHashFetcher logic to > execute to the IO thread. > Note that it could be possible to keep the new SimpleURLLoader logic > in the "file tasks" thread. However, this would impose a way bigger change, > and require unittests to be considerably rewritten as well (see for > patchsets 7, 8 and 9. > The issue is that mojo objects are not thread safe. We could create a new > URLLoaderFactory for the extensions thread, but then we'd need to create a > new one when the network service crashes (Something this SharedURLLoaderFactory > one, already bound to the IOThread, magically handles), but that would be > significantly more complicated. > > 2) content_verifier_hash_fetch_behavior_browsertest.cc was changed > to support running with the network service feature both enabled > and disabled. This is a pattern that is also present in various other > unittests. > > In summary, the migration to use SimpleURLLoader performed here includes > also a change in the thread that ContentHashFetcher runs on, but no > functionality change is expected from this CL. > > BUG=773295,844926 > > Change-Id: If570f69d01ff75ac59d8d043f8687621336dddcf > Reviewed-on: https://chromium-review.googlesource.com/1056587 > Commit-Queue: Antonio Gomes <tonikitoo@igalia.com> > Reviewed-by: Devlin <rdevlin.cronin@chromium.org> > Reviewed-by: Matt Menke <mmenke@chromium.org> > Cr-Commit-Position: refs/heads/master@{#561480} Bug: 773295, 844926 Cq-Include-Trybots: luci.chromium.try:linux_mojo Change-Id: Ie2f3665bcca5378ff67bf78bec38bdc225ff6c26 Reviewed-on: https://chromium-review.googlesource.com/1076947 Commit-Queue: Antonio Gomes <tonikitoo@igalia.com> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Matt Menke <mmenke@chromium.org> Cr-Commit-Position: refs/heads/master@{#568178}
Chromium is an open-source browser project that aims to build a safer, faster, and more stable way for all users to experience the web.
The project's web site is https://www.chromium.org.
Documentation in the source is rooted in docs/README.md.
Learn how to Get Around the Chromium Source Code Directory Structure .