WebView: add {set,get}SafeBrowsingEnabled per WebView

WebView: add {set,get}SafeBrowsingEnabled per WebView

Add APIs to ContentSettingsAdapter to set/get SafeBrowsingEnabled, per
WebView. When checking getSafeBrowsingEnabled, if SafeBrowsing hasn't
been configured per WebView, default back to
AwContentsStatics.getSafeBrowsingEnabled() (which corresponds to the
manifest flag value).

This change removes most checks to
AwContentsStatics.getSafeBrowsingEnabled()--we must initialize static
SafeBrowsing pieces since SafeBrowsing may be enabled per webview. If
SafeBrowsing is disabled, we will skip actual URL checks by not
adding the resource throttle.

BUG=706631
Review-Url: https://codereview.chromium.org/2814593002
Cr-Commit-Position: refs/heads/master@{#463494}
Committed: https://chromium.googlesource.com/chromium/src/+/385c5d07e57f7ae48f7b6f4eedb2b9850b75e47a

[Nate Fischer, 2017-04-10 22:02:50]

ntfschr@chromium.org changed reviewers:
+ sgurun@chromium.org

[Nate Fischer, 2017-04-10 22:03:05]

The CQ bit was checked by ntfschr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-10 22:04:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nate Fischer, 2017-04-10 22:08:35]

Description was changed from

==========
WebView: add {set,get}SafeBrowsingEnabled per WebView

Add APIs to ContentSettingsAdapter to set/get SafeBrowsingEnabled, per
WebView. When checking getSafeBrowsingEnabled, if SafeBrowsing hasn't
been configured per WebView, default back to
AwContentsStatics.getSafeBrowsingEnabled() (which corresponds to the
manifest flag value).

This change means we must always initialize SafeBrowsing, even if
disabled (since it may be enabled later). If SafeBrowsing is disabled,
we will skip actual URL checks by not adding the resource throttle.

BUG=706631
==========

to

==========
WebView: add {set,get}SafeBrowsingEnabled per WebView

Add APIs to ContentSettingsAdapter to set/get SafeBrowsingEnabled, per
WebView. When checking getSafeBrowsingEnabled, if SafeBrowsing hasn't
been configured per WebView, default back to
AwContentsStatics.getSafeBrowsingEnabled() (which corresponds to the
manifest flag value).

This change removes most checks to
AwContentsStatics.getSafeBrowsingEnabled()--we must initialize static
SafeBrowsing pieces since SafeBrowsing may be enabled per webview. If
SafeBrowsing is disabled, we will skip actual URL checks by not
adding the resource throttle.

BUG=706631
==========

[Nate Fischer, 2017-04-10 22:25:27]

PTAL

[commit-bot: I haz the power, 2017-04-10 22:34:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-10 22:34:42]

Dry run: This issue passed the CQ dry run.

[sgurun-gerrit only, 2017-04-11 00:38:52]

https://codereview.chromium.org/2814593002/diff/20001/android_webview/apk/jav...
File android_webview/apk/java/proguard.flags (right):

https://codereview.chromium.org/2814593002/diff/20001/android_webview/apk/jav...
android_webview/apk/java/proguard.flags:19: # TODO(ntfschr): remove this once
Android O is released (crbug/706631)
I don't think you need to remove it. Just like the ones below, we can leave it.

https://codereview.chromium.org/2814593002/diff/20001/android_webview/java/sr...
File
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerController.java
(right):

https://codereview.chromium.org/2814593002/diff/20001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerController.java:88:
return false;
return the value that is dictated by the manifest.

[Nate Fischer, 2017-04-11 00:58:02]

PTAL

https://codereview.chromium.org/2814593002/diff/20001/android_webview/apk/jav...
File android_webview/apk/java/proguard.flags (right):

https://codereview.chromium.org/2814593002/diff/20001/android_webview/apk/jav...
android_webview/apk/java/proguard.flags:19: # TODO(ntfschr): remove this once
Android O is released (crbug/706631)
On 2017/04/11 at 00:38:52, sgurun wrote:
> I don't think you need to remove it. Just like the ones below, we can leave
it.

Done

https://codereview.chromium.org/2814593002/diff/20001/android_webview/java/sr...
File
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerController.java
(right):

https://codereview.chromium.org/2814593002/diff/20001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerController.java:88:
return false;
On 2017/04/11 at 00:38:52, sgurun wrote:
> return the value that is dictated by the manifest.

done

[Nate Fischer, 2017-04-11 00:58:07]

The CQ bit was checked by ntfschr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-11 00:58:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sgurun-gerrit only, 2017-04-11 01:06:57]

lgtm with one nit

https://codereview.chromium.org/2814593002/diff/80001/android_webview/java/sr...
File
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerController.java
(right):

https://codereview.chromium.org/2814593002/diff/80001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerController.java:86:
public boolean getSafeBrowsingEnabled() {
this is called on IO thread. Please also add comments to Awcontentsstatics
method that it can be called from multiple threads. the native method for
AwSafebrowsingconfighelper already supports it anyway.

[Nate Fischer, 2017-04-11 01:12:41]

https://codereview.chromium.org/2814593002/diff/80001/android_webview/java/sr...
File
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerController.java
(right):

https://codereview.chromium.org/2814593002/diff/80001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerController.java:86:
public boolean getSafeBrowsingEnabled() {
On 2017/04/11 at 01:06:56, sgurun wrote:
> this is called on IO thread. Please also add comments to Awcontentsstatics
method that it can be called from multiple threads. the native method for
AwSafebrowsingconfighelper already supports it anyway.

done

[Nate Fischer, 2017-04-11 01:12:43]

The CQ bit was checked by ntfschr@chromium.org

[Nate Fischer, 2017-04-11 01:12:43]

The patchset sent to the CQ was uploaded after l-g-t-m from sgurun@chromium.org
Link to the patchset: https://codereview.chromium.org/2814593002/#ps100001
(title: "Add comment about getSafeBrowsingEnabled()")

[commit-bot: I haz the power, 2017-04-11 01:13:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-11 01:53:03]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1491873163174120,
"parent_rev": "54a3dd064413a8d341253c27cb944ba739acba2c", "commit_rev":
"385c5d07e57f7ae48f7b6f4eedb2b9850b75e47a"}

[commit-bot: I haz the power, 2017-04-11 01:58:09]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1491873163174120,
"parent_rev": "54a3dd064413a8d341253c27cb944ba739acba2c", "commit_rev":
"385c5d07e57f7ae48f7b6f4eedb2b9850b75e47a"}

[commit-bot: I haz the power, 2017-04-11 01:58:12]

Description was changed from

==========
WebView: add {set,get}SafeBrowsingEnabled per WebView

Add APIs to ContentSettingsAdapter to set/get SafeBrowsingEnabled, per
WebView. When checking getSafeBrowsingEnabled, if SafeBrowsing hasn't
been configured per WebView, default back to
AwContentsStatics.getSafeBrowsingEnabled() (which corresponds to the
manifest flag value).

This change removes most checks to
AwContentsStatics.getSafeBrowsingEnabled()--we must initialize static
SafeBrowsing pieces since SafeBrowsing may be enabled per webview. If
SafeBrowsing is disabled, we will skip actual URL checks by not
adding the resource throttle.

BUG=706631
==========

to

==========
WebView: add {set,get}SafeBrowsingEnabled per WebView

Add APIs to ContentSettingsAdapter to set/get SafeBrowsingEnabled, per
WebView. When checking getSafeBrowsingEnabled, if SafeBrowsing hasn't
been configured per WebView, default back to
AwContentsStatics.getSafeBrowsingEnabled() (which corresponds to the
manifest flag value).

This change removes most checks to
AwContentsStatics.getSafeBrowsingEnabled()--we must initialize static
SafeBrowsing pieces since SafeBrowsing may be enabled per webview. If
SafeBrowsing is disabled, we will skip actual URL checks by not
adding the resource throttle.

BUG=706631

Review-Url: https://codereview.chromium.org/2814593002
Cr-Commit-Position: refs/heads/master@{#463494}
Committed:
https://chromium.googlesource.com/chromium/src/+/385c5d07e57f7ae48f7b6f4eedb2...
==========

[commit-bot: I haz the power, 2017-04-11 01:58:13]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/385c5d07e57f7ae48f7b6f4eedb2...