ChromeActivityTestRule.loadUrl flaky and does not verify the passed in URL was loaded before returning |
|||||
Issue description
ChromeActivityTestRule.loadUrl (or possibly the underlying Tab.loadUrl) seems to flakily report load finish too early when in Incognito mode. This was discovered in VrBrowserNavigationTest#testIncognitoMaintainsSeparateHistoryStack, but is reproducible in 2D Chrome by repeating the following steps many times in an Incognito tab:
ChromeActivityTestRule.loadUrl(url1)
ChromeActivityTestRule.loadUrl(url2)
Tab.goBack()
ChromeTabUtils.waitForTabPageLoaded(url1)
Tab.goForward()
ChromeTabUtils.waitForTabPageLoaded(url2)
ChromeActivityTestRule.loadUrl("chrome-native://newtab")
What happens is that the first waitForTabPageLoaded fails due to loading "chrome-native://newtab" instead of url1. This does not happen outside of Incognito mode, nor does it seem to happen if we add additional waitForTabPageLoaded calls after loadUrl.
Based on that, it seems like loadUrl is occasionally reporting load finish too early, causing goBack() to be run while there's still only one page in the history stack, which causes it to go back to the new tab page instead of the first loaded URL.
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7f492a092a3e64801c8775f50213b67a9f6c5794 commit 7f492a092a3e64801c8775f50213b67a9f6c5794 Author: bsheedy <bsheedy@chromium.org> Date: Fri Jul 27 21:04:12 2018 Add VR workaround for Incognito loading flakiness Adds a couple of extra waitForTabPageLoaded to VrBrowserNavigationTest#testIncognitoMaintainsSeparateHistoryStack as a workaround for Incognito tabs reporting page load too quickly, which was causing flakiness. This can be removed once the root cause is fixed. TBR=asimjour@chromium.org Bug: 868506 Change-Id: Ie91a8213e641d9f51dc353d5215e8ac0695db059 Reviewed-on: https://chromium-review.googlesource.com/1153614 Reviewed-by: Brian Sheedy <bsheedy@chromium.org> Commit-Queue: Brian Sheedy <bsheedy@chromium.org> Cr-Commit-Position: refs/heads/master@{#578785} [modify] https://crrev.com/7f492a092a3e64801c8775f50213b67a9f6c5794/chrome/android/javatests/src/org/chromium/chrome/browser/vr/VrBrowserNavigationTest.java
,
Jul 30
ChromeActivityTestRule.loadUrl triggers loadUrlInTab, which is not resilient. It waits for a load signal to happen before returning, but it doesn't verify the load signal is for the URL you requested. Calling ChromeTabUtils.waitForTabPageLoaded(...) after each loadUrl is the only safe thing to do. I recall trying to change the usage many, many years ago, but too many tests relied on redirects that made the final URL not match the one passed in and thus couldn't land. We likely should add new methods/deprecate these to have methods that reliably ensure that the URL passed in is the one that was loaded (or maybe it should cancel the pending loads if present, wait for the cancel to be committed, and then submit the new request). But yeah, it's flaky and needs to be updated/removed to fix this.
,
Jul 30
Updating the title to be more descriptive and adding the test label.
,
Jul 30
,
Aug 29
,
Nov 20
I believe this should have been fixed with https://chromium-review.googlesource.com/c/chromium/src/+/1318893. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bsheedy@chromium.org
, Jul 27