New issue
Advanced search Search tips

Issue 733815 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 680790
issue 738192



Sign in to add a comment

Use JavaScript in WebView SafeBrowsing integration tests

Project Member Reported by ntfschr@chromium.org, Jun 15 2017

Issue description

Using JavaScript in WebView's Safe Browsing integration tests would have a couple big advantages:

 - Actually check page content, instead of only checking pixels
   - This lets us differentiate between quiet and loud interstitials
   - We can check for the appearance of buttons, like back-to-safety
 - Click UI buttons via JavaScript
   - This would let us properly test back-to-safety for malicious subresource (some logic sits above InterstitialPage::DontProceed)
   - This would verify that all internals are working for quiet interstitials

The issue right now is that our JavaScript helper functions are only interacting with the main page, not the interstitial page, so they see an entirely different DOM. I think the trick is to use something like content::ExecuteScriptAndGetValue() [1]. We can't currently use this method in aw_contents because the method is in a testonly dependency.

[1] https://cs.chromium.org/chromium/src/content/public/test/test_utils.cc?sq=package:chromium&l=188
 

Comment 1 by vakh@chromium.org, Jun 16 2017

Labels: SafeBrowsing-Triaged
Status: Started (was: Assigned)
I have a prototype where this works for some of the tests.

The trick is that we need to wait for the interstitial DOM to be fully loaded, which means waiting for `document.readState == 'complete'`.
Blocking: 738192
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 1 2017

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

commit 5fdb7e978ede05e7236a04b7295194d864e499f6
Author: Nate Fischer <ntfschr@chromium.org>
Date: Sat Jul 01 01:08:01 2017

AW: change SafeBrowsing tests to use JavaScript

No change in production logic.

This changes WebView Safe Browsing instrumentation tests to use
JavaScript on interstitial pages instead of calling underlying methods.
This lets us test more of the underlying logic, since we can verify the
interstitial HTML/JavaScript and the logic in SafeBrowsing*ErrorUI.

This adds EvaluateJavaScriptOnInterstitialForTesting(), based off
WebContentsAndroid::EvaluateJavaScriptForTests(), but using the
InterstitialPage's RenderFrameHost.

This replaces proceedThroughInterstitial() and
dontProceedThroughInterstitial() with clickVisitUnsafePage() and
clickBackToSafety(). Quiet interstitials use
clickVisitUnsafePageQuietInterstitial() because they use a #details-link
instead of #details-button.

This adds one test (DontProceedNavigatesBackForSubResource) and disables
another (DontProceedCausesNetworkErrorForSubresource). This is because
clicking back to safety has different behavior than calling
InterstitialPage::DontProceed(). "Back to safety" causes a backwards
navigation for malicious subresources, skipping our mechanism for the
network error (see crbug/737820).

Bug:  733815 , 737820
Test: run_webview_instrumentation_test_apk -f SafeBrowsingTest#*
Change-Id: I4829f3cc5d9863ddd2a1fc51050f583ac9758bcf
Reviewed-on: https://chromium-review.googlesource.com/557918
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483883}
[modify] https://crrev.com/5fdb7e978ede05e7236a04b7295194d864e499f6/android_webview/browser/aw_contents.cc
[modify] https://crrev.com/5fdb7e978ede05e7236a04b7295194d864e499f6/android_webview/browser/aw_contents.h
[modify] https://crrev.com/5fdb7e978ede05e7236a04b7295194d864e499f6/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/5fdb7e978ede05e7236a04b7295194d864e499f6/android_webview/javatests/src/org/chromium/android_webview/test/SafeBrowsingTest.java

Status: Fixed (was: Started)
Filed  issue 738192  to track adding additional tests.
Labels: WebView-SafeBrowsing
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