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

Issue 715673 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
Proj-Servicification

Blocked on:
issue 689549
issue 712913
issue 747130
issue 748187
issue 761468
issue 776509

Blocking:
issue 598073


Participants' hotlists:
EnamelAndFriendsFixIt


Sign in to add a comment

Change how safe browsing checks are performed

Project Member Reported by jam@chromium.org, Apr 26 2017

Issue description

The motivation is that safe browsing wouldn't hook into net/ directly, which is needed to get networking out of process.

The design doc is here: https://docs.google.com/document/d/1cCmfhVJHPJLNjlTj7ztmAhdYgdx1W7bN3uEHa0q5dTk/edit#heading=h.bpvmkllpqyp

The thoughts so far are (assuming PlzNavigate):
-navigations can synchronously check in the browser process before making the request to the network service
-for subresources, the renderer can race the request to the network service and to safe browsing. if the network service responds first, it blocks waiting for the safe browsing check. this is similar to how android races (in the browser process)
 

Comment 2 by jam@chromium.org, May 2 2017

Blockedon: 712913
Components: Services>Safebrowsing
Adding Safebrowsing component for more visibility.
Labels: SafeBrowsing-Triaged

Comment 5 by lpz@chromium.org, May 5 2017

Cc: lpz@chromium.org

Comment 6 by yzshen@chromium.org, May 24 2017

Components: Internals>Network>Service
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 1 2017

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

commit 2d8fb42490681813d05294e44166615c03a6aaff
Author: yzshen <yzshen@chromium.org>
Date: Thu Jun 01 20:29:40 2017

This CL:
- introduces mojom interface for safe browsing URL checks. This interface is registered on each render frame.
- introduces a C++ interface URLLoaderThrottle in content/ for renderers, which is similar to ResourceThrottle at the browser side; implements a subclass SafeBrowsingURLLoaderThrottle in chrome/.
- extends ContentRendererClient::WillSendRequest() to also return a list of throttle objects, so that chrome can attach the safe browsing throttle.
- provides ThrottlingURLLoader which wraps URLLoader(Factory) mojom interfaces and handles throttling properly.

The following will be in follow-up CLs:
- make sure it works on Android. The work should be minimal. But I haven't got a chance to test it on Android so I didn't bother adding Android-specific code.
- delete cache entry when safe browsing considers a URL as bad.
- handle sync loading.

BUG= 715673 

Review-Url: https://codereview.chromium.org/2900563002
Cr-Commit-Position: refs/heads/master@{#476414}

[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/browser/BUILD.gn
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/browser/chrome_content_browser_manifest_overlay.json
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/browser/safe_browsing/mojo_safe_browsing_impl.cc
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/browser/safe_browsing/mojo_safe_browsing_impl.h
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/common/BUILD.gn
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/common/safe_browsing.mojom
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/renderer/BUILD.gn
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/renderer/chrome_content_renderer_client.h
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/renderer/chrome_content_renderer_client_browsertest.cc
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/renderer/safe_browsing/safe_browsing_url_loader_throttle.cc
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/chrome/renderer/safe_browsing/safe_browsing_url_loader_throttle.h
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/child/BUILD.gn
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/child/request_extra_data.h
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/child/resource_dispatcher.cc
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/child/resource_dispatcher.h
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/child/resource_dispatcher_unittest.cc
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/child/throttling_url_loader.cc
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/child/throttling_url_loader.h
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/child/throttling_url_loader_unittest.cc
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/child/url_loader_client_impl_unittest.cc
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/child/url_response_body_consumer_unittest.cc
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/child/web_url_loader_impl.cc
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/child/web_url_loader_impl_unittest.cc
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/public/child/BUILD.gn
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/public/child/url_loader_throttle.h
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/public/common/BUILD.gn
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/public/common/resource_type.mojom
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/public/common/resource_type.typemap
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/public/common/resource_type_enum_traits.cc
[add] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/public/common/resource_type_enum_traits.h
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/public/common/typemaps.gni
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/public/renderer/content_renderer_client.cc
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/public/renderer/content_renderer_client.h
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/content/test/BUILD.gn
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/extensions/shell/renderer/shell_content_renderer_client.cc
[modify] https://crrev.com/2d8fb42490681813d05294e44166615c03a6aaff/extensions/shell/renderer/shell_content_renderer_client.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 6 2017

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

commit 208145bd191e5d51595c96d4173a0c6f4513435b
Author: yzshen <yzshen@chromium.org>
Date: Tue Jun 06 00:47:42 2017

Network service: move URLLoaderThrottle and ThrottlingURLLoader.

The files are moved from content/[public/]chlid/ to content/[public/]common/, so
that they can be used in the browser process as well.

In follow-up CLs, they will be used by browser-initiated SB checks.

BUG= 715673 

Review-Url: https://codereview.chromium.org/2914423002
Cr-Commit-Position: refs/heads/master@{#477134}

[modify] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/chrome/renderer/chrome_content_renderer_client_browsertest.cc
[modify] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/chrome/renderer/safe_browsing/safe_browsing_url_loader_throttle.h
[modify] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/child/BUILD.gn
[modify] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/child/request_extra_data.h
[modify] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/child/resource_dispatcher.cc
[modify] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/child/resource_dispatcher.h
[modify] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/common/BUILD.gn
[rename] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/common/throttling_url_loader.cc
[rename] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/common/throttling_url_loader.h
[rename] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/common/throttling_url_loader_unittest.cc
[modify] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/public/child/BUILD.gn
[modify] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/public/common/BUILD.gn
[rename] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/public/common/url_loader_throttle.h
[modify] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/208145bd191e5d51595c96d4173a0c6f4513435b/content/test/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 7 2017

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

commit e2e435ef7d38319633f7a01d051f2760af8806c7
Author: yzshen <yzshen@chromium.org>
Date: Wed Jun 07 17:05:13 2017

Make content::ThrottlingURLLoader take a task runner and more efficient.

- The task runner is used to dispatch URLLoaderClient messages.
- Previously it had got many members to cache params when operations were
  deferred. This CL groups those params into structs and only creates instances
  when necessary.

BUG= 729769 ,  715673 

Review-Url: https://codereview.chromium.org/2926693002
Cr-Commit-Position: refs/heads/master@{#477687}

[modify] https://crrev.com/e2e435ef7d38319633f7a01d051f2760af8806c7/content/child/resource_dispatcher.cc
[modify] https://crrev.com/e2e435ef7d38319633f7a01d051f2760af8806c7/content/child/url_loader_client_impl.cc
[modify] https://crrev.com/e2e435ef7d38319633f7a01d051f2760af8806c7/content/child/url_loader_client_impl.h
[modify] https://crrev.com/e2e435ef7d38319633f7a01d051f2760af8806c7/content/common/throttling_url_loader.cc
[modify] https://crrev.com/e2e435ef7d38319633f7a01d051f2760af8806c7/content/common/throttling_url_loader.h

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 16 2017

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

commit f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2
Author: yzshen <yzshen@chromium.org>
Date: Fri Jun 16 04:44:51 2017

Move safe_browsing.mojom into components/safe_browsing.

This way it could be used by Android WebView.

BUG= 715673 

Review-Url: https://codereview.chromium.org/2936773002
Cr-Commit-Position: refs/heads/master@{#479949}

[modify] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/chrome/browser/BUILD.gn
[modify] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/chrome/browser/safe_browsing/mojo_safe_browsing_impl.cc
[modify] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/chrome/browser/safe_browsing/mojo_safe_browsing_impl.h
[modify] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/chrome/common/BUILD.gn
[modify] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/chrome/renderer/BUILD.gn
[modify] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/chrome/renderer/DEPS
[modify] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/chrome/renderer/chrome_content_renderer_client.h
[modify] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/chrome/renderer/safe_browsing/safe_browsing_url_loader_throttle.cc
[modify] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/chrome/renderer/safe_browsing/safe_browsing_url_loader_throttle.h
[modify] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/components/safe_browsing/common/BUILD.gn
[modify] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/components/safe_browsing/common/OWNERS
[rename] https://crrev.com/f36f0f6fbc6bb6aa4fcce9e68bcf467ff68d65c2/components/safe_browsing/common/safe_browsing.mojom

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 16 2017

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

commit efcb7c795d6d84d1ab17e17d51efef624d79ce81
Author: yzshen <yzshen@chromium.org>
Date: Fri Jun 16 23:12:30 2017

Network service: SafeBrowsing check for frame-resources from browser.

This CL:
- adds safe_browsing::BrowserURLLoaderThrottle (which is a URLLoaderThrottle subclass) to throttle browser-side URL loading.

- adds ContentBrowserClient::CreateURLLoaderThrottles() and implements it in ChromeContentBrowserClient to return a safe_browsing::BrowserURLLoaderThrottle instance.

- changes NavigationURLLoaderNetworkService to use ThrottlingURLLoader on the IO thread. It no longer owns URLLoaderPtr or Binding<URLLoaderClient> on the UI thread. That means I had to proxy calls of these two interfaces between the IO and UI thread. But that is necessary because ThrottlingURLLoader needs to apply throttles living on the IO thread.

- changes ThrottlingURLLoader to also support StartLoaderCallback, which we use in place of URLLoaderFactory in the browser.

- uses PossiblyAssociatedInterfacePtr<URLLoader> and gets rid of AssociatedURLLoaderWrapper. It is less code and more efficient. (Eventually we will use non-associated URLLoaderPtr consistently.)

BUG= 715673 

Review-Url: https://codereview.chromium.org/2924723002
Cr-Commit-Position: refs/heads/master@{#480212}

[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/chrome/browser/BUILD.gn
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/chrome/browser/chrome_content_browser_client.h
[add] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/chrome/browser/safe_browsing/browser_url_loader_throttle.cc
[add] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/chrome/browser/safe_browsing/browser_url_loader_throttle.h
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/chrome/browser/safe_browsing/mojo_safe_browsing_impl.cc
[add] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/chrome/browser/safe_browsing/safe_browsing_url_checker_impl.cc
[add] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/chrome/browser/safe_browsing/safe_browsing_url_checker_impl.h
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/chrome/renderer/BUILD.gn
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/chrome/renderer/chrome_content_renderer_client.cc
[rename] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/chrome/renderer/safe_browsing/renderer_url_loader_throttle.cc
[add] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/chrome/renderer/safe_browsing/renderer_url_loader_throttle.h
[delete] https://crrev.com/958a49a3e9f2f2fe39e5059f73f660e89de8fba3/chrome/renderer/safe_browsing/safe_browsing_url_loader_throttle.h
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/content/browser/loader/navigation_url_loader_network_service.h
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/content/child/resource_dispatcher.cc
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/content/common/BUILD.gn
[rename] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/content/common/possibly_associated_interface_ptr.h
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/content/common/throttling_url_loader.cc
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/content/common/throttling_url_loader.h
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/content/common/throttling_url_loader_unittest.cc
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/content/public/browser/content_browser_client.h
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/content/renderer/BUILD.gn
[modify] https://crrev.com/efcb7c795d6d84d1ab17e17d51efef624d79ce81/content/renderer/renderer_blink_platform_impl.h

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 18 2017

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

commit 44094e255a6f73633b4e4703c5121df37e894676
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Tue Jul 18 22:52:35 2017

SafeBrowsing for Network Service: refactor the browser-side code.

This is a preparation for adding support for Android WebView:
- moves some files from chrome/browser to components/safe_browsing/browser.
- introduces UrlCheckerDelegate interface and moves chrome-specific logic into a subclass of UrlCheckerDelegate.

Bug= 715673 

Change-Id: I7c7e440f5146e6b7928e71265e7d5657f2c5b295
Reviewed-on: https://chromium-review.googlesource.com/567586
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487644}
[modify] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/chrome/browser/safe_browsing/BUILD.gn
[add] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/chrome/browser/safe_browsing/url_checker_delegate_impl.cc
[add] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/chrome/browser/safe_browsing/url_checker_delegate_impl.h
[modify] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/components/safe_browsing/DEPS
[modify] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/components/safe_browsing/browser/BUILD.gn
[modify] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/components/safe_browsing/browser/DEPS
[rename] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/components/safe_browsing/browser/browser_url_loader_throttle.cc
[rename] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/components/safe_browsing/browser/browser_url_loader_throttle.h
[rename] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/components/safe_browsing/browser/mojo_safe_browsing_impl.cc
[rename] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/components/safe_browsing/browser/mojo_safe_browsing_impl.h
[rename] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/components/safe_browsing/browser/safe_browsing_url_checker_impl.cc
[rename] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/components/safe_browsing/browser/safe_browsing_url_checker_impl.h
[add] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/components/safe_browsing/browser/url_checker_delegate.h
[modify] https://crrev.com/44094e255a6f73633b4e4703c5121df37e894676/components/safe_browsing/common/BUILD.gn

Comment 13 by jam@chromium.org, Jul 20 2017

Blockedon: 747130
Blockedon: 748187
Project Member

Comment 15 by bugdroid1@chromium.org, Jul 28 2017

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

commit 6060d58b42df4b996d6fc97a47adfdcf29e6142f
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Fri Jul 28 04:42:33 2017

SafeBrowsing: extend safe_browsing.mojom intefaces.

This CL extends the interfaces in safe_browsing.mojom:
- passes more info related to the request from client to impl in order to meet
  the requirements of Android WebView;
- passes "showed_interstitial" from impl to client so that the client could
  record metrics more accurately.

This CL also removes some duplicate code between the old/new code path by making
SafeBrowsingResourceThrottle calls into safe_browsing::UrlCheckerDelegateImpl on
Chrome.

BUG= 715673 
TBR=jam@chromium.org (deferred to other reviewers)

Change-Id: If7f25c3bc03bd35a8c7450615b15a47b7e727130
Reviewed-on: https://chromium-review.googlesource.com/582468
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490264}
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/android_webview/BUILD.gn
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/chrome/browser/loader/safe_browsing_resource_throttle.cc
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/chrome/browser/loader/safe_browsing_resource_throttle.h
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/chrome/browser/safe_browsing/url_checker_delegate_impl.cc
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/chrome/browser/safe_browsing/url_checker_delegate_impl.h
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/chrome/renderer/BUILD.gn
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/browser/browser_url_loader_throttle.cc
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/browser/browser_url_loader_throttle.h
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/browser/mojo_safe_browsing_impl.cc
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/browser/mojo_safe_browsing_impl.h
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/browser/safe_browsing_url_checker_impl.cc
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/browser/safe_browsing_url_checker_impl.h
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/browser/url_checker_delegate.h
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/common/safe_browsing.mojom
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/renderer/BUILD.gn
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/renderer/DEPS
[rename] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/renderer/renderer_url_loader_throttle.cc
[rename] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/renderer/renderer_url_loader_throttle.h
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/renderer/websocket_sb_handshake_throttle.cc
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/renderer/websocket_sb_handshake_throttle.h
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/components/safe_browsing/renderer/websocket_sb_handshake_throttle_unittest.cc
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/content/common/throttling_url_loader.cc
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/content/common/throttling_url_loader_unittest.cc
[modify] https://crrev.com/6060d58b42df4b996d6fc97a47adfdcf29e6142f/content/public/common/url_loader_throttle.h

Project Member

Comment 16 by bugdroid1@chromium.org, Aug 9 2017

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

commit 7a6f47e3a5e77e77fcd6334285dcb942db0916d2
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Wed Aug 09 06:03:51 2017

SafeBrowsing for network service: add support in android webView.

This change moves most of AwSafeBrowsingResourceThrottle contents into
AwUrlCheckerDelegateImpl, so that it could be shared between the
old/new code path.

Bug= 715673 

Change-Id: I3bff2bf3d0373462a6817622242bb6cc3ca0a6f0
Reviewed-on: https://chromium-review.googlesource.com/568405
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Tao Bai <michaelbai@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492846}
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/BUILD.gn
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/browser/aw_browser_manifest_overlay.json
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/browser/aw_content_browser_client.cc
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/browser/aw_content_browser_client.h
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/browser/aw_contents_client_bridge.cc
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/browser/aw_contents_client_bridge.h
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/browser/aw_safe_browsing_resource_throttle.cc
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/browser/aw_safe_browsing_resource_throttle.h
[add] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/browser/aw_url_checker_delegate_impl.cc
[add] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/browser/aw_url_checker_delegate_impl.h
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/browser/net/aw_web_resource_request.cc
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/browser/net/aw_web_resource_request.h
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/renderer/aw_content_renderer_client.cc
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/android_webview/renderer/aw_content_renderer_client.h
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/components/safe_browsing/base_resource_throttle.cc
[modify] https://crrev.com/7a6f47e3a5e77e77fcd6334285dcb942db0916d2/components/safe_browsing/base_resource_throttle.h

Project Member

Comment 17 by bugdroid1@chromium.org, Aug 15 2017

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

commit 6499bea9d6b7ae6f1c7b5e548977d04346befae5
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Tue Aug 15 21:00:23 2017

SB new code path: add histograms and trace events.

BUG= 715673 

Change-Id: I50b57d3b02d6dc9ae3c385da4764509ca36a53c5
Reviewed-on: https://chromium-review.googlesource.com/611330
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494538}
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/BUILD.gn
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/base_resource_throttle.cc
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/base_ui_manager.cc
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/base_ui_manager.h
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/browser/BUILD.gn
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/browser/browser_url_loader_throttle.cc
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/browser/browser_url_loader_throttle.h
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/browser/safe_browsing_url_checker_impl.cc
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/browser/safe_browsing_url_checker_impl.h
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/common/utils.cc
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/common/utils.h
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/renderer/BUILD.gn
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/renderer/renderer_url_loader_throttle.cc
[modify] https://crrev.com/6499bea9d6b7ae6f1c7b5e548977d04346befae5/components/safe_browsing/renderer/renderer_url_loader_throttle.h

Project Member

Comment 18 by bugdroid1@chromium.org, Aug 22 2017

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

commit 161941e6a49a41314655dceb8e4da6782f8240f6
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Tue Aug 22 16:34:57 2017

SafeBrowsingUrlCheckerImpl: make sure it is safe to destroy the object while it runs callbacks.

Bug:  715673 , 748187 
Change-Id: I2a127b64e7fab5fc8461dbbefdc600dff2e743dd
Reviewed-on: https://chromium-review.googlesource.com/624527
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496326}
[modify] https://crrev.com/161941e6a49a41314655dceb8e4da6782f8240f6/components/safe_browsing/browser/safe_browsing_url_checker_impl.cc
[modify] https://crrev.com/161941e6a49a41314655dceb8e4da6782f8240f6/components/safe_browsing/browser/safe_browsing_url_checker_impl.h

Project Member

Comment 19 by bugdroid1@chromium.org, Aug 22 2017

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

commit 5e35aff5a58b32e79125e6a1fa99e98df71e0d87
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Tue Aug 22 22:55:42 2017

Add a feature flag "S13nSafeBrowsingParallelUrlCheck".

If enabled, SafeBrowsing URL checks will behave in the same way as when Network
Service is enabled. In other words, they don't defer starting requests or
following redirects, no matter on desktop or mobile. Instead they only defer
response processing.

Currently the feature is disabled by default. Follow-up work:
- Make the corresponding change on Android WebView.
- Following the finch experiment process, send out a doc/email to request a
  finch experiment.
- Remove the old behavior eventually.

Bug:  715673 
Change-Id: I9b23efc58965a82dcb3ba0ccaa91a77bdccbca92
Reviewed-on: https://chromium-review.googlesource.com/625034
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Varun Khaneja (slow) <vakh@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496481}
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/chrome/browser/loader/safe_browsing_resource_throttle.cc
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/chrome/browser/loader/safe_browsing_resource_throttle.h
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/BUILD.gn
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/base_resource_throttle.cc
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/base_resource_throttle.h
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/browser/BUILD.gn
[add] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/browser/base_parallel_resource_throttle.cc
[add] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/browser/base_parallel_resource_throttle.h
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/browser/browser_url_loader_throttle.cc
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/browser/browser_url_loader_throttle.h
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/browser/safe_browsing_url_checker_impl.cc
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/browser/safe_browsing_url_checker_impl.h
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/features.cc
[modify] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/features.h
[add] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/net_event_logger.cc
[add] https://crrev.com/5e35aff5a58b32e79125e6a1fa99e98df71e0d87/components/safe_browsing/net_event_logger.h

Project Member

Comment 20 by bugdroid1@chromium.org, Aug 31 2017

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

commit 48f0446021e72269b571966b31b4b13ca1109a29
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Thu Aug 31 20:48:46 2017

Android WebView support for feature flag "S13nSafeBrowsingParallelUrlCheck".

If enabled, SafeBrowsing URL checks will behave in the same way as when
Network Service is enabled. In other words, they don't defer starting requests
or following redirects. Instead they only defer response processing.

Currently the feature is disabled by default. Follow-up work:
- Following the finch experiment process, send out a doc/email to request a
  finch experiment.
- Remove the old behavior eventually.

Bug:  715673 
Change-Id: I4e0ba91ffebd0d909656e071c19d010faf90924c
Reviewed-on: https://chromium-review.googlesource.com/642365
Reviewed-by: Selim Gurun <sgurun@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498989}
[modify] https://crrev.com/48f0446021e72269b571966b31b4b13ca1109a29/android_webview/browser/aw_safe_browsing_resource_throttle.cc
[modify] https://crrev.com/48f0446021e72269b571966b31b4b13ca1109a29/android_webview/browser/aw_safe_browsing_resource_throttle.h
[modify] https://crrev.com/48f0446021e72269b571966b31b4b13ca1109a29/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/48f0446021e72269b571966b31b4b13ca1109a29/components/safe_browsing/browser/base_parallel_resource_throttle.cc
[modify] https://crrev.com/48f0446021e72269b571966b31b4b13ca1109a29/components/safe_browsing/browser/base_parallel_resource_throttle.h

Blockedon: 761468
Project Member

Comment 22 by bugdroid1@chromium.org, Sep 15 2017

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

commit e7b81ba027495b0f8ac6674e72cbd4561c42edec
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Fri Sep 15 00:18:10 2017

SafeBrowsing URL checks: supporting notification of starting slow checks.

This notification will be used by the URL check initiator (e.g.,
ThrottlingURLLoader) to ask the network service to pause reading response body.
It is useful to reduce the time window during which the network service could
write potentially unsafe resources into disk cache.

Bug:  715673 
Change-Id: Ibc606998a58709df8901a85915942af0d8986f83
Reviewed-on: https://chromium-review.googlesource.com/661888
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502107}
[modify] https://crrev.com/e7b81ba027495b0f8ac6674e72cbd4561c42edec/components/safe_browsing/browser/browser_url_loader_throttle.cc
[modify] https://crrev.com/e7b81ba027495b0f8ac6674e72cbd4561c42edec/components/safe_browsing/browser/browser_url_loader_throttle.h
[modify] https://crrev.com/e7b81ba027495b0f8ac6674e72cbd4561c42edec/components/safe_browsing/browser/mojo_safe_browsing_impl.cc
[modify] https://crrev.com/e7b81ba027495b0f8ac6674e72cbd4561c42edec/components/safe_browsing/browser/safe_browsing_url_checker_impl.cc
[modify] https://crrev.com/e7b81ba027495b0f8ac6674e72cbd4561c42edec/components/safe_browsing/browser/safe_browsing_url_checker_impl.h
[modify] https://crrev.com/e7b81ba027495b0f8ac6674e72cbd4561c42edec/components/safe_browsing/common/safe_browsing.mojom
[modify] https://crrev.com/e7b81ba027495b0f8ac6674e72cbd4561c42edec/components/safe_browsing/renderer/renderer_url_loader_throttle.cc
[modify] https://crrev.com/e7b81ba027495b0f8ac6674e72cbd4561c42edec/components/safe_browsing/renderer/renderer_url_loader_throttle.h
[modify] https://crrev.com/e7b81ba027495b0f8ac6674e72cbd4561c42edec/components/safe_browsing/renderer/websocket_sb_handshake_throttle.cc
[modify] https://crrev.com/e7b81ba027495b0f8ac6674e72cbd4561c42edec/components/safe_browsing/renderer/websocket_sb_handshake_throttle.h
[modify] https://crrev.com/e7b81ba027495b0f8ac6674e72cbd4561c42edec/components/safe_browsing/renderer/websocket_sb_handshake_throttle_unittest.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Sep 16 2017

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

commit 0bec954a7aef23107caebe44b10c4868670a0737
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Sat Sep 16 01:31:59 2017

Extend URLLoader mojo interface to support pause/resume caching response body.

Two methods are added:
- PauseCachingResponseBody();
- ResumeCachingResponseBody();

These methods will be used later by ThrottlingURLLoader. When it gets
notified by SafeBrowsing that a resource is potentially unsafe, it needs
to notify the network service to pause caching. That way we minimize the
time window in which the network service could write unsafe resources
into disk cache.

The reason *not* to introduce Pause/Resume() instead: although they are more
general-purposed, Pause/Resume() apply to all kinds of URLLoaders which means
we will need to implement these methods for all of them. That would be a
waste of effort because the current use case only cares about pause/resume
caching when fetching from network.

This CL only changes the interface. The actual implementation in network
service will be in a follow-up CL.

BUG= 715673 

Change-Id: If718e5aeabe0019c91dd95e9b988f8dda3d861c6
Reviewed-on: https://chromium-review.googlesource.com/667783
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502464}
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/browser/appcache/appcache_url_loader_job.h
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/browser/blob_storage/blob_url_loader_factory.cc
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/browser/loader/mojo_async_resource_handler.cc
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/browser/loader/mojo_async_resource_handler.h
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/browser/service_worker/service_worker_fetch_dispatcher.cc
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/browser/service_worker/service_worker_script_url_loader.cc
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/browser/service_worker/service_worker_script_url_loader.h
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/browser/service_worker/service_worker_url_loader_job.cc
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/browser/service_worker/service_worker_url_loader_job.h
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/child/loader/cors_url_loader.cc
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/child/loader/cors_url_loader.h
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/child/service_worker/service_worker_subresource_loader.cc
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/child/service_worker/service_worker_subresource_loader.h
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/network/url_loader_impl.cc
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/network/url_loader_impl.h
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/public/common/simple_url_loader_unittest.cc
[modify] https://crrev.com/0bec954a7aef23107caebe44b10c4868670a0737/content/public/common/url_loader.mojom

Project Member

Comment 24 by bugdroid1@chromium.org, Sep 28 2017

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

commit 99681b18f6fbb1d5725b55f12cdc03d9926b540f
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Thu Sep 28 06:11:10 2017

URLLoader: mechanical renaming of Pause/ResumeCachingResponseBody().

These methods are renamed to Pause/ResumeReadingBodyFromNet().

After some discussion (crosreview.com/679612), people agreed that it is
more clear to use "pause reading from network" than "pause caching".

BUG= 715673 

Change-Id: Ia5809ada65a5b1ca112d66131bcbde1b01a5e019
Reviewed-on: https://chromium-review.googlesource.com/688219
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504919}
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/browser/appcache/appcache_url_loader_job.h
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/browser/blob_storage/blob_url_loader_factory.cc
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/browser/loader/mojo_async_resource_handler.cc
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/browser/loader/mojo_async_resource_handler.h
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/browser/service_worker/service_worker_fetch_dispatcher.cc
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/browser/service_worker/service_worker_script_url_loader.cc
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/browser/service_worker/service_worker_script_url_loader.h
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/browser/service_worker/service_worker_url_loader_job.cc
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/browser/service_worker/service_worker_url_loader_job.h
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/child/loader/cors_url_loader.cc
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/child/loader/cors_url_loader.h
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/child/service_worker/service_worker_subresource_loader.cc
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/child/service_worker/service_worker_subresource_loader.h
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/network/url_loader_impl.cc
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/network/url_loader_impl.h
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/public/common/simple_url_loader_unittest.cc
[modify] https://crrev.com/99681b18f6fbb1d5725b55f12cdc03d9926b540f/content/public/common/url_loader.mojom

Project Member

Comment 25 by bugdroid1@chromium.org, Oct 2 2017

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

commit c0c0e0360568965be2d4438f369d7508f42ce2b3
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Mon Oct 02 20:29:22 2017

Network service: pause/resume reading body from network controlled by SafeBrowsing throttles.

This CL:
- implements Pause/ResumeReadingBodyFromNet() for URLLoaderImpl.
- extends ThrottlingURLLoader and its delegate interface to support
  Pause/ResumeReadingBodyFromNet().
- makes browser/renderer SafeBrowsing throttles pause/resume when
  necessary.

BUG= 715673 

Change-Id: I055c2c5052e3dc50346e53ae9fb2eb6241353ba3
Reviewed-on: https://chromium-review.googlesource.com/679612
Reviewed-by: Randy Smith <rdsmith@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505759}
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/components/safe_browsing/browser/browser_url_loader_throttle.cc
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/components/safe_browsing/browser/browser_url_loader_throttle.h
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/components/safe_browsing/renderer/renderer_url_loader_throttle.cc
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/components/safe_browsing/renderer/renderer_url_loader_throttle.h
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/content/common/throttling_url_loader.cc
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/content/common/throttling_url_loader.h
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/content/common/throttling_url_loader_unittest.cc
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/content/network/DEPS
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/content/network/url_loader_impl.cc
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/content/network/url_loader_impl.h
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/content/network/url_loader_unittest.cc
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/content/public/common/url_loader_throttle.cc
[modify] https://crrev.com/c0c0e0360568965be2d4438f369d7508f42ce2b3/content/public/common/url_loader_throttle.h

Project Member

Comment 26 by bugdroid1@chromium.org, Oct 2 2017

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

commit b78b1dfcb91968dc5cd7e526a9a31c0d90f440de
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Mon Oct 02 22:26:42 2017

Introduce Network.URLLoader.BodyReadBeforePaused histogram.

This histogram records how much, in bytes, of the response body has been read
from network by a URLLoader before it pauses reading, when it receives a
PauseReadingBodyFromNet() call. If there are multiple calls to
PauseReadingBodyFromNet(), only a single value is recorded for the last call.
This histogram is recorded by URLLoader implementations that fetch from network.
When SafeBrowsing indicates that a resource may be unsafe and therefore a more
time-consuming check is required to classify it, reading response body from
network is paused in order to reduce the chance of writing unsafe contents into
cache. This histogram is useful to evaluate how much data is cached during this
window.

BUG= 715673 

Change-Id: I4e0878b10f4622ce48113b25a107df6248f16126
Reviewed-on: https://chromium-review.googlesource.com/685747
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Randy Smith <rdsmith@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505815}
[modify] https://crrev.com/b78b1dfcb91968dc5cd7e526a9a31c0d90f440de/content/network/url_loader_impl.cc
[modify] https://crrev.com/b78b1dfcb91968dc5cd7e526a9a31c0d90f440de/content/network/url_loader_impl.h
[modify] https://crrev.com/b78b1dfcb91968dc5cd7e526a9a31c0d90f440de/tools/metrics/histograms/histograms.xml

Project Member

Comment 27 by bugdroid1@chromium.org, Oct 10 2017

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

commit fae705d4ae416d8478a92bcb30a87b8ff8a757f2
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Tue Oct 10 07:12:48 2017

URLLoaderThrottle: make sure a throttle is not destroyed synchronously when it is calling into its delegate.

Bug:  715673 
Change-Id: Id0569d4f9e16097eed9f05bb4b999dd6cc1add83
Reviewed-on: https://chromium-review.googlesource.com/703737
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507607}
[modify] https://crrev.com/fae705d4ae416d8478a92bcb30a87b8ff8a757f2/content/common/throttling_url_loader.cc
[modify] https://crrev.com/fae705d4ae416d8478a92bcb30a87b8ff8a757f2/content/common/throttling_url_loader.h
[modify] https://crrev.com/fae705d4ae416d8478a92bcb30a87b8ff8a757f2/content/common/throttling_url_loader_unittest.cc
[modify] https://crrev.com/fae705d4ae416d8478a92bcb30a87b8ff8a757f2/content/public/common/url_loader_throttle.h

Project Member

Comment 28 by bugdroid1@chromium.org, Oct 12 2017

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

commit af74316ab93bf63b1b43af591106d4a3d479deb8
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Thu Oct 12 00:13:45 2017

safe_browsing::BaseParallelResourceThrottle fixes.

- makes sure that the BrowserURLLoaderThrottle is not destroyed synchronously while it is calling into its delegate. This is guaranteed by URLLoaderThrottle::Delegate interface spec.
- makes MustProcessResponseBeforeReadingBody() return true. This ensures that response is not cached before SafeBrowsing confirms that it is safe to do so.

BUG= 715673 

Change-Id: Ifdb7d86e17fc376a62b91a484661595936b24994
Reviewed-on: https://chromium-review.googlesource.com/698927
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Randy Smith <rdsmith@chromium.org>
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508185}
[modify] https://crrev.com/af74316ab93bf63b1b43af591106d4a3d479deb8/components/safe_browsing/browser/base_parallel_resource_throttle.cc
[modify] https://crrev.com/af74316ab93bf63b1b43af591106d4a3d479deb8/components/safe_browsing/browser/base_parallel_resource_throttle.h

Project Member

Comment 29 by bugdroid1@chromium.org, Oct 17 2017

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

commit d8e44fb94e19d1a340508160cbaa7e5083fedd28
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Tue Oct 17 16:49:18 2017

Add yzshen@ as owner for some SafeBrowsing code under components/safe_browsing.

BUG= 715673 

Change-Id: I5d2439550c4253bc0cfc43b6b50585e26f4e45af
Reviewed-on: https://chromium-review.googlesource.com/717066
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Nathan Parker <nparker@chromium.org>
Reviewed-by: Jialiu Lin (Slow Oct 13-20, OOO Oct  21-29) <jialiul@chromium.org>
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509415}
[modify] https://crrev.com/d8e44fb94e19d1a340508160cbaa7e5083fedd28/components/safe_browsing/OWNERS

Blockedon: 776509
Project Member

Comment 31 by bugdroid1@chromium.org, Oct 26 2017

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

commit cf93c232dd2b5ac77b2dd2d16358a8257cf49cc5
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Thu Oct 26 22:21:30 2017

SafeBrowsing URL check: change the new code path to use trace events for logging.

Previously it uses net event log, which is not available if the network stack is
run in a separate process.

Bug= 715673 

Change-Id: I1d8108558e88dfd5649db76f2a796a312c951363
Reviewed-on: https://chromium-review.googlesource.com/736432
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511986}
[modify] https://crrev.com/cf93c232dd2b5ac77b2dd2d16358a8257cf49cc5/components/safe_browsing/browser/base_parallel_resource_throttle.cc
[modify] https://crrev.com/cf93c232dd2b5ac77b2dd2d16358a8257cf49cc5/components/safe_browsing/browser/base_parallel_resource_throttle.h
[modify] https://crrev.com/cf93c232dd2b5ac77b2dd2d16358a8257cf49cc5/components/safe_browsing/browser/browser_url_loader_throttle.cc
[modify] https://crrev.com/cf93c232dd2b5ac77b2dd2d16358a8257cf49cc5/components/safe_browsing/browser/browser_url_loader_throttle.h
[modify] https://crrev.com/cf93c232dd2b5ac77b2dd2d16358a8257cf49cc5/components/safe_browsing/browser/safe_browsing_url_checker_impl.cc
[modify] https://crrev.com/cf93c232dd2b5ac77b2dd2d16358a8257cf49cc5/components/safe_browsing/browser/safe_browsing_url_checker_impl.h
[modify] https://crrev.com/cf93c232dd2b5ac77b2dd2d16358a8257cf49cc5/components/safe_browsing/renderer/renderer_url_loader_throttle.cc
[modify] https://crrev.com/cf93c232dd2b5ac77b2dd2d16358a8257cf49cc5/components/safe_browsing/renderer/renderer_url_loader_throttle.h

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
Labels: Hotlist-EnamelAndFriendsFixIt
Project Member

Comment 34 by bugdroid1@chromium.org, Jan 8 2018

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

commit 0f69279f3c83a0de09fea1b1c066e0bd1227e0fd
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Mon Jan 08 22:25:54 2018

Make S13nSafeBrowsingParallelUrlCheck the default behavior.

This CL removes the old behavior and the feature flag.

BUG= 777028 , 715673 

Change-Id: Idaee7f5161d77b146930150c0b98afa6952fed99
Reviewed-on: https://chromium-review.googlesource.com/847819
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527784}
[modify] https://crrev.com/0f69279f3c83a0de09fea1b1c066e0bd1227e0fd/android_webview/browser/aw_safe_browsing_resource_throttle.cc
[modify] https://crrev.com/0f69279f3c83a0de09fea1b1c066e0bd1227e0fd/android_webview/browser/aw_safe_browsing_resource_throttle.h
[modify] https://crrev.com/0f69279f3c83a0de09fea1b1c066e0bd1227e0fd/chrome/browser/loader/safe_browsing_resource_throttle.cc
[modify] https://crrev.com/0f69279f3c83a0de09fea1b1c066e0bd1227e0fd/chrome/browser/loader/safe_browsing_resource_throttle.h
[modify] https://crrev.com/0f69279f3c83a0de09fea1b1c066e0bd1227e0fd/components/safe_browsing/BUILD.gn
[delete] https://crrev.com/69ce64a1b5eabdf06186c33f91d8f8ba76cd2f02/components/safe_browsing/base_resource_throttle.cc
[delete] https://crrev.com/69ce64a1b5eabdf06186c33f91d8f8ba76cd2f02/components/safe_browsing/base_resource_throttle.h
[modify] https://crrev.com/0f69279f3c83a0de09fea1b1c066e0bd1227e0fd/components/safe_browsing/browser/base_parallel_resource_throttle.h
[modify] https://crrev.com/0f69279f3c83a0de09fea1b1c066e0bd1227e0fd/components/safe_browsing/browser/browser_url_loader_throttle.h
[modify] https://crrev.com/0f69279f3c83a0de09fea1b1c066e0bd1227e0fd/components/safe_browsing/browser/safe_browsing_url_checker_impl.cc
[modify] https://crrev.com/0f69279f3c83a0de09fea1b1c066e0bd1227e0fd/components/safe_browsing/browser/safe_browsing_url_checker_impl.h
[modify] https://crrev.com/0f69279f3c83a0de09fea1b1c066e0bd1227e0fd/components/safe_browsing/features.cc
[modify] https://crrev.com/0f69279f3c83a0de09fea1b1c066e0bd1227e0fd/components/safe_browsing/features.h
[delete] https://crrev.com/69ce64a1b5eabdf06186c33f91d8f8ba76cd2f02/components/safe_browsing/net_event_logger.cc
[delete] https://crrev.com/69ce64a1b5eabdf06186c33f91d8f8ba76cd2f02/components/safe_browsing/net_event_logger.h
[modify] https://crrev.com/0f69279f3c83a0de09fea1b1c066e0bd1227e0fd/testing/variations/fieldtrial_testing_config.json

Project Member

Comment 35 by bugdroid1@chromium.org, Feb 1 2018

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

commit 19cafda00c1a3d1c905b26ead8c103ccef044937
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Thu Feb 01 19:45:20 2018

Add Pause/ResumeReadingBodyFromNet support to MojoAsyncResourceHandler.

This is needed in order to switch frame resource loading to use the new
safe_browsing::BrowserURLLoaderThrottle (once feature NavigationMojoResponse is
enabled).

Bug:  715673 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: If7919dbdd2ad0c8b09c38a0ba350f04c02ab5f3b
Reviewed-on: https://chromium-review.googlesource.com/887796
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533781}
[modify] https://crrev.com/19cafda00c1a3d1c905b26ead8c103ccef044937/content/browser/loader/mojo_async_resource_handler.cc
[modify] https://crrev.com/19cafda00c1a3d1c905b26ead8c103ccef044937/content/browser/loader/resource_handler.cc
[modify] https://crrev.com/19cafda00c1a3d1c905b26ead8c103ccef044937/content/browser/loader/resource_handler.h
[modify] https://crrev.com/19cafda00c1a3d1c905b26ead8c103ccef044937/content/browser/loader/resource_loader.cc
[modify] https://crrev.com/19cafda00c1a3d1c905b26ead8c103ccef044937/content/browser/loader/resource_loader.h
[modify] https://crrev.com/19cafda00c1a3d1c905b26ead8c103ccef044937/content/browser/loader/resource_loader_unittest.cc
[modify] https://crrev.com/19cafda00c1a3d1c905b26ead8c103ccef044937/content/browser/loader/test_resource_handler.cc
[modify] https://crrev.com/19cafda00c1a3d1c905b26ead8c103ccef044937/content/browser/loader/test_resource_handler.h
[modify] https://crrev.com/19cafda00c1a3d1c905b26ead8c103ccef044937/services/network/public/interfaces/url_loader.mojom
[modify] https://crrev.com/19cafda00c1a3d1c905b26ead8c103ccef044937/services/network/url_loader.cc
[modify] https://crrev.com/19cafda00c1a3d1c905b26ead8c103ccef044937/services/network/url_loader.h
[modify] https://crrev.com/19cafda00c1a3d1c905b26ead8c103ccef044937/services/network/url_loader_unittest.cc

Status: Fixed (was: Assigned)
I think it is code complete now, for both the pre-network-service and post-network-service code paths.

The remaining work is to conduct Finch experiment for S13nSafeBrowsingCheckByURLLoaderThrottle. Please see crbug.com/807399 for more details.

Sign in to add a comment