Commit 1d1cc79a authored by Nate Fischer's avatar Nate Fischer Committed by Commit Bot

AW: deflake Safe Browsing tests

No change to production logic.

This aims to deflake
SafeBrowsingTest#testSafeBrowsingDontProceedNavigatesBackFor*.

These tests previously depended on the interstitial to change the page
title to the string "Security error." This is unreliable, however, as
chromium translates this for non-English locales.

This CL removes the dependency on page title, and asserts that the
previous page is visible after clicking "back to safety."

I couldn't locally reproduce test failures except by changing device
locale, but I expect this method to be more robust than relying on page
title. As mentioned below, I tested this ~200 times without failure, so
I don't expect many flakes.

This also removes the @FlakyTest annotation previously added.

Bug: 822753
Test: run_webview_instrumentation_test_apk -f SafeBrowsingTest#testSafeBrowsingDontProceedNavigatesBackFor*
Change-Id: I91ded411514302c2c34fdb8e06f7694f58f7e526
Reviewed-on: https://chromium-review.googlesource.com/1013686Reviewed-by: default avatarBo <boliu@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551173}
parent c68dbc95
...@@ -46,7 +46,6 @@ import org.chromium.base.test.util.CallbackHelper; ...@@ -46,7 +46,6 @@ import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.CommandLineFlags; import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest; import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.FlakyTest;
import org.chromium.base.test.util.InMemorySharedPreferences; import org.chromium.base.test.util.InMemorySharedPreferences;
import org.chromium.components.safe_browsing.SafeBrowsingApiBridge; import org.chromium.components.safe_browsing.SafeBrowsingApiBridge;
import org.chromium.components.safe_browsing.SafeBrowsingApiHandler; import org.chromium.components.safe_browsing.SafeBrowsingApiHandler;
...@@ -119,8 +118,6 @@ public class SafeBrowsingTest { ...@@ -119,8 +118,6 @@ public class SafeBrowsingTest {
// A gray page with an iframe to MALWARE_HTML_PATH // A gray page with an iframe to MALWARE_HTML_PATH
private static final String IFRAME_HTML_PATH = RESOURCE_PATH + "/iframe.html"; private static final String IFRAME_HTML_PATH = RESOURCE_PATH + "/iframe.html";
private static final String INTERSTITIAL_PAGE_TITLE = "Security error";
// These URLs will be CTS-tested and should not be changed. // These URLs will be CTS-tested and should not be changed.
private static final String WEB_UI_MALWARE_URL = "chrome://safe-browsing/match?type=malware"; private static final String WEB_UI_MALWARE_URL = "chrome://safe-browsing/match?type=malware";
private static final String WEB_UI_PHISHING_URL = "chrome://safe-browsing/match?type=phishing"; private static final String WEB_UI_PHISHING_URL = "chrome://safe-browsing/match?type=phishing";
...@@ -193,21 +190,32 @@ public class SafeBrowsingTest { ...@@ -193,21 +190,32 @@ public class SafeBrowsingTest {
private static class TestAwWebContentsObserver extends AwWebContentsObserver { private static class TestAwWebContentsObserver extends AwWebContentsObserver {
private CallbackHelper mDidAttachInterstitialPageHelper; private CallbackHelper mDidAttachInterstitialPageHelper;
private CallbackHelper mDidDetachInterstitialPageHelper;
public TestAwWebContentsObserver(WebContents webContents, AwContents awContents, public TestAwWebContentsObserver(WebContents webContents, AwContents awContents,
TestAwContentsClient contentsClient) { TestAwContentsClient contentsClient) {
super(webContents, awContents, contentsClient); super(webContents, awContents, contentsClient);
mDidAttachInterstitialPageHelper = new CallbackHelper(); mDidAttachInterstitialPageHelper = new CallbackHelper();
mDidDetachInterstitialPageHelper = new CallbackHelper();
} }
public CallbackHelper getAttachedInterstitialPageHelper() { public CallbackHelper getAttachedInterstitialPageHelper() {
return mDidAttachInterstitialPageHelper; return mDidAttachInterstitialPageHelper;
} }
public CallbackHelper getDetachedInterstitialPageHelper() {
return mDidDetachInterstitialPageHelper;
}
@Override @Override
public void didAttachInterstitialPage() { public void didAttachInterstitialPage() {
mDidAttachInterstitialPageHelper.notifyCalled(); mDidAttachInterstitialPageHelper.notifyCalled();
} }
@Override
public void didDetachInterstitialPage() {
mDidDetachInterstitialPageHelper.notifyCalled();
}
} }
private static class MockAwContents extends AwContents { private static class MockAwContents extends AwContents {
...@@ -403,15 +411,6 @@ public class SafeBrowsingTest { ...@@ -403,15 +411,6 @@ public class SafeBrowsingTest {
evaluateJavaScriptOnInterstitialOnUiThread(script, null); evaluateJavaScriptOnInterstitialOnUiThread(script, null);
} }
private void waitForInterstitialToChangeTitle() {
CriteriaHelper.pollUiThread(new Criteria() {
@Override
public boolean isSatisfied() {
return INTERSTITIAL_PAGE_TITLE.equals(mAwContents.getTitle());
}
});
}
private void loadPathAndWaitForInterstitial(final String path) throws Exception { private void loadPathAndWaitForInterstitial(final String path) throws Exception {
int interstitialCount = int interstitialCount =
mWebContentsObserver.getAttachedInterstitialPageHelper().getCallCount(); mWebContentsObserver.getAttachedInterstitialPageHelper().getCallCount();
...@@ -638,40 +637,35 @@ public class SafeBrowsingTest { ...@@ -638,40 +637,35 @@ public class SafeBrowsingTest {
@Feature({"AndroidWebView"}) @Feature({"AndroidWebView"})
public void testSafeBrowsingDontProceedNavigatesBackForMainFrame() throws Throwable { public void testSafeBrowsingDontProceedNavigatesBackForMainFrame() throws Throwable {
loadGreenPage(); loadGreenPage();
final String originalTitle = mActivityTestRule.getTitleOnUiThread(mAwContents);
loadPathAndWaitForInterstitial(MALWARE_HTML_PATH); loadPathAndWaitForInterstitial(MALWARE_HTML_PATH);
waitForInterstitialToChangeTitle();
waitForInterstitialDomToLoad(); waitForInterstitialDomToLoad();
int interstitialCount =
mWebContentsObserver.getDetachedInterstitialPageHelper().getCallCount();
clickBackToSafety(); clickBackToSafety();
mWebContentsObserver.getDetachedInterstitialPageHelper().waitForCallback(interstitialCount);
// Make sure we navigate back to previous page mActivityTestRule.waitForVisualStateCallback(mAwContents);
CriteriaHelper.pollUiThread(new Criteria() { assertTargetPageNotShowing(MALWARE_PAGE_BACKGROUND_COLOR);
@Override assertGreenPageShowing();
public boolean isSatisfied() {
return originalTitle.equals(mAwContents.getTitle());
}
});
} }
@Test @Test
@SmallTest @SmallTest
@Feature({"AndroidWebView"}) @Feature({"AndroidWebView"})
@FlakyTest(message = "crbug/822753")
public void testSafeBrowsingDontProceedNavigatesBackForSubResource() throws Throwable { public void testSafeBrowsingDontProceedNavigatesBackForSubResource() throws Throwable {
loadGreenPage(); loadGreenPage();
final String originalTitle = mActivityTestRule.getTitleOnUiThread(mAwContents);
loadPathAndWaitForInterstitial(IFRAME_HTML_PATH); loadPathAndWaitForInterstitial(IFRAME_HTML_PATH);
waitForInterstitialToChangeTitle();
waitForInterstitialDomToLoad(); waitForInterstitialDomToLoad();
int interstitialCount =
mWebContentsObserver.getDetachedInterstitialPageHelper().getCallCount();
clickBackToSafety(); clickBackToSafety();
mWebContentsObserver.getDetachedInterstitialPageHelper().waitForCallback(interstitialCount);
// Make sure we navigate back to previous page mActivityTestRule.waitForVisualStateCallback(mAwContents);
CriteriaHelper.pollUiThread(new Criteria() { assertTargetPageNotShowing(IFRAME_EMBEDDER_BACKGROUND_COLOR);
@Override assertGreenPageShowing();
public boolean isSatisfied() {
return originalTitle.equals(mAwContents.getTitle());
}
});
} }
@Test @Test
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment