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

Issue 700351 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 731744
issue 731747
issue 732373



Sign in to add a comment

componentize safe_browsing bits for WebView

Project Member Reported by timvolod...@chromium.org, Mar 10 2017

Issue description

- componentize safe_browsing, create component and move common/ bits
patch landed:
Issue 2450993003: Componentize safe_browsing [1]: create component, move messages, constants and switches.

- componentize safe_browsing renderer/ side
patch in flight:
crrev.com/2667343006: Componentize safe_browsing [X+1] : move the renderer part to component.

- also requires componentization of protos (in chrome/common/safe_browsing)
patch for review:
crrev.com/2743563006: Componentize safe_browsing [+1]: move protos to component

- ..


 
Description: Show this description
Components: Services>Safebrowsing
Status: Assigned (was: Untriaged)
Description: Show this description

Comment 5 by vakh@chromium.org, Mar 10 2017

Cc: jialiul@chromium.org
If these componentizations are only for WebView, you don't need to move the following files:
c/b/safe_browsing/client_side_*   -- they are not used in mobile, and will be deprecated soon.
c/b/safe_browsing/download*   -- no download protection on Android so far. 
c/b/safe_browsing/incident_reporting/*  -- no incident reporting on Android.
c/b/safe_browsing/permission*   -- no use for WebView
c/b/safe_browsing/notification*

w.r.t protos, client_model.proto, crx_info.proto, permission_report.proto are not useful for WebView. 

Hope those can simplify this issue a little bit. 
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 21 2017

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

commit c881a5d65d8e3276d186c57f9a91e378fa0141a4
Author: timvolodine <timvolodine@chromium.org>
Date: Tue Mar 21 02:46:36 2017

Componentize safe_browsing: move renderer/ reporting part for WebView.

Moves the chrome/renderer safe browsing reporting code to component
in order to make it available for Android WebView.

BUG= 700351 ,688629

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

[modify] https://crrev.com/c881a5d65d8e3276d186c57f9a91e378fa0141a4/chrome/browser/safe_browsing/threat_details.cc
[modify] https://crrev.com/c881a5d65d8e3276d186c57f9a91e378fa0141a4/chrome/renderer/BUILD.gn
[modify] https://crrev.com/c881a5d65d8e3276d186c57f9a91e378fa0141a4/chrome/renderer/DEPS
[modify] https://crrev.com/c881a5d65d8e3276d186c57f9a91e378fa0141a4/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/c881a5d65d8e3276d186c57f9a91e378fa0141a4/chrome/renderer/safe_browsing/DEPS
[modify] https://crrev.com/c881a5d65d8e3276d186c57f9a91e378fa0141a4/chrome/renderer/safe_browsing/threat_dom_details_browsertest.cc
[add] https://crrev.com/c881a5d65d8e3276d186c57f9a91e378fa0141a4/components/safe_browsing/renderer/BUILD.gn
[add] https://crrev.com/c881a5d65d8e3276d186c57f9a91e378fa0141a4/components/safe_browsing/renderer/DEPS
[rename] https://crrev.com/c881a5d65d8e3276d186c57f9a91e378fa0141a4/components/safe_browsing/renderer/threat_dom_details.cc
[rename] https://crrev.com/c881a5d65d8e3276d186c57f9a91e378fa0141a4/components/safe_browsing/renderer/threat_dom_details.h

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 24 2017

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

commit ac4f2ee6eef06c8c345eed0a01bf64ab3df642eb
Author: lpz <lpz@chromium.org>
Date: Fri Mar 24 15:09:00 2017

Remove disable warnings compile flag in safebrowsing component, and fix some type win64 type errors that were found.

BUG= 700351 , 702773 

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

[modify] https://crrev.com/ac4f2ee6eef06c8c345eed0a01bf64ab3df642eb/components/safe_browsing/renderer/BUILD.gn
[modify] https://crrev.com/ac4f2ee6eef06c8c345eed0a01bf64ab3df642eb/components/safe_browsing/renderer/threat_dom_details.cc

Labels: SafeBrowsing-Triaged
Status: Started (was: Assigned)
browser-side refactoring design doc for reference: https://goo.gl/TfoY9K

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 6 2017

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

commit 5d734ba20ca41dd77486aee0bdcb8ddf34165053
Author: timvolodine <timvolodine@chromium.org>
Date: Thu Apr 06 15:18:33 2017

Componentize safe_browsing: factor out chrome/ deps in ThreatDetailsRedirectsCollector.

In preparation for moving browser side reporting code to
the safe_browsing component perform some refactoring
to decrease dependency on the chrome/ layer.

In particular for threat_details_history.{h,cc}:
- factor out Profile,
- pass HistoryService in constructor,
- use HistoryServiceObserver instead of NotificationObserver
and chrome::NOTIFICATION_PROFILE_DESTROYED.

design doc:
https://goo.gl/8zVufq

BUG= 700351 , 688629

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

[modify] https://crrev.com/5d734ba20ca41dd77486aee0bdcb8ddf34165053/chrome/browser/safe_browsing/threat_details.cc
[modify] https://crrev.com/5d734ba20ca41dd77486aee0bdcb8ddf34165053/chrome/browser/safe_browsing/threat_details_history.cc
[modify] https://crrev.com/5d734ba20ca41dd77486aee0bdcb8ddf34165053/chrome/browser/safe_browsing/threat_details_history.h

Project Member

Comment 13 by bugdroid1@chromium.org, Apr 10 2017

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

commit 351fb5afeafd16e28c65cae7dfbe9b13d27f2786
Author: timvolodine <timvolodine@chromium.org>
Date: Mon Apr 10 15:44:38 2017

Componentize safe_browsing: decouple threat_details* from the chrome/ layer.

Decouple chrome/browser/safe_browsing/threat_details* from the
dependency on the chrome/ layer. Also update the relevant tests.
This is in preparation for moving the reporting code to the
components/safe_browsing/.. to be usable by Android WebView.

Design doc:
https://goo.gl/8zVufq

BUG= 700351 , 688629

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

[modify] https://crrev.com/351fb5afeafd16e28c65cae7dfbe9b13d27f2786/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
[modify] https://crrev.com/351fb5afeafd16e28c65cae7dfbe9b13d27f2786/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
[modify] https://crrev.com/351fb5afeafd16e28c65cae7dfbe9b13d27f2786/chrome/browser/safe_browsing/threat_details.cc
[modify] https://crrev.com/351fb5afeafd16e28c65cae7dfbe9b13d27f2786/chrome/browser/safe_browsing/threat_details.h
[modify] https://crrev.com/351fb5afeafd16e28c65cae7dfbe9b13d27f2786/chrome/browser/safe_browsing/threat_details_cache.cc
[modify] https://crrev.com/351fb5afeafd16e28c65cae7dfbe9b13d27f2786/chrome/browser/safe_browsing/threat_details_cache.h
[modify] https://crrev.com/351fb5afeafd16e28c65cae7dfbe9b13d27f2786/chrome/browser/safe_browsing/threat_details_unittest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, May 3 2017

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

commit 42a7b7f4765a74b3a61bcf1940636f27f4a2a38d
Author: timvolodine <timvolodine@chromium.org>
Date: Wed May 03 17:15:15 2017

Componentize safe_browsing: move threat_details* to component.

Move threat_details* to the safe_browsing component and
update build files and dependencies.

BUG= 700351 , 688629

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

[modify] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/chrome/browser/BUILD.gn
[modify] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
[modify] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
[modify] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
[modify] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/chrome/browser/safe_browsing/threat_details_unittest.cc
[modify] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/chrome/browser/safe_browsing/ui_manager.cc
[add] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/components/safe_browsing/browser/BUILD.gn
[add] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/components/safe_browsing/browser/DEPS
[rename] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/components/safe_browsing/browser/threat_details.cc
[rename] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/components/safe_browsing/browser/threat_details.h
[rename] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/components/safe_browsing/browser/threat_details_cache.cc
[rename] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/components/safe_browsing/browser/threat_details_cache.h
[rename] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/components/safe_browsing/browser/threat_details_history.cc
[rename] https://crrev.com/42a7b7f4765a74b3a61bcf1940636f27f4a2a38d/components/safe_browsing/browser/threat_details_history.h

added further steps in the design doc section 2 (https://goo.gl/TfoY9K)

Project Member

Comment 16 by bugdroid1@chromium.org, May 25 2017

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

commit b00d167d8b2d7a51814f748585860ea510744a2c
Author: timvolodine <timvolodine@chromium.org>
Date: Thu May 25 13:23:37 2017

Componentize safe_browsing: factor out SafeBrowsingURLRequestContextGetter.

Factor out SafeBrowsingURLRequestContextGetter from safe_browsing_service
and move it as a separate class into components/safe_browsing/browser/.
This patch also ensures that it's possible to provide a custom "user data
directory" upon construction.

This refactoring allows for the SafeBrowsingURLRequestContextGetter and its
dedicated cookie store to be resused in the Android WebView implementation.

Design doc: https://goo.gl/TfoY9K, section 2.

BUG= 700351 , 688629
TBR=pauljensen@chromium.org

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

[modify] https://crrev.com/b00d167d8b2d7a51814f748585860ea510744a2c/chrome/browser/safe_browsing/safe_browsing_service.cc
[modify] https://crrev.com/b00d167d8b2d7a51814f748585860ea510744a2c/components/safe_browsing/browser/BUILD.gn
[modify] https://crrev.com/b00d167d8b2d7a51814f748585860ea510744a2c/components/safe_browsing/browser/DEPS
[add] https://crrev.com/b00d167d8b2d7a51814f748585860ea510744a2c/components/safe_browsing/browser/safe_browsing_url_request_context_getter.cc
[add] https://crrev.com/b00d167d8b2d7a51814f748585860ea510744a2c/components/safe_browsing/browser/safe_browsing_url_request_context_getter.h
[modify] https://crrev.com/b00d167d8b2d7a51814f748585860ea510744a2c/components/safe_browsing/common/safebrowsing_constants.cc
[modify] https://crrev.com/b00d167d8b2d7a51814f748585860ea510744a2c/components/safe_browsing/common/safebrowsing_constants.h

Blockedon: 731744
Blockedon: 731747
Blockedon: 732373
Project Member

Comment 20 by bugdroid1@chromium.org, Jun 14 2017

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

commit 2ef5d746246fd46bccc0035070da7a2988c1772f
Author: timvolodine <timvolodine@chromium.org>
Date: Wed Jun 14 19:41:26 2017

Plumbing for safe browsing reporting for Android WebView.

Provide the necessary hooks for the reporting to work in
Android WebView. In particular
- implement AwSafeBrowsingUIManager::SendSerializedThreatDetails,
- create ping manager and url request context getter on IO/UI threads,
- create trigger_manager and expose in AwBrowserContext,
- add/create DIR_SAFE_BROWSING for storing safe browsing cookies,
- provide Android WebView specific protocol config client name (crbug.com/732373),
- move the relevant constants from safebrowsing_service to component.

TODO (in a follow-up patch):
- invoke {Start,Finish}CollectingThreatDetails() from the AwSafeBrowsingBlockingPage ( crbug.com/731747 )

Design doc: https://goo.gl/TfoY9K, section 3.

BUG= 700351 , 688629, 732373

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

[modify] https://crrev.com/2ef5d746246fd46bccc0035070da7a2988c1772f/android_webview/BUILD.gn
[modify] https://crrev.com/2ef5d746246fd46bccc0035070da7a2988c1772f/android_webview/browser/aw_browser_context.cc
[modify] https://crrev.com/2ef5d746246fd46bccc0035070da7a2988c1772f/android_webview/browser/aw_browser_context.h
[modify] https://crrev.com/2ef5d746246fd46bccc0035070da7a2988c1772f/android_webview/browser/aw_browser_main_parts.cc
[modify] https://crrev.com/2ef5d746246fd46bccc0035070da7a2988c1772f/android_webview/browser/aw_safe_browsing_blocking_page.cc
[modify] https://crrev.com/2ef5d746246fd46bccc0035070da7a2988c1772f/android_webview/browser/aw_safe_browsing_ui_manager.cc
[modify] https://crrev.com/2ef5d746246fd46bccc0035070da7a2988c1772f/android_webview/browser/aw_safe_browsing_ui_manager.h
[modify] https://crrev.com/2ef5d746246fd46bccc0035070da7a2988c1772f/android_webview/common/aw_paths.cc
[modify] https://crrev.com/2ef5d746246fd46bccc0035070da7a2988c1772f/android_webview/common/aw_paths.h
[modify] https://crrev.com/2ef5d746246fd46bccc0035070da7a2988c1772f/chrome/browser/safe_browsing/safe_browsing_service.cc
[modify] https://crrev.com/2ef5d746246fd46bccc0035070da7a2988c1772f/components/safe_browsing/common/safebrowsing_constants.cc
[modify] https://crrev.com/2ef5d746246fd46bccc0035070da7a2988c1772f/components/safe_browsing/common/safebrowsing_constants.h

Project Member

Comment 21 by bugdroid1@chromium.org, Jun 30 2017

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

commit 840e30ceebc2e8c39657d144f2aea24db1124256
Author: timvolodine <timvolodine@chromium.org>
Date: Fri Jun 30 17:51:57 2017

Implement code for Start/Finish threat details collection in WebView.

Provide calls to start/finish threat details collection
in Android WebView. In particular:
- implement calls to {Start,Finish}CollectThreadDetails in
aw_safe_browsing_blocking_page and related code,
- add crbug reference regarding HistoryService,
- refactor common parts into base blocking page class.

BUG= 700351 , 688629,  731747 

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

[modify] https://crrev.com/840e30ceebc2e8c39657d144f2aea24db1124256/android_webview/browser/aw_safe_browsing_blocking_page.cc
[modify] https://crrev.com/840e30ceebc2e8c39657d144f2aea24db1124256/android_webview/browser/aw_safe_browsing_blocking_page.h
[modify] https://crrev.com/840e30ceebc2e8c39657d144f2aea24db1124256/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
[modify] https://crrev.com/840e30ceebc2e8c39657d144f2aea24db1124256/chrome/browser/safe_browsing/safe_browsing_blocking_page.h
[modify] https://crrev.com/840e30ceebc2e8c39657d144f2aea24db1124256/components/safe_browsing/base_blocking_page.cc
[modify] https://crrev.com/840e30ceebc2e8c39657d144f2aea24db1124256/components/safe_browsing/base_blocking_page.h

Project Member

Comment 22 by bugdroid1@chromium.org, Jul 11 2017

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

commit b27231d3217b7100e0a7e6759cbda079c83ac01d
Author: timvolodine <timvolodine@chromium.org>
Date: Tue Jul 11 11:55:14 2017

Always create safe browsing specific directory in WebView.

We need to create the safe browsing directory for storing
safe browsing related cookies. This should happen in any case
as the safe browsing functionality can be enabled on a
per webview basis at runtime.

BUG= 700351 , 688629

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

[modify] https://crrev.com/b27231d3217b7100e0a7e6759cbda079c83ac01d/android_webview/browser/aw_browser_main_parts.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Jul 14 2017

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

commit 192d555b973a2ee84d1417ca8a78fe695586e77f
Author: Tim Volodine <timvolodine@chromium.org>
Date: Fri Jul 14 16:34:19 2017

Use a PrefService and enable extended Safe Browsing reporting (by means of opt-in).

In this patch:
- Refactoring in order to use a PrefService for obtaining and updating
the preference values related to the interstitials.
- Use the pref service for setting the default values (for the display
options).
- Enable user opt-in (using a check box) for the extended reporting when
the interstitial is shown.
- Update AwSafeBrowsingUIManager::SetExtendedReportingAllowed to modify
the corresponding PrefService preference (instead of keeping a boolean
inside the class).

BUG= 700351 , 688629

Change-Id: Ifa2e2c692670213e792d05c5d3cb0ba7facc74f7
Reviewed-on: https://chromium-review.googlesource.com/567144
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Selim Gurun <sgurun@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Commit-Queue: Tim Volodine <timvolodine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486774}
[modify] https://crrev.com/192d555b973a2ee84d1417ca8a78fe695586e77f/android_webview/browser/aw_browser_context.cc
[modify] https://crrev.com/192d555b973a2ee84d1417ca8a78fe695586e77f/android_webview/browser/aw_safe_browsing_blocking_page.cc
[modify] https://crrev.com/192d555b973a2ee84d1417ca8a78fe695586e77f/android_webview/browser/aw_safe_browsing_blocking_page.h
[modify] https://crrev.com/192d555b973a2ee84d1417ca8a78fe695586e77f/android_webview/browser/aw_safe_browsing_resource_throttle.cc
[modify] https://crrev.com/192d555b973a2ee84d1417ca8a78fe695586e77f/android_webview/browser/aw_safe_browsing_ui_manager.cc
[modify] https://crrev.com/192d555b973a2ee84d1417ca8a78fe695586e77f/android_webview/browser/aw_safe_browsing_ui_manager.h
[modify] https://crrev.com/192d555b973a2ee84d1417ca8a78fe695586e77f/chrome/browser/safe_browsing/test_safe_browsing_blocking_page_quiet.cc
[modify] https://crrev.com/192d555b973a2ee84d1417ca8a78fe695586e77f/components/safe_browsing/base_blocking_page.cc
[modify] https://crrev.com/192d555b973a2ee84d1417ca8a78fe695586e77f/components/safe_browsing/base_blocking_page.h
[modify] https://crrev.com/192d555b973a2ee84d1417ca8a78fe695586e77f/components/security_interstitials/core/controller_client.cc

Labels: WebView-SafeBrowsing
Labels: Hotlist-EnamelAndFriendsFixIt
Labels: -Hotlist-EnamelAndFriendsFixIt
Cc: ntfschr@chromium.org
Tim, this issue is fixed, right?
Labels: WebView-triaged
Status: Verified (was: Started)
I think general componentization is done. Any further work should be tracked in separate bugs.
ack, thanks for updating this bug

Sign in to add a comment