Issue metadata
Sign in to add a comment
|
NavigationControllerBrowserTest.RaceCrossOriginNavigationAndSameDocumentHistoryNavigation in content_browsertests failing on multiple builders |
||||||||||||||||||||||
Issue descriptionFiled by sheriff-o-matic@appspot.gserviceaccount.com on behalf of gsennton@google.com NavigationControllerBrowserTest.RaceCrossOriginNavigationAndSameDocumentHistoryNavigation in content_browsertests failing on multiple builders Builders failed on: - KitKat Tablet Tester: https://build.chromium.org/p/chromium.android/builders/KitKat%20Tablet%20Tester - Marshmallow 64 bit Tester: https://build.chromium.org/p/chromium.android/builders/Marshmallow%2064%20bit%20Tester First failing build on Marshmallow tester: https://uberchromegw.corp.google.com/i/chromium.android/builders/Marshmallow%2064%20bit%20Tester/builds/19377
,
Feb 6 2018
Oh nvm, Clamy@'s CL touches the actual test, assigning to clamy@
,
Feb 6 2018
Yes that's likely related. I will disable the test on Android while I investigate.
,
Feb 7 2018
This is P1 if not P0; it is causing many CLs to be rejected from the CQ or retried due the high failure rate. Only one example: https://ci.chromium.org/buildbot/tryserver.chromium.android/android_n5x_swarming_rel/356477 I think the test should be turned off by the sheriffs until it's stabilized again.
,
Feb 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/15cad144a25c5f8593c17b1cfe38e91ec315a397 commit 15cad144a25c5f8593c17b1cfe38e91ec315a397 Author: clamy <clamy@chromium.org> Date: Wed Feb 07 10:56:58 2018 Disable one NavigationController test on Android This CL disables NavigationControllerBrowserTest.RaceCrossOriginNavigationAndSameDocumentHistoryNavigation on Android while we investigate why it is flaky on this platform. BUG= 809488 Change-Id: Ib7cdc2fce5c817e466123a4e476c378b119a29ed Reviewed-on: https://chromium-review.googlesource.com/904663 Reviewed-by: Gustav Sennton <gsennton@chromium.org> Commit-Queue: Camille Lamy <clamy@chromium.org> Cr-Commit-Position: refs/heads/master@{#534979} [modify] https://crrev.com/15cad144a25c5f8593c17b1cfe38e91ec315a397/content/browser/frame_host/navigation_controller_impl_browsertest.cc
,
Feb 7 2018
,
Feb 7 2018
So it seems that half of the time, we don't get the fallback for the same-document navigations to trigger. I'm investigating why.
,
Feb 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e3c894eb0b22d2a85e7e46f6f24cb80961c8245d commit e3c894eb0b22d2a85e7e46f6f24cb80961c8245d Author: Andrey Kosyakov <caseq@chromium.org> Date: Wed Feb 07 22:39:38 2018 Disable NavigationControllerBrowserTest.RaceCrossOriginNavigationAndSameDocumentHistoryNavigation It's a flaky timeout on most platforms. TBR: clamy NOTRY: true Bug: 809488 Change-Id: Idfa6896f55622c5b79b7ffc81fe008241991daa3 Reviewed-on: https://chromium-review.googlesource.com/907646 Reviewed-by: Andrey Kosyakov <caseq@chromium.org> Commit-Queue: Andrey Kosyakov <caseq@chromium.org> Cr-Commit-Position: refs/heads/master@{#535167} [modify] https://crrev.com/e3c894eb0b22d2a85e7e46f6f24cb80961c8245d/content/browser/frame_host/navigation_controller_impl_browsertest.cc
,
Feb 8 2018
So looking at the test, I think we do have a race condition that leads to the deletion of the same-document NavigationRequest, preventing the fallback to take effect. I think this is a real issue, and will work on a CL to address it. Basically, we should be storing NavigationRequests for cross-document and same-document separately, and not delete them when one another starts or commits. This matches what happens in the renderer process.
,
Feb 22 2018
,
Feb 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fc3850d936fa5922f720404cc8611462f8b32a1d commit fc3850d936fa5922f720404cc8611462f8b32a1d Author: clamy <clamy@chromium.org> Date: Fri Feb 23 14:35:28 2018 Fix for same-document navigation fallback This CL fixes an issue where the fallback to cross-document navigation for a same-document navigation racing with a commit didn't work. The commit would delete the same-document NavigationRequest, preventing it from being restarted cross-document. In this CL, we store cross-document and same-document NavigationRequests separately, as they do not cancel each other. In particular, this means that a cross-document navigation commit will not erase a same-document NavigationRequest and vice-versa. This more accurately matches what happens in Blink's FrameLoader. BUG= 809488 Change-Id: I4ff91e938f18d6e6bed20e0a05a926569e970924 Reviewed-on: https://chromium-review.googlesource.com/925423 Commit-Queue: Camille Lamy <clamy@chromium.org> Reviewed-by: Nasko Oskov <nasko@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Cr-Commit-Position: refs/heads/master@{#538777} [modify] https://crrev.com/fc3850d936fa5922f720404cc8611462f8b32a1d/content/browser/frame_host/navigation_controller_impl_browsertest.cc [modify] https://crrev.com/fc3850d936fa5922f720404cc8611462f8b32a1d/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/fc3850d936fa5922f720404cc8611462f8b32a1d/content/browser/frame_host/render_frame_host_impl.h [modify] https://crrev.com/fc3850d936fa5922f720404cc8611462f8b32a1d/content/browser/frame_host/render_frame_host_manager.cc [modify] https://crrev.com/fc3850d936fa5922f720404cc8611462f8b32a1d/content/browser/web_contents/web_contents_impl.cc [modify] https://crrev.com/fc3850d936fa5922f720404cc8611462f8b32a1d/content/public/test/navigation_simulator.cc
,
Feb 23 2018
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by gsennton@chromium.org
, Feb 6 2018