ReloadUrlWithAnchor test flaky on Android Site-Isolation bots |
|||||
Issue description
,
Dec 9 2016
creis@, I tried run this locally but I did not get the failure. ``` $ adb_content_shell_command_line --enable-site-per-process $ ninja -C out/Android content_browsertests_apk $ out/Android/bin/run_content_browsertests --gtest-filter="NavigationControllerBrowserTest.ReloadWithUrlAnchor" C 18.631s Main ******************************************************************************** C 18.631s Main Summary C 18.631s Main ******************************************************************************** C 18.631s Main [==========] 1 test ran. C 18.631s Main [ PASSED ] 1 test. C 18.631s Main ******************************************************************************** ```
,
Dec 9 2016
The correct flag to use is --site-per-process.
,
Dec 9 2016
creis@, I also got pass with --site-per-process ``` $ adb_content_shell_command_line --site-per-process Wrote flags to /data/local/tmp/content-shell-command-line 84B5T15A29018477 (angler-userdebug 6.0 MDB08Q 2462291 dev-keys): 'chrome --site-per-process' $ out/Android/bin/run_content_browsertests --gtest-filter="NavigationControllerBrowserTest.ReloadWithUrlAnchor" C 19.093s Main ******************************************************************************** C 19.093s Main Summary C 19.093s Main ******************************************************************************** C 19.093s Main [==========] 1 test ran. C 19.094s Main [ PASSED ] 1 test. C 19.094s Main ******************************************************************************** ```
,
Dec 9 2016
Do tests actually read the content_shell_command_line file or do flags have to be passed in via the test runner? I'd start by outputting in LOG whether that flag is on when the test runs to make sure you're actually running with it on.
,
Dec 9 2016
Comment 5: That's worth confirming. I'd also suggest running multiple iterations, since the test is flaky on the Site Isolation Android bot: https://build.chromium.org/p/chromium.fyi/builders/Site%20Isolation%20Android?numbuilds=200 Failing builds: 2299, 2278, 2269, 2266, 2263, 2261, etc. (Some of the other runs seem affected by issue 672382 , where tons of tests are failing. For example: 2297, 2296.)
,
Dec 9 2016
I wanted to share my attempt to repro the intermittent failures from the bot: $ git log --oneline | head -1 # Just trying to show at what revision I am building/testing. 465a1a9 Remove some keyboard cruft $ . build/android/envsetup.sh $ cat out/andr/args.gn target_os = "android" target_cpu = "x86" is_debug = false is_component_build = true symbol_level = 1 use_goma = true $ ninja -C out/andr -j 1500 -l 15 content_browsertests ... $ build/android/avd.py run --sdcard-size 4096M --partition-size 4096M --api-level 23 ... $ out/andr/bin/run_content_browsertests --test-arguments=--site-per-process --gtest-filter=*NavigationControllerBrowserTest.ReloadWithUrlAnchor* --gtest-repeat=99 The test surprisingly passed 100 times. I am not sure what hypothesis to form regarding the bot failures: 1. Maybe the test only fails intermittently when run on an ARM architecture (the emulator is x86)? 2. Maybe the bot has some other characteristics (slow?) that make the repro more likely? 3. Maybe state from other, previously run tests leaks and is needed for the repro (this seems unlikely given what I know about how browser tests run). ... Please note that I double-checked that --site-per-process flag is taking effect by 1. Adding CHECK(!SiteIsolationPolicy::UseDedicatedProcessesForAllSites()) to the NavigationControllerBrowserTest.ReloadWithUrlAnchor test 2. Verifying that the run_content_browsertests commandline above does pass --site-per-process (verified by observing a crash caused by the CHECK statement from above). 3. Verifying no crash without --test-arguments=--site-per-process 4. Removing the CHECK
,
Dec 14 2016
I can easily reproduce this issue now. We can use same gn args with buildbot: ``` ffmpeg_branding = "Chrome" is_component_build = false is_debug = false proprietary_codecs = true symbol_level = 1 target_cpu = "arm64" target_os = "android" use_goma = true ``` Then build the content shell apk. Open http://chaopeng.me/htmltest/reload-with-url-anchor.html#d2, then reload repeatedly. It goes to incorrect position sometime. Also I can not reproduce this issue on chrome apk.
,
Dec 15 2016
This is also failing on linux asan from time to time: https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinuxASan%20tester/builds/1771/steps/content_browsertests/logs/NavigationControllerBrowserTest.ReloadWithUrlAnchor
,
Dec 28 2016
Issue 677301 has been merged into this issue.
,
Dec 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d5c112c459321ac69ddf41d11ef3644ebfd275d0 commit d5c112c459321ac69ddf41d11ef3644ebfd275d0 Author: alexmos <alexmos@chromium.org> Date: Wed Dec 28 19:42:01 2016 Disable flaky test: NavigationControllerBrowserTest.ReloadWithUrlAnchor BUG= 672545 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2606963002 Cr-Commit-Position: refs/heads/master@{#440880} [modify] https://crrev.com/d5c112c459321ac69ddf41d11ef3644ebfd275d0/content/browser/frame_host/navigation_controller_impl_browsertest.cc
,
Jan 9 2017
This flaky is caused by |FrameLoader::restoreScrollPositionAndViewState| calls before |FrameLoader::processFragment|. And |FrameLoader::restoreScrollPositionAndViewState| sets |didRestoreFromHistory = true| to forbid |FrameLoader::processFragment|. We call |FrameLoader::checkCompleted| in |FrameLoader::finishedParsing| when reload that should early return by checking |!shouldComplete(m_frame->document()) == true|. But |Document::isDelayingLoadEvent| return false sometimes(need more investigation) that makes |shouldComplete(m_frame->document()) == true| then calls |FrameLoader::restoreScrollPositionAndViewState|. That makes |FrameLoader::restoreScrollPositionAndViewState| called before |FrameLoader::processFragment|.
,
Jan 13 2017
,
Feb 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1033be6d096d430b3d1fe6439550448b2374ea4c commit 1033be6d096d430b3d1fe6439550448b2374ea4c Author: chaopeng <chaopeng@chromium.org> Date: Thu Feb 16 22:08:58 2017 Add DidSaveScrollOrScaleState flag to prevent restoreScrollPositionAndViewState restore from default This flaky is caused by |FrameLoader::restoreScrollPositionAndViewState| called before |FrameLoader::processFragment| in |checkCompleted| and restore from default. So we add DidSaveScrollOrScaleState flag to prevent restoreScrollPositionAndViewState restore from default. BUG= 672545 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2628683003 Cr-Commit-Position: refs/heads/master@{#451111} [modify] https://crrev.com/1033be6d096d430b3d1fe6439550448b2374ea4c/content/browser/frame_host/navigation_controller_impl_browsertest.cc [modify] https://crrev.com/1033be6d096d430b3d1fe6439550448b2374ea4c/content/common/page_state_serialization.cc [modify] https://crrev.com/1033be6d096d430b3d1fe6439550448b2374ea4c/content/common/page_state_serialization.h [modify] https://crrev.com/1033be6d096d430b3d1fe6439550448b2374ea4c/content/common/page_state_serialization_unittest.cc [modify] https://crrev.com/1033be6d096d430b3d1fe6439550448b2374ea4c/content/renderer/history_serialization.cc [modify] https://crrev.com/1033be6d096d430b3d1fe6439550448b2374ea4c/content/test/data/navigation_controller/reload-with-url-anchor.html [add] https://crrev.com/1033be6d096d430b3d1fe6439550448b2374ea4c/content/test/data/page_state/serialized_v23.dat [modify] https://crrev.com/1033be6d096d430b3d1fe6439550448b2374ea4c/third_party/WebKit/Source/core/loader/FrameLoader.cpp [modify] https://crrev.com/1033be6d096d430b3d1fe6439550448b2374ea4c/third_party/WebKit/Source/core/loader/HistoryItem.cpp [modify] https://crrev.com/1033be6d096d430b3d1fe6439550448b2374ea4c/third_party/WebKit/Source/core/loader/HistoryItem.h [modify] https://crrev.com/1033be6d096d430b3d1fe6439550448b2374ea4c/third_party/WebKit/Source/web/WebHistoryItem.cpp [modify] https://crrev.com/1033be6d096d430b3d1fe6439550448b2374ea4c/third_party/WebKit/public/web/WebHistoryItem.h
,
Feb 17 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by creis@chromium.org
, Dec 8 2016