org.chromium.chrome.browser.externalnav.UrlOverridingTest#testOpenWindowFromUserGesture fails consistently on tablets |
||||
Issue descriptionhttp://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=chrome_public_test_apk&tests=org.chromium.chrome.browser.externalnav.UrlOverridingTest%23testOpenWindowFromUserGesture failure mode appears to be: junit.framework.AssertionFailedError: Criteria not met in allotted time. at org.chromium.content.browser.test.util.CriteriaHelper.pollInstrumentationThread(CriteriaHelper.java:74) at org.chromium.content.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:112) at org.chromium.content.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:133) at org.chromium.chrome.browser.externalnav.UrlOverridingTest.loadUrlAndWaitForIntentUrl(UrlOverridingTest.java:197) at org.chromium.chrome.browser.externalnav.UrlOverridingTest.testOpenWindowFromUserGesture(UrlOverridingTest.java:322) at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214) at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199) at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:192) at org.chromium.chrome.test.ChromeActivityTestCaseBase.runTest(ChromeActivityTestCaseBase.java:726) at org.chromium.base.test.BaseTestResult.runParameterized(BaseTestResult.java:161) at org.chromium.base.test.BaseTestResult.run(BaseTestResult.java:124) at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191) at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176) at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555) at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1853) https://chromium.googlesource.com/chromium/src/+/318ecffe833701cfd6db2ac43491ace0a68e18af appears to be the most likely culprit from the blamelist. bmcquade, can you take a look?
,
Dec 8 2016
will revert
,
Dec 8 2016
friendly ping :)
,
Dec 8 2016
as noted yesterday, i'm working on a revert: https://codereview.chromium.org/2557233002 the revert did not apply cleanly so i'm having it re-reviewed before committing.
,
Dec 12 2016
Hopefully the revert can be applied soon. The test has been broken for over a week now.
,
Dec 12 2016
Sure, should land today. For future reference, how should developers find these test failures before landing? I don't recall seeing these tests run as part of the try jobs before submitting my original patch. Any reason these tests don't run with the tryjobs by default? If these had been included in the try jobs, this patch never would have landed, you'd save time pinging this bug, and I'd not have had to spend my time landing a revert.
,
Dec 12 2016
This test doesn't run on the CQ because it's restricted to tablets (https://codesearch.chromium.org/chromium/src/chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/UrlOverridingTest.java?rcl=0&l=320) and we don't have sufficient hardware to support tablets on the CQ.
,
Dec 12 2016
I'm sorry, some tests are now failing consistently in the rollback patch, so this will need to wait another day as I debug. Will try to land this tomorrow.
,
Dec 12 2016
This didn't land today, but bmcquade@ is still working on a fix. The a revert is apparently causing issues with other tests. He is hoping to get the fix landed 2016-12-13 (Tuesday).
,
Dec 13 2016
For the record, I did a git bisect, and it is quite clear that this is the culprit (if that ever was a question :-): ### 318ecffe833701cfd6db2ac43491ace0a68e18af is the first bad commit commit 318ecffe833701cfd6db2ac43491ace0a68e18af Author: bmcquade <bmcquade@chromium.org> Date: Wed Nov 30 10:44:25 2016 -0800 Set user_gesture bit at NavigationHandle creation time. Previously, the user_gesture bit was set at different places and could be updated during the lifetime of a navigation, such as in WillStartNavigation and at commit time. In practice, whether a navigation was initiated by user gesture is known at the time a NavigationHandle is created. This change makes has_user_gesture_ a const member of NavigationHandleImpl, and sets it once at construction time. clamy suggested that we could set the user gesture bit at NavHandle construction time in the following places: - PlzNavigate: we have it when we create the handle. - Regular navs: we add the parameter to DidStartProvisionalLoad. - same-page navs: we create the NavigationHandle in DidCommitProvisionalLoad, where we have the information. This patch makes the following changes: - adds a gesture param to NavigationHandleImpl::Create - adds a gesture boolean param to FrameHostMsg_DidStartProvisionalLoad, which is passed to RenderFrameHostImpl::OnDidStartProvisionalLoad - removes the has_user_gesture boolean param from NavigationHandleImpl::WillStartRequest For the time being, we continue to update the gesture_ value at commit time. Once crbug.com/667572 is addressed, gesture_ can be made a const member and we can stop updating it at commit time. BUG= 665952 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2499313003 Cr-Commit-Position: refs/heads/master@{#435354} ### For future reference, this is what I did: Setup: $ git bisect start $ git bisect bad # HEAD was at a28b757f7cb3cd93b0be18e7b3de5eaede46d41a $ git bisect good 448b3411da25817b84202bff7c37bbc816c4b2b2 Then iteratively: Run test: $ gclient sync -r src@HEAD && time CHROMIUM_OUT_DIR="out-sheriff" ninja -j10000 -l30 -C out-sheriff/Debug chrome_public_test_apk && ./out-sheriff/Debug/bin/run_chrome_public_test_apk -vv -f "org.chromium.chrome.browser.externalnav.UrlOverridingTest#testOpenWindowFromUserGesture" Mark with 'git bisect good' or 'git bisect bad'. My device cable was flaky, so I couldn't easily run the automated 'git bisect run'.
,
Dec 13 2016
The revert has landed, so the test should go green soon. Apologies for the delay here. Thanks Tommy for the instructions on how to bisect, and for investigating the cause here. I'll use those instructions next time around. |
||||
►
Sign in to add a comment |
||||
Comment 1 by yolandyan@chromium.org
, Dec 8 2016