Skip to content

GitLab

T
tangled
Commit 98d81e33 authored by Colin Blundell's avatar Colin Blundell Committed by Commit Bot
Browse files
Options

[Android Autofill] Fix buggy test of autofill triggering on reload 

The test was expecting events in the wrong order: upon reload and click
on the form it was expecting to see a CANCEL event followed by a
VIEW_EXITED event, but the latter event is fired first as part of focus
moving off of the
current form, followed by the former event as part of starting the new
autofill session. I did not detect this problem in my local testing
because the CANCEL event is not sent (/listened for by the testing
infrastructure) on Android versions > O.

I verified locally that the test fails on an O emulator before the
change in this CL and succeeds after the change.

One thing that's not clear is how this test ever succeeded on the O
trybot, e.g. on this run:
https://ci.chromium.org/p/chromium/builders/ci/Android%20WebView%20O%20%28dbg%29/12151

This CL also changes the test to do the reload via
AwActivityTestRule#reloadSync() rather than
AwActivityTestRule#loadUrlSync() to reflect production behavior.
That change, however, does not impact the above logic.

Bug: 1142486
Change-Id: Id2e9d6dbb44615356b43603afb11a2b4de40467e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2502043Reviewed-by: default avatarMichael Bai <michaelbai@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822074}
parent be8cbff2
Show whitespace changes
Inline Side-by-side
Showing with 10 additions and 4 deletions
android_webview/javatests/src/org/chromium/android_webview/test/AwAutofillTest.java
View file @ 98d81e33
...@@ -47,7 +47,6 @@ import org.chromium.base.metrics.RecordHistogram; ...@@ -47,7 +47,6 @@ import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.task.PostTask; import org.chromium.base.task.PostTask;
import org.chromium.base.test.util.CallbackHelper; import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.DisableIf; import org.chromium.base.test.util.DisableIf;
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.FlakyTest;
import org.chromium.base.test.util.MetricsUtils; import org.chromium.base.test.util.MetricsUtils;
...@@ -993,7 +992,6 @@ public class AwAutofillTest { ...@@ -993,7 +992,6 @@ public class AwAutofillTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"AndroidWebView"}) @Feature({"AndroidWebView"})
@DisabledTest(message = "https://crbug.com/1142486")
public void testAutofillTriggersAfterReload() throws Throwable { public void testAutofillTriggersAfterReload() throws Throwable {
final String data = "<html><head></head><body><form action='a.html' name='formname'>" final String data = "<html><head></head><body><form action='a.html' name='formname'>"
+ "<input type='text' id='text1' name='username'" + "<input type='text' id='text1' name='username'"
...@@ -1012,12 +1010,12 @@ public class AwAutofillTest { ...@@ -1012,12 +1010,12 @@ public class AwAutofillTest {
// Reload the page and check that the user clicking on the same form field ends the current // Reload the page and check that the user clicking on the same form field ends the current
// autofill session and starts a new session. // autofill session and starts a new session.
loadUrlSync(url); reloadSync();
DOMUtils.waitForNonZeroNodeBounds(mAwContents.getWebContents(), "text1"); DOMUtils.waitForNonZeroNodeBounds(mAwContents.getWebContents(), "text1");
// TODO(changwan): mock out IME interaction. // TODO(changwan): mock out IME interaction.
Assert.assertTrue(DOMUtils.clickNode(mTestContainerView.getWebContents(), "text1")); Assert.assertTrue(DOMUtils.clickNode(mTestContainerView.getWebContents(), "text1"));
cnt += waitForCallbackAndVerifyTypes(cnt, cnt += waitForCallbackAndVerifyTypes(cnt,
new Integer[] {AUTOFILL_CANCEL, AUTOFILL_VIEW_EXITED, AUTOFILL_VIEW_ENTERED, new Integer[] {AUTOFILL_VIEW_EXITED, AUTOFILL_CANCEL, AUTOFILL_VIEW_ENTERED,
AUTOFILL_SESSION_STARTED}); AUTOFILL_SESSION_STARTED});
} }
...@@ -2091,6 +2089,14 @@ public class AwAutofillTest { ...@@ -2091,6 +2089,14 @@ public class AwAutofillTest {
done.waitForCallback(callCount); done.waitForCallback(callCount);
} }
private void reloadSync() throws Exception {
CallbackHelper done = mContentsClient.getOnPageCommitVisibleHelper();
int callCount = done.getCallCount();
mRule.reloadSync(
mTestContainerView.getAwContents(), mContentsClient.getOnPageFinishedHelper());
done.waitForCallback(callCount);
}
private String executeJavaScriptAndWaitForResult(String code) throws Throwable { private String executeJavaScriptAndWaitForResult(String code) throws Throwable {
return mRule.executeJavaScriptAndWaitForResult( return mRule.executeJavaScriptAndWaitForResult(
mTestContainerView.getAwContents(), mContentsClient, code); mTestContainerView.getAwContents(), mContentsClient, code);
......
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