New issue
Advanced search Search tips

Issue 737274 link

Starred by 1 user
Status: Verified
Owner:
ntfschr@chromium.org
Closed: Jun 2017
Cc:
ricea@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Filter UwS warnings using the threat type set

Project Member Reported by ntfschr@chromium.org, Jun 27 2017 
Follow up to issue 728732.

We still want to avoid UwS errors, but now that http://crrev/2938993002 is landed, we can tell GMS to ignore the errors, instead of explicitly ignoring them after-the-fact in AwSafeBrowsingResourceThrottle::OnCheckBrowseUrlResult() [1].

We should instead exlude UwS from the SBThreatTypeSet passed to BaseResourceThrottle [2]. This keeps code cleaner, and will un-block https://codereview.chromium.org/2876473003/.

This also requires changing the assumptions of MockSafeBrowsingApiHandler. This class needs to be changed to respect the threatTypes array, since the GMS dependency actually examines it.

[1] https://cs.chromium.org/chromium/src/android_webview/browser/aw_safe_browsing_resource_throttle.cc?q=f:android_webview+f:resource_throttle&sq=package:chromium&l=60
[2] https://cs.chromium.org/chromium/src/android_webview/browser/aw_safe_browsing_resource_throttle.cc?q=f:android_webview+f:resource_throttle&sq=package:chromium&l=47
Project Member

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

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

commit e20f0cd3de8dc862ab5bebdcf31715f48a28e69c
Author: Nate Fischer <ntfschr@chromium.org>
Date: Wed Jun 28 00:12:14 2017

AW: refactor how we ignore UwS threats

We currently ignore Unwanted Software (UwS) threats by explicitly
marking them as safe in OnCheckBrowseUrlResult(). We can instead ignore
them with the SBThreatTypeSet passed into BaseResourceThrottle.

This changes MockSafeBrowsingApiHandler to respect the value of
threatsOfInterest, since that value is explicitly changed by WebView.

Additionally, since we're temporarily blocking UwS errors, this adds an
explicit assert to check that UwS errors are always ignored by our code.
This also includes logic for UwS errors after the assert, in preparation
for when UwS errors are correctly handled in WebView.

Bug:  737274 
Change-Id: I828df39aa1634ee1a4aef55e725f0045692c2dcc
Reviewed-on: https://chromium-review.googlesource.com/550860
Reviewed-by: Bo Liu <boliu@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482808}
[modify] https://crrev.com/e20f0cd3de8dc862ab5bebdcf31715f48a28e69c/android_webview/browser/aw_safe_browsing_resource_throttle.cc
[modify] https://crrev.com/e20f0cd3de8dc862ab5bebdcf31715f48a28e69c/android_webview/browser/aw_safe_browsing_resource_throttle.h
[modify] https://crrev.com/e20f0cd3de8dc862ab5bebdcf31715f48a28e69c/android_webview/javatests/src/org/chromium/android_webview/test/SafeBrowsingTest.java

Comment 2 by ntfschr@chromium.org, Jun 28 2017

Status: Fixed (was: Started)
Verification steps: same as in issue 728732.

Comment 3 by ntfschr@chromium.org, Oct 11 2017

Labels: WebView-SafeBrowsing

Comment 4 by ntfschr@chromium.org, Aug 24

Status: Verified (was: Fixed)
Bulk edit: marking stale 'fixed' bugs as 'verified' since they don't need verification at this point.
►

Sign in to add a comment