WebView: carefully check size for SafeBrowsing interstitial

WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is invisible or unattached. Instead of
    an interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small/Medium/Giant interstitial - if the WebView is small, it may be used
    as a piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation. We intend to treat giant webviews
    the same way as either small or medium interstitials.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately.

BUG=702072
Review-Url: https://codereview.chromium.org/2833923002
Cr-Commit-Position: refs/heads/master@{#467119}
Committed: https://chromium.googlesource.com/chromium/src/+/22119b62178b195b177b6e5629624d80f52efcd1

[Nate Fischer, 2017-04-20 23:01:09]

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

[commit-bot: I haz the power, 2017-04-20 23:02:09]

Dry run: 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-20 23:04:01]

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

[commit-bot: I haz the power, 2017-04-20 23:04:02]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Nate Fischer, 2017-04-20 23:13:57]

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

[commit-bot: I haz the power, 2017-04-20 23:14:18]

Dry run: 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-20 23:39:03]

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

[commit-bot: I haz the power, 2017-04-20 23:39:04]

Dry run: This issue passed the CQ dry run.

[Nate Fischer, 2017-04-22 02:05:32]

Description was changed from

==========
WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is not visible, or it is larger than
    the screen, or parts of it extend off the device screen. Instead of
    an interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small interstitial - if the WebView is small, it may be used as a
    piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately.

BUG=702072
==========

to

==========
WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is not visible. Instead of an
    interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small/Giant interstitial - if the WebView is small, it may be used
    as a piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately.

BUG=702072
==========

[Nate Fischer, 2017-04-22 02:05:41]

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

[commit-bot: I haz the power, 2017-04-22 02:06:07]

Dry run: 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-22 02:30:11]

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

[commit-bot: I haz the power, 2017-04-22 02:30:12]

Dry run: This issue passed the CQ dry run.

[Nate Fischer, 2017-04-24 21:13:57]

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

[commit-bot: I haz the power, 2017-04-24 21:14:27]

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-24 21:42:35]

Description was changed from

==========
WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is not visible. Instead of an
    interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small/Giant interstitial - if the WebView is small, it may be used
    as a piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately.

BUG=702072
==========

to

==========
WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is not visible. Instead of an
    interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small/Giant interstitial - if the WebView is small, it may be used
    as a piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately. This leaves
CanShowInterstitial() exposed to native, since we'll eventually use it to
distinguish between 1 & 3.

BUG=702072
==========

[Nate Fischer, 2017-04-24 21:43:39]

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

[Nate Fischer, 2017-04-24 21:43:40]

PTAL

[commit-bot: I haz the power, 2017-04-24 21:46:23]

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

[commit-bot: I haz the power, 2017-04-24 21:46:24]

Dry run: This issue passed the CQ dry run.

[sgurun-gerrit only, 2017-04-24 22:44:11]

https://codereview.chromium.org/2833923002/diff/60001/android_webview/java/sr...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/2833923002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:2928: int
displayHeight = displayAndroid.getDisplayHeight();
nit: inline

https://codereview.chromium.org/2833923002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:2929: int
displayWidth = displayAndroid.getDisplayWidth();
nit:inline

https://codereview.chromium.org/2833923002/diff/60001/android_webview/native/...
File android_webview/native/aw_contents.cc (right):

https://codereview.chromium.org/2833923002/diff/60001/android_webview/native/...
android_webview/native/aw_contents.cc:1410: bool
AwContents::CanShowInterstitial() {
nobody seems to call this, what is your plan on this?

[Nate Fischer, 2017-04-24 23:00:53]

https://codereview.chromium.org/2833923002/diff/60001/android_webview/java/sr...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/2833923002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:2928: int
displayHeight = displayAndroid.getDisplayHeight();
On 2017/04/24 at 22:44:10, sgurun wrote:
> nit: inline

done

https://codereview.chromium.org/2833923002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContents.java:2929: int
displayWidth = displayAndroid.getDisplayWidth();
On 2017/04/24 at 22:44:10, sgurun wrote:
> nit:inline

done

https://codereview.chromium.org/2833923002/diff/60001/android_webview/native/...
File android_webview/native/aw_contents.cc (right):

https://codereview.chromium.org/2833923002/diff/60001/android_webview/native/...
android_webview/native/aw_contents.cc:1410: bool
AwContents::CanShowInterstitial() {
On 2017/04/24 at 22:44:10, sgurun wrote:
> nobody seems to call this, what is your plan on this?

Once we support a second interstitial, we should use this to distinguish between
when to use the big interstitial vs. small interstitial:

if (CanShowBigInterstitial()) {
  // Use the big interstitial
} else if (CanShowInterstitial()) {
  // Use the small interstitial
} else {
  // Straight to network error
}

As you pointed out, we can remove this code now, but it sounds like we would be
adding it back once we support this. Your call if we should remove it for now.

[Nate Fischer, 2017-04-24 23:41:41]

Description was changed from

==========
WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is not visible. Instead of an
    interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small/Giant interstitial - if the WebView is small, it may be used
    as a piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately. This leaves
CanShowInterstitial() exposed to native, since we'll eventually use it to
distinguish between 1 & 3.

BUG=702072
==========

to

==========
WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is not visible. Instead of an
    interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small/Medium/Giant interstitial - if the WebView is small, it may be used
    as a piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation. We intend to treat giant webviews
    the same way as either small or medium interstitials.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately. This leaves
CanShowInterstitial() exposed to native, since we'll eventually use it to
distinguish between 1 & 3.

BUG=702072
==========

[Nate Fischer, 2017-04-24 23:43:43]

As discussed offline, I removed the native parts of CanShowInterstitial, we'll
add it back if we need it. No logic change in the Java code.

[Nate Fischer, 2017-04-24 23:44:12]

Description was changed from

==========
WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is not visible. Instead of an
    interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small/Medium/Giant interstitial - if the WebView is small, it may be used
    as a piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation. We intend to treat giant webviews
    the same way as either small or medium interstitials.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately. This leaves
CanShowInterstitial() exposed to native, since we'll eventually use it to
distinguish between 1 & 3.

BUG=702072
==========

to

==========
WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is not visible. Instead of an
    interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small/Medium/Giant interstitial - if the WebView is small, it may be used
    as a piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation. We intend to treat giant webviews
    the same way as either small or medium interstitials.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately.

BUG=702072
==========

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

https://codereview.chromium.org/2833923002/diff/100001/android_webview/java/s...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/2833923002/diff/100001/android_webview/java/s...
android_webview/java/src/org/chromium/android_webview/AwContents.java:2941:
return mContainerView.getVisibility() == View.VISIBLE;
We have a local variable mIsViewVisible. Can you check if there is a difference
between using mIsViewVisible vs. using mContainerView.getVisibility in this
case.

https://codereview.chromium.org/2833923002/diff/100001/android_webview/java/s...
android_webview/java/src/org/chromium/android_webview/AwContents.java:2952:
protected boolean canShowBigInterstitial() {
nit: use @visiblefortesting
here and above

[Nate Fischer, 2017-04-25 02:11:29]

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

[Nate Fischer, 2017-04-25 02:11:43]

https://codereview.chromium.org/2833923002/diff/100001/android_webview/java/s...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/2833923002/diff/100001/android_webview/java/s...
android_webview/java/src/org/chromium/android_webview/AwContents.java:2941:
return mContainerView.getVisibility() == View.VISIBLE;
On 2017/04/25 at 00:04:38, sgurun wrote:
> We have a local variable mIsViewVisible. Can you check if there is a
difference between using mIsViewVisible vs. using mContainerView.getVisibility
in this case.

I see no difference. Switched to mIsViewVisible. Also, this doesn't check if
it's actually attached, so I'm also using mIsAttachedToWindow.

https://codereview.chromium.org/2833923002/diff/100001/android_webview/java/s...
android_webview/java/src/org/chromium/android_webview/AwContents.java:2952:
protected boolean canShowBigInterstitial() {
On 2017/04/25 at 00:04:38, sgurun wrote:
> nit: use @visiblefortesting
> here and above

done

[commit-bot: I haz the power, 2017-04-25 02:12:04]

Dry run: 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-25 02:37:09]

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

[commit-bot: I haz the power, 2017-04-25 02:37:09]

Dry run: This issue passed the CQ dry run.

[sgurun-gerrit only, 2017-04-25 19:50:01]

lgtm

[Nate Fischer, 2017-04-25 21:21:43]

Description was changed from

==========
WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is not visible. Instead of an
    interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small/Medium/Giant interstitial - if the WebView is small, it may be used
    as a piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation. We intend to treat giant webviews
    the same way as either small or medium interstitials.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately.

BUG=702072
==========

to

==========
WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is invisible or unattached. Instead of
    an interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small/Medium/Giant interstitial - if the WebView is small, it may be used
    as a piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation. We intend to treat giant webviews
    the same way as either small or medium interstitials.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately.

BUG=702072
==========

[Nate Fischer, 2017-04-25 21:22:05]

Thanks!

[Nate Fischer, 2017-04-25 21:22:19]

The CQ bit was checked by ntfschr@chromium.org

[commit-bot: I haz the power, 2017-04-25 21:23:14]

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-25 21:34:11]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1493155339911360,
"parent_rev": "ac414937946e8e50537698495b6eb8887875c453", "commit_rev":
"22119b62178b195b177b6e5629624d80f52efcd1"}

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

Description was changed from

==========
WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is invisible or unattached. Instead of
    an interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small/Medium/Giant interstitial - if the WebView is small, it may be used
    as a piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation. We intend to treat giant webviews
    the same way as either small or medium interstitials.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately.

BUG=702072
==========

to

==========
WebView: carefully check size for SafeBrowsing interstitial

In the case of a SafeBrowsing hit, we want 3 options for which warning
the user sees:

 1. No interstitial - the WebView is invisible or unattached. Instead of
    an interstitial, we immediately return a network error.
 2. Big interstitial - if the WebView is large enough that it's probably
    used for user navigation, then we should display the same
    interstitial Chrome uses (a scary warning page to catch the user's
    attention).
 3. Small/Medium/Giant interstitial - if the WebView is small, it may be used
    as a piece of a larger composed app page. In this case, we eventually
    want to show a less-scary interstitial, since the page is typically
    not the result of user navigation. We intend to treat giant webviews
    the same way as either small or medium interstitials.

We don't yet have a small interstitial, so we handle cases 1 & 3 the
same for now (create a network error). Once the new interstitial is
implemented, we'll handle cases 1 & 3 separately.

BUG=702072

Review-Url: https://codereview.chromium.org/2833923002
Cr-Commit-Position: refs/heads/master@{#467119}
Committed:
https://chromium.googlesource.com/chromium/src/+/22119b62178b195b177b6e562962...
==========

[commit-bot: I haz the power, 2017-04-25 21:34:23]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/22119b62178b195b177b6e562962...