New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 825242 link

Starred by 3 users
Status: Fixed
Owner:
mmenke@chromium.org
Closed: Jun 2018
Cc:
█ lpz@chromium.org
jam@chromium.org
mmenke@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug
Proj-Servicification

Blocking:
issue 598073



Sign in to add a comment

Don't use URLRequestContexts in the browser when network service is enabled

Project Member Reported by mmenke@chromium.org, Mar 23 2018 
Currently, when the network service is enabled, we still create a stub in-process URLRequestContext, which causes a lot of tests of code that's not network-service-ready to incorrectly pass, since it can be used to make URLRequests, DNS requests, access the network quality estimator, etc.

We should look into disabling it in tests, so we have a better way of discovering code that depends on the legacy in-process network stack.  Unfortunately, if we just removed the in-process URLRequestContext completely, all browser tests would crash.  There are enough things going on at startup that have hard dependencies on the legacy pathway that even just disabling it for tests will be a major undertaking.

Comment 1 by jam@chromium.org, Mar 23 2018

Owner: jam@chromium.org
Status: Assigned (was: Untriaged)
Summary: Don't use URLRequestContexts in the browser when network service is enabled (was: Add a way to disable the legacy in-process URLRequestContext)
I suspect the number of features that uses the URLRequestContext is not that large, but like you say they might run during all tests. I'm not sure that adding a flag to disable them would be useful, because each conversion is a fixed cost.

I'll take a look at the existing usages and look into converting them to use NetworkContext/URLLoaderFactory instead.

Comment 2 by mmenke@chromium.org, Mar 24 2018 

There are still over 100 URLFetcher consumers and 47 NetworkChangeNotifier consumers (Not all of which are in Chrome, but I suspect most are).  Other network stack objects are less frequently used, but they are used.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 28 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6

commit 71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6
Author: John Abd-El-Malek <jam@chromium.org>
Date: Wed Mar 28 19:52:36 2018

Move components/certificate_reporting back to src/chrome since it didn't end up getting used on iOS.

This allows the helper in https://chromium-review.googlesource.com/c/chromium/src/+/982104 to stay in src/chrome.

Bug: 516697,  825242 
Change-Id: I0410562c1913c25d2b38bd18fbef2233c4f8c8b5
Reviewed-on: https://chromium-review.googlesource.com/983755
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546573}
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/BUILD.gn
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/safe_browsing/certificate_reporting_service.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/safe_browsing/certificate_reporting_service.h
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/safe_browsing/certificate_reporting_service_browsertest.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/safe_browsing/certificate_reporting_service_test_utils.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/safe_browsing/certificate_reporting_service_unittest.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/BUILD.gn
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/bad_clock_blocking_page.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/captive_portal_blocking_page.cc
[rename] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/cert_logger.proto
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/cert_report_helper.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/cert_report_helper.h
[add] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/certificate_error_report.cc
[rename] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/certificate_error_report.h
[rename] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/certificate_error_report_unittest.cc
[rename] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/certificate_error_reporter.cc
[rename] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/certificate_error_reporter.h
[rename] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/certificate_error_reporter_unittest.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/certificate_reporting_test_utils.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/certificate_reporting_test_utils.h
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/mitm_software_blocking_page.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/ssl_blocking_page.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/ssl_blocking_page_base.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/ssl_blocking_page_base.h
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/ssl_browsertest.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ssl/ssl_error_navigation_throttle_unittest.cc
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/chrome/test/BUILD.gn
[modify] https://crrev.com/71e4f9f4f2341fc0405434a0e9bb14ddbaa078b6/components/BUILD.gn
[delete] https://crrev.com/42624492cd5e80855f1dd3a5a8146b258def2c2a/components/certificate_reporting/BUILD.gn
[delete] https://crrev.com/42624492cd5e80855f1dd3a5a8146b258def2c2a/components/certificate_reporting/DEPS
[delete] https://crrev.com/42624492cd5e80855f1dd3a5a8146b258def2c2a/components/certificate_reporting/OWNERS
[delete] https://crrev.com/42624492cd5e80855f1dd3a5a8146b258def2c2a/components/certificate_reporting/error_report.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 28 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/19fcd2685b4af798be5d736627881c1a8c091320

commit 19fcd2685b4af798be5d736627881c1a8c091320
Author: John Abd-El-Malek <jam@chromium.org>
Date: Wed Mar 28 22:12:39 2018

Create a helper method in chrome/ to be used for replacing net::ReportSender for code in src/chrome.

Code in net/ that uses net::ReportSender will remain unchanged, as they will run in the network process. This replacement is for code which will run in the browser process. The helper wraps SimpleURLLoader.

Bug:  825242 
Change-Id: Ic66b2e62403c3786a02428cf8c0e4098c2d41a38
Reviewed-on: https://chromium-review.googlesource.com/982104
Reviewed-by: Emily Stark <estark@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546618}
[modify] https://crrev.com/19fcd2685b4af798be5d736627881c1a8c091320/chrome/browser/BUILD.gn
[add] https://crrev.com/19fcd2685b4af798be5d736627881c1a8c091320/chrome/browser/net/chrome_report_sender.cc
[add] https://crrev.com/19fcd2685b4af798be5d736627881c1a8c091320/chrome/browser/net/chrome_report_sender.h
[modify] https://crrev.com/19fcd2685b4af798be5d736627881c1a8c091320/net/url_request/report_sender.cc
[modify] https://crrev.com/19fcd2685b4af798be5d736627881c1a8c091320/net/url_request/report_sender.h
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 30 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d6257778e096e65c4d195b7a7a1b73fce363dd83

commit d6257778e096e65c4d195b7a7a1b73fce363dd83
Author: John Abd-El-Malek <jam@chromium.org>
Date: Fri Mar 30 22:30:16 2018

Create SafeBrowsingNetworkContext to help convert safe browsing code to use the network service.

This is a helper class which returns mojo interfaces for making requests. Eventually it will create
and own the URLRequestContext in the network process. For now, it simply wraps the
net::URLRequestContext that's in the browser. Once all calls to the safe browsing context use the
mojo APIs then we can change the internal SafeBrowsingNetworkContext implementation to create the
request context remotely.

Bug:  825242 
Change-Id: I6032d82febc72467ae1769e65a44eb57d28bdc71
Reviewed-on: https://chromium-review.googlesource.com/982260
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547291}
[modify] https://crrev.com/d6257778e096e65c4d195b7a7a1b73fce363dd83/chrome/browser/safe_browsing/safe_browsing_service.cc
[modify] https://crrev.com/d6257778e096e65c4d195b7a7a1b73fce363dd83/chrome/browser/safe_browsing/safe_browsing_service.h
[modify] https://crrev.com/d6257778e096e65c4d195b7a7a1b73fce363dd83/components/safe_browsing/browser/BUILD.gn
[modify] https://crrev.com/d6257778e096e65c4d195b7a7a1b73fce363dd83/components/safe_browsing/browser/DEPS
[add] https://crrev.com/d6257778e096e65c4d195b7a7a1b73fce363dd83/components/safe_browsing/browser/safe_browsing_network_context.cc
[add] https://crrev.com/d6257778e096e65c4d195b7a7a1b73fce363dd83/components/safe_browsing/browser/safe_browsing_network_context.h
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 2 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a8766bb5dae9ccc357f3a4d032ca8153d17fab71

commit a8766bb5dae9ccc357f3a4d032ca8153d17fab71
Author: John Abd-El-Malek <jam@chromium.org>
Date: Mon Apr 02 17:06:12 2018

Move WeakWrapperSharedURLLoaderFactory to content/public.

It's helpful for tests outside of content.

Bug:  825242 
Change-Id: I44c3f39a4c2c01bf31481c68bd095f7de34b1cbe
Reviewed-on: https://chromium-review.googlesource.com/989847
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547455}
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/browser/loader/prefetch_url_loader_service.cc
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/browser/loader/resource_message_filter.cc
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/browser/web_package/signed_exchange_cert_fetcher_unittest.cc
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/browser/web_package/web_package_prefetch_handler.cc
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/common/BUILD.gn
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/common/throttling_url_loader_unittest.cc
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/public/common/BUILD.gn
[rename] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/public/common/weak_wrapper_shared_url_loader_factory.cc
[rename] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/public/common/weak_wrapper_shared_url_loader_factory.h
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/renderer/loader/resource_dispatcher_unittest.cc
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/renderer/loader/url_loader_client_impl_unittest.cc
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/renderer/loader/url_response_body_consumer_unittest.cc
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/renderer/loader/web_url_loader_impl_unittest.cc
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/renderer/service_worker/service_worker_context_client.cc
[modify] https://crrev.com/a8766bb5dae9ccc357f3a4d032ca8153d17fab71/content/renderer/service_worker/service_worker_network_provider.cc
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 3 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c37fec27209442169da861f3b717dac2d71f68b

commit 7c37fec27209442169da861f3b717dac2d71f68b
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue Apr 03 04:09:37 2018

Convert Client Side Protection to use SimpleURLLoader instead of URLFetcher.

Also change network::TestURLLoaderFactory to behave more like net::FakeURLFetcherFactory to make
test conversions simpler. In particular, now it has persistent setting of responses instead of
being consumed once used.

Bug:  825242 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: If49f1671979267aa1551c8e391e6a6aae41cb017
Reviewed-on: https://chromium-review.googlesource.com/989835
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Nathan Parker <nparker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547621}
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/DEPS
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/client_side_detection_service.cc
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/client_side_detection_service.h
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/client_side_model_loader.cc
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/client_side_model_loader.h
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/client_side_model_loader_unittest.cc
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/protocol_manager.cc
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/protocol_manager.h
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/safe_browsing_service.cc
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/services_delegate.h
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/services_delegate_impl.cc
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/services_delegate_impl.h
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/services_delegate_stub.cc
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/chrome/browser/safe_browsing/services_delegate_stub.h
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/services/network/test/test_url_loader_factory.cc
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/services/network/test/test_url_loader_factory.h
[modify] https://crrev.com/7c37fec27209442169da861f3b717dac2d71f68b/services/network/test/test_url_loader_factory_unittest.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 3 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b4b5ddf11784888f500e3aedc93e3f16fa25263

commit 5b4b5ddf11784888f500e3aedc93e3f16fa25263
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue Apr 03 04:23:48 2018

Convert PasswordProtectionRequest to use SimpleURLLoader instead of URLFetcher.

Bug:  825242 
Change-Id: Id08270e339e5a7ae33d2374f77d56d2de0da2254
Reviewed-on: https://chromium-review.googlesource.com/988693
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547624}
[modify] https://crrev.com/5b4b5ddf11784888f500e3aedc93e3f16fa25263/chrome/browser/safe_browsing/chrome_password_protection_service.cc
[modify] https://crrev.com/5b4b5ddf11784888f500e3aedc93e3f16fa25263/components/safe_browsing/password_protection/BUILD.gn
[modify] https://crrev.com/5b4b5ddf11784888f500e3aedc93e3f16fa25263/components/safe_browsing/password_protection/DEPS
[modify] https://crrev.com/5b4b5ddf11784888f500e3aedc93e3f16fa25263/components/safe_browsing/password_protection/password_protection_request.cc
[modify] https://crrev.com/5b4b5ddf11784888f500e3aedc93e3f16fa25263/components/safe_browsing/password_protection/password_protection_request.h
[modify] https://crrev.com/5b4b5ddf11784888f500e3aedc93e3f16fa25263/components/safe_browsing/password_protection/password_protection_service.cc
[modify] https://crrev.com/5b4b5ddf11784888f500e3aedc93e3f16fa25263/components/safe_browsing/password_protection/password_protection_service.h
[modify] https://crrev.com/5b4b5ddf11784888f500e3aedc93e3f16fa25263/components/safe_browsing/password_protection/password_protection_service_unittest.cc
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 3 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1993375f641bc7f76dfd79c9266fab0736811dcb

commit 1993375f641bc7f76dfd79c9266fab0736811dcb
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue Apr 03 15:58:04 2018

Convert Incident Reporting to use SimpleURLLoader instead of URLFetcher.

Bug:  825242 
Change-Id: Idf2df12e459f26b91aafbb7cb434e79f26850ae8
Reviewed-on: https://chromium-review.googlesource.com/989118
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547720}
[add] https://crrev.com/1993375f641bc7f76dfd79c9266fab0736811dcb/chrome/browser/safe_browsing/incident_reporting/DEPS
[modify] https://crrev.com/1993375f641bc7f76dfd79c9266fab0736811dcb/chrome/browser/safe_browsing/incident_reporting/incident_report_uploader_impl.cc
[modify] https://crrev.com/1993375f641bc7f76dfd79c9266fab0736811dcb/chrome/browser/safe_browsing/incident_reporting/incident_report_uploader_impl.h
[modify] https://crrev.com/1993375f641bc7f76dfd79c9266fab0736811dcb/chrome/browser/safe_browsing/incident_reporting/incident_report_uploader_impl_unittest.cc
[modify] https://crrev.com/1993375f641bc7f76dfd79c9266fab0736811dcb/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
[modify] https://crrev.com/1993375f641bc7f76dfd79c9266fab0736811dcb/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.h
[modify] https://crrev.com/1993375f641bc7f76dfd79c9266fab0736811dcb/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service_unittest.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 3 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a6aadcce39e544094dc0ec25d538d4636462d03e

commit a6aadcce39e544094dc0ec25d538d4636462d03e
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue Apr 03 17:49:18 2018

Fix race that can cause UAF in SafeBrowsingNetworkContext::SharedURLLoaderFactory::InternalState.

The problem was the constructor was posting a task to a different thread, which could execute before the constructor returns and a reference is held.

This was seen in
https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.memory%2FLinux_ASan_LSan_Tests__1_%2F44439%2F%2B%2Frecipes%2Fsteps%2Fbrowser_tests%2F0%2Flogs%2FMSE_ExternalClearKey__x2f_EncryptedMediaTest.Playback_VP9Video_WebM_Subsample__x2f_0%2F0

Bug:  825242 
Change-Id: Ib851536adc57bfab14748e1f7324b143aff55033
Reviewed-on: https://chromium-review.googlesource.com/992876
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547758}
[modify] https://crrev.com/a6aadcce39e544094dc0ec25d538d4636462d03e/components/safe_browsing/browser/safe_browsing_network_context.cc
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 3 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40c999fb0c071dfabf18be1a9c2b074176486f4b

commit 40c999fb0c071dfabf18be1a9c2b074176486f4b
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue Apr 03 19:15:44 2018

Convert Download Protection to use SimpleURLLoader instead of URLFetcher.

Bug:  825242 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I21ab64e7570f7600031dbf566fc46214051bbc67
Reviewed-on: https://chromium-review.googlesource.com/989034
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547787}
[add] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/DEPS
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/check_client_download_request.cc
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/check_client_download_request.h
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/download_feedback.cc
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/download_feedback.h
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/download_feedback_service.cc
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/download_feedback_service.h
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/download_feedback_service_unittest.cc
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/download_feedback_unittest.cc
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/download_protection_service.cc
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/download_protection_service.h
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/ppapi_download_request.cc
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/ppapi_download_request.h
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/two_phase_uploader.cc
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/two_phase_uploader.h
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/download_protection/two_phase_uploader_unittest.cc
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/chrome/browser/safe_browsing/safe_browsing_service.h
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/services/network/test/test_url_loader_factory.cc
[modify] https://crrev.com/40c999fb0c071dfabf18be1a9c2b074176486f4b/services/network/test/test_url_loader_factory.h
Project Member

Comment 13 by bugdroid1@chromium.org, Apr 5 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/109b7fed0be71c74250b0c12879bbdabafb26e0e

commit 109b7fed0be71c74250b0c12879bbdabafb26e0e
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Apr 05 17:44:33 2018

Convert certificate reporting service to use SimpleURLLoader.

With this change, CertificateReportingService now only lives on the UI thread.

Bug:  825242 
TBR=rhalavati for test annotation removal

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I3945ab88a8d2f0507b4a2572df6c79edd7aab036
Reviewed-on: https://chromium-review.googlesource.com/996021
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548473}
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/net/trial_comparison_cert_verifier.h
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/net/trial_comparison_cert_verifier_unittest.cc
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/safe_browsing/certificate_reporting_service.cc
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/safe_browsing/certificate_reporting_service.h
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/safe_browsing/certificate_reporting_service_browsertest.cc
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/safe_browsing/certificate_reporting_service_factory.cc
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/safe_browsing/certificate_reporting_service_factory.h
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/safe_browsing/certificate_reporting_service_test_utils.cc
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/safe_browsing/certificate_reporting_service_test_utils.h
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/safe_browsing/certificate_reporting_service_unittest.cc
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/safe_browsing/safe_browsing_service.cc
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/safe_browsing/safe_browsing_service.h
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/ssl/DEPS
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/ssl/certificate_error_reporter.cc
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/ssl/certificate_error_reporter.h
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/chrome/browser/ssl/certificate_error_reporter_unittest.cc
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/services/network/test/test_url_loader_factory.cc
[modify] https://crrev.com/109b7fed0be71c74250b0c12879bbdabafb26e0e/tools/traffic_annotation/summary/annotations.xml
Project Member

Comment 14 by bugdroid1@chromium.org, Apr 10 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7

commit 7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue Apr 10 04:49:17 2018

Convert safe browsing ping manager to use SimpleURLLoader and SendReport.

With this change, the code now only lives on the UI thread.

Bug:  825242 , 777915
Change-Id: I3330347910559bb464a693a412182930415cb639
Reviewed-on: https://chromium-review.googlesource.com/998526
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Reviewed-by: Luke Z <lpz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549425}
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/android_webview/browser/aw_safe_browsing_blocking_page.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/android_webview/browser/aw_safe_browsing_ui_manager.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/android_webview/browser/aw_safe_browsing_ui_manager.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/notifications/platform_notification_service_impl.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/chrome_password_protection_service.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/notification_image_reporter.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/notification_image_reporter.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/notification_image_reporter_unittest.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/ping_manager.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/ping_manager.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/safe_browsing_service.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/safe_browsing_service.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/threat_details_unittest.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/trigger_creator.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/ui_manager.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/chrome/browser/safe_browsing/ui_manager.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/DEPS
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/base_ping_manager.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/base_ping_manager.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/base_ping_manager_unittest.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/base_ui_manager.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/base_ui_manager.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/browser/safe_browsing_url_request_context_getter.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/browser/threat_details.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/browser/threat_details.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/browser/threat_details_cache.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/browser/threat_details_cache.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/browser/threat_details_history.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/browser/threat_details_history.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/triggers/ad_sampler_trigger.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/triggers/ad_sampler_trigger.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/triggers/mock_trigger_manager.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/triggers/suspicious_site_trigger.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/triggers/suspicious_site_trigger.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/triggers/trigger_manager.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/triggers/trigger_manager.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/components/safe_browsing/triggers/trigger_manager_unittest.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/content/browser/loader/resource_loader.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/services/network/public/cpp/resource_response_info.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/services/network/public/cpp/resource_response_info.h
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/services/network/test/test_url_loader_factory.cc
[modify] https://crrev.com/7870ba0cd867fe9f4dd5b8d473f0f29f2b6740b7/services/network/url_loader.cc
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 10 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/01cb433767bbd1da18e9d3494eb1425071b9fb7e

commit 01cb433767bbd1da18e9d3494eb1425071b9fb7e
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue Apr 10 23:33:46 2018

Update V4UpdateProtocolManager, V4GetHashProtocolManager and V4LocalDatabaseManager to use SimpleURLLoader.

Also add a SharedURLLoaderFactory getter on SafeBrowsingService for IO thread users.

Bug:  825242 
Change-Id: Ib41699d932469b4bb92130103b5257509d6b2a36
Reviewed-on: https://chromium-review.googlesource.com/1002473
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549677}
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/android_webview/browser/aw_browser_context.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/android_webview/browser/aw_safe_browsing_ui_manager.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/android_webview/browser/aw_safe_browsing_ui_manager.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/chrome/browser/safe_browsing/local_database_manager.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/chrome/browser/safe_browsing/local_database_manager.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/chrome/browser/safe_browsing/safe_browsing_service.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/chrome/browser/safe_browsing/safe_browsing_service.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/chrome/browser/safe_browsing/services_delegate.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/chrome/browser/safe_browsing/services_delegate_impl.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/chrome/browser/safe_browsing/services_delegate_impl.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/chrome/browser/safe_browsing/services_delegate_stub.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/chrome/browser/safe_browsing/services_delegate_stub.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/android/remote_database_manager.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/android/remote_database_manager.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/BUILD.gn
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/DEPS
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/database_manager.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/database_manager.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/database_manager_unittest.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/test_database_manager.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/test_database_manager.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_get_hash_protocol_manager.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_get_hash_protocol_manager.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_get_hash_protocol_manager_unittest.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_local_database_manager.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_local_database_manager.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_local_database_manager_unittest.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_protocol_manager_util.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_protocol_manager_util.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_test_util.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_test_util.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_update_protocol_manager.cc
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_update_protocol_manager.h
[modify] https://crrev.com/01cb433767bbd1da18e9d3494eb1425071b9fb7e/components/safe_browsing/db/v4_update_protocol_manager_unittest.cc

Comment 16 Deleted

Comment 17 by jialiul@chromium.org, Apr 12 2018 

CL in #14 is causing crash in Debug builds. 

Sorry, I didn't catch this earlier. 
Manual repro:
1. go to  http://testsafebrowsing.appspot.com/
2. click on the first link, you should see an interstitial. Make sure the extended reporting check box is checked
3. click on "back to safety" button. 
4. see crash. Stacktrace looks like this:
[234283:234283:0411/181334.315018:FATAL:ui_manager.cc(176)] Check failed: ::content::BrowserThread::CurrentlyOn(BrowserThread::IO). Must be called on Chrome_IOThread; actually called on CrBrowserMain.
#0 0x559b55d9a81c base::debug::StackTrace::StackTrace()
#1 0x559b55dbb34b logging::LogMessage::~LogMessage()
#2 0x559b57f850bd safe_browsing::SafeBrowsingUIManager::SendSerializedThreatDetails()
#3 0x559b54bc9a2e safe_browsing::ThreatDetails::OnCacheCollectionReady()
#4 0x559b53a2336b _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo3edk12_GLOBAL__N_112ChannelPosixEFvvEJ13scoped_refptrIS6_EEEEFvvEE7RunOnceEPNS0_13BindStateBaseE
#5 0x559b55d9bee0 base::debug::TaskAnnotator::RunTask()
#6 0x559b55dc3db6 base::internal::IncomingTaskQueue::RunTask()
#7 0x559b55dc2247 base::MessageLoop::RunTask()
#8 0x559b55dc265a base::MessageLoop::DeferOrRunPendingTask()
#9 0x559b55dc28ee base::MessageLoop::DoWork()
#10 0x559b55dc68a9 base::MessagePumpGlib::Run()
#11 0x559b55dc1abc base::MessageLoop::Run()
#12 0x559b55df2b36 base::RunLoop::Run()
#13 0x559b55a10697 ChromeBrowserMainParts::MainMessageLoopRun()
#14 0x559b542e1a57 content::BrowserMainLoop::RunMainMessageLoopParts()
#15 0x559b542e5103 content::BrowserMainRunnerImpl::Run()
#16 0x559b542dda79 content::BrowserMain()
#17 0x559b559ddbb1 content::RunNamedProcessTypeMain()
#18 0x559b559decb2 content::ContentMainRunnerImpl::Run()
#19 0x559b559ec0d9 service_manager::Main()
#20 0x559b559dd084 content::ContentMain()
#21 0x559b5358f1b3 ChromeMain
#22 0x7f4dd423d2b1 __libc_start_main
#23 0x559b5358f02a _start

https://chromium-review.googlesource.com/c/chromium/src/+/998526/14/chrome/browser/safe_browsing/ui_manager.cc#176

Comment 18 by jialiul@chromium.org, Apr 12 2018

Cc: lpz@chromium.org
+cc lpz@

Comment 20 by jam@chromium.org, Apr 12 2018 

@jialiul: thanks, looking into this now.
Project Member

Comment 23 by bugdroid1@chromium.org, May 11 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/067364dfbb2b96b762a9f1b157d7f657f1bf6e36

commit 067364dfbb2b96b762a9f1b157d7f657f1bf6e36
Author: John Abd-El-Malek <jam@chromium.org>
Date: Fri May 11 03:09:18 2018

Don't use the browser-process URLRequestContext for safe browsing with network service.

Instead use a NetworkContext, which runs in the network process when network service is enabled.

Bug:  825242 ,  789640 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I1f1244fbc71968a78bbb8ad1235e69f9935658fa
Reviewed-on: https://chromium-review.googlesource.com/1008839
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Matt Mueller <mattm@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557776}
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/android_webview/browser/DEPS
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/android_webview/browser/aw_safe_browsing_ui_manager.cc
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/android_webview/browser/aw_safe_browsing_ui_manager.h
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/chrome/browser/net/network_context_configuration_browsertest.cc
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/chrome/browser/policy/policy_network_browsertest.cc
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/chrome/browser/safe_browsing/notification_image_reporter_unittest.cc
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/chrome/browser/safe_browsing/safe_browsing_service.cc
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/chrome/browser/safe_browsing/safe_browsing_service.h
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/components/safe_browsing/DEPS
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/components/safe_browsing/browser/safe_browsing_network_context.cc
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/components/safe_browsing/browser/safe_browsing_network_context.h
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/content/browser/network_service_instance.cc
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/content/public/browser/network_service_instance.h
[modify] https://crrev.com/067364dfbb2b96b762a9f1b157d7f657f1bf6e36/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Comment 24 by dxie@chromium.org, May 22 2018

Owner: xunji...@chromium.org
helen to follow up with matt and update the bug.

Comment 25 by dxie@chromium.org, May 22 2018

Labels: Hotlist-KnownIssue

Comment 26 by xunji...@chromium.org, May 25 2018

Owner: mmenke@chromium.org
mmenke@: Should we archive this bug and file individual bugs?
Most URLRequestContexts uses within the browser are by URLFetchers as you've noted in Comment #2. I believe we have tracking bugs for most of other cases.

Comment 27 by mmenke@chromium.org, May 25 2018 

My original plan was to use this for a CL disabling the URLRequestContext when the network service is enabled, and disabling a bunch of tests (A bit along the lines of what I mentioned at the meeting last week).  It does get us a bit better coverage of broken tests, depending on how thoroughly we disable it.  I have two CLs out for review in that space, so decided to put off poking at that CL until they land, but do still plan on getting back to it.

Comment 28 by xunji...@chromium.org, May 25 2018

Labels: -Pri-3 Proj-Servicification-Canary Pri-1
Ah, right. This is the stub setup for URLRequestContext that you mentioned last week. Thank you. I believe this blocks Canary as we do need a programmatic way to detect any incorrectly passing tests.

Comment 29 by dxie@chromium.org, Jun 8 2018

Labels: OS-Chrome OS-Windows OS-Mac OS-Linux
Project Member

Comment 30 by bugdroid1@chromium.org, Jun 16 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f34bba7c38758baf556aef6649fdd97f1b8a41a9

commit f34bba7c38758baf556aef6649fdd97f1b8a41a9
Author: Matt Menke <mmenke@chromium.org>
Date: Sat Jun 16 05:38:49 2018

Simplify in-process URLRequestContext creation with the NetworkService.

The old code hooked up things to the URLRequestContext that hadn't been
ported over to work with the NetworkService yet, when the NetworkService
was enabled. This CL just does the minimum setup that's needed to not
crash. It also makes requests made with the in-process URLRequestContext
fail when the network service is enabled. These changes will help
identify code that still depends on the legacy path, and allow for some
cleanup of URLRequestContextBuilderMojo and NetworkContext.

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I4c3f40e6dc3c235844846bff7d1d43d0b1c986d0
Bug:  825242 
Reviewed-on: https://chromium-review.googlesource.com/1096075
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567879}
[modify] https://crrev.com/f34bba7c38758baf556aef6649fdd97f1b8a41a9/chrome/browser/BUILD.gn
[modify] https://crrev.com/f34bba7c38758baf556aef6649fdd97f1b8a41a9/chrome/browser/io_thread.cc
[add] https://crrev.com/f34bba7c38758baf556aef6649fdd97f1b8a41a9/chrome/browser/net/failing_url_request_interceptor.cc
[add] https://crrev.com/f34bba7c38758baf556aef6649fdd97f1b8a41a9/chrome/browser/net/failing_url_request_interceptor.h
[modify] https://crrev.com/f34bba7c38758baf556aef6649fdd97f1b8a41a9/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/f34bba7c38758baf556aef6649fdd97f1b8a41a9/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
Project Member

Comment 31 by bugdroid1@chromium.org, Jun 18 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d6870941e6becaaa00888c02c5ff7697ce2c2c6f

commit d6870941e6becaaa00888c02c5ff7697ce2c2c6f
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Mon Jun 18 04:37:51 2018

Revert "Simplify in-process URLRequestContext creation with the NetworkService."

This reverts commit f34bba7c38758baf556aef6649fdd97f1b8a41a9.

Reason for revert: A lot of tests are failing on Mac10.10 and Mac10.12.
Looking at both blame[1][2] lists when the tests started failing, this
seems like the most likely culprit.

[1] https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.10%20Tests/33158
[2] https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.12%20Tests/13801

Original change's description:
> Simplify in-process URLRequestContext creation with the NetworkService.
>
> The old code hooked up things to the URLRequestContext that hadn't been
> ported over to work with the NetworkService yet, when the NetworkService
> was enabled. This CL just does the minimum setup that's needed to not
> crash. It also makes requests made with the in-process URLRequestContext
> fail when the network service is enabled. These changes will help
> identify code that still depends on the legacy path, and allow for some
> cleanup of URLRequestContextBuilderMojo and NetworkContext.
>
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: I4c3f40e6dc3c235844846bff7d1d43d0b1c986d0
> Bug:  825242 
> Reviewed-on: https://chromium-review.googlesource.com/1096075
> Commit-Queue: Matt Menke <mmenke@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#567879}

TBR=jam@chromium.org,mmenke@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  825242 
Change-Id: I36da9fb06fa89296e031c88d7c75c16e63f3b725
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1103818
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567932}
[modify] https://crrev.com/d6870941e6becaaa00888c02c5ff7697ce2c2c6f/chrome/browser/BUILD.gn
[modify] https://crrev.com/d6870941e6becaaa00888c02c5ff7697ce2c2c6f/chrome/browser/io_thread.cc
[delete] https://crrev.com/99a0b04e55cfb10c64437d898928e9de55c43b47/chrome/browser/net/failing_url_request_interceptor.cc
[delete] https://crrev.com/99a0b04e55cfb10c64437d898928e9de55c43b47/chrome/browser/net/failing_url_request_interceptor.h
[modify] https://crrev.com/d6870941e6becaaa00888c02c5ff7697ce2c2c6f/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/d6870941e6becaaa00888c02c5ff7697ce2c2c6f/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
Project Member

Comment 32 by bugdroid1@chromium.org, Jun 18 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/68b2894c4de2ebb35319813291c9d09533d379f8

commit 68b2894c4de2ebb35319813291c9d09533d379f8
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Mon Jun 18 08:12:42 2018

Reland "Simplify in-process URLRequestContext creation with the NetworkService."

This reverts commit d6870941e6becaaa00888c02c5ff7697ce2c2c6f.

Reason for revert: The original CL wasn't the cause of the mac bots
failing.

Original change's description:
> Revert "Simplify in-process URLRequestContext creation with the NetworkService."
> 
> This reverts commit f34bba7c38758baf556aef6649fdd97f1b8a41a9.
> 
> Reason for revert: A lot of tests are failing on Mac10.10 and Mac10.12.
> Looking at both blame[1][2] lists when the tests started failing, this
> seems like the most likely culprit.
> 
> [1] https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.10%20Tests/33158
> [2] https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.12%20Tests/13801
> 
> Original change's description:
> > Simplify in-process URLRequestContext creation with the NetworkService.
> >
> > The old code hooked up things to the URLRequestContext that hadn't been
> > ported over to work with the NetworkService yet, when the NetworkService
> > was enabled. This CL just does the minimum setup that's needed to not
> > crash. It also makes requests made with the in-process URLRequestContext
> > fail when the network service is enabled. These changes will help
> > identify code that still depends on the legacy path, and allow for some
> > cleanup of URLRequestContextBuilderMojo and NetworkContext.
> >
> > Cq-Include-Trybots: luci.chromium.try:linux_mojo
> > Change-Id: I4c3f40e6dc3c235844846bff7d1d43d0b1c986d0
> > Bug:  825242 
> > Reviewed-on: https://chromium-review.googlesource.com/1096075
> > Commit-Queue: Matt Menke <mmenke@chromium.org>
> > Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#567879}
> 
> TBR=jam@chromium.org,mmenke@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug:  825242 
> Change-Id: I36da9fb06fa89296e031c88d7c75c16e63f3b725
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Reviewed-on: https://chromium-review.googlesource.com/1103818
> Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#567932}

TBR=jam@chromium.org,mmenke@chromium.org,ortuno@chromium.org

Change-Id: I4f379142535672553027802c10d0a678a5734aad
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  825242 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1103858
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567948}
[modify] https://crrev.com/68b2894c4de2ebb35319813291c9d09533d379f8/chrome/browser/BUILD.gn
[modify] https://crrev.com/68b2894c4de2ebb35319813291c9d09533d379f8/chrome/browser/io_thread.cc
[add] https://crrev.com/68b2894c4de2ebb35319813291c9d09533d379f8/chrome/browser/net/failing_url_request_interceptor.cc
[add] https://crrev.com/68b2894c4de2ebb35319813291c9d09533d379f8/chrome/browser/net/failing_url_request_interceptor.h
[modify] https://crrev.com/68b2894c4de2ebb35319813291c9d09533d379f8/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/68b2894c4de2ebb35319813291c9d09533d379f8/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Comment 33 by mmenke@chromium.org, Jun 18 2018

Status: Fixed (was: Assigned)
Going to close this, though obviously we have a lot more to do.
Project Member

Comment 34 by bugdroid1@chromium.org, Jun 19 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/707fa79d5c6aad561da0dcc7626dda588465d8a4

commit 707fa79d5c6aad561da0dcc7626dda588465d8a4
Author: Matt Menke <mmenke@chromium.org>
Date: Tue Jun 19 00:24:19 2018

NetworkService: Clean up URLRequestContext creation.

As of https://chromium-review.googlesource.com/c/chromium/src/+/1096075,
URLRequestContextBuilderMojo no longer needs to call into
NetworkContext as part of IOThread/ProfileIOData setup when the network
service is enabled.

This means we can make NetworkContext::ApplyContextParamsToBuilder
non-static, and remove its rather long argument list.

Bug:  825242 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Iecf240f3091c9f57380e0f06d2e455532ccff4ab
Reviewed-on: https://chromium-review.googlesource.com/1104808
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568257}
[modify] https://crrev.com/707fa79d5c6aad561da0dcc7626dda588465d8a4/services/network/network_context.cc
[modify] https://crrev.com/707fa79d5c6aad561da0dcc7626dda588465d8a4/services/network/network_context.h
[modify] https://crrev.com/707fa79d5c6aad561da0dcc7626dda588465d8a4/services/network/url_request_context_builder_mojo.cc
[modify] https://crrev.com/707fa79d5c6aad561da0dcc7626dda588465d8a4/services/network/url_request_context_builder_mojo.h
►

Sign in to add a comment