NavigationEntry can be incorrectly replaced due to a non-matching pending entry |
||
Issue descriptionVersion: 51.0.2671.0 OS: All What steps will reproduce the problem? (1) Visit http://www.chromium.org/developers/design-documents/site-isolation. (2) In DevTools, open the "Network Conditions" panel and add a Custom preset with a 5000ms latency, so that the next navigation won't commit for 5 seconds. (3) In the DevTools Console, enter: location.replace("https://chrome.google.com/webstore"); This will try to start a cross-process navigation but it will stall for 5 seconds. (4) Before that commits, click an in-page fragment link on the Table of Contents (e.g., "5. Project Tasks"). What is the expected output? What do you see instead? Going back should take you to the same page without the fragment. Instead, the fragment navigation replaced the previous entry and it no longer exists. This is because we rely on the pending entry to know whether to replace the entry at commit time, but the pending entry doesn't match the committing navigation in NavigationControllerImpl::RendererDidNavigate in this case. We can fix this using the new pending_nav_entry_id() on NavigationHandleImpl. (Note that this was likely the cause of the crashes in issue 585499 from r374223. Fixing it should let us re-land that location.replace CL.)
,
Mar 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b4dc9337cacb2152671ea1bcfbcbd236d6fbff20 commit b4dc9337cacb2152671ea1bcfbcbd236d6fbff20 Author: creis <creis@chromium.org> Date: Mon Mar 14 21:39:19 2016 Ensure the NavigationHandle's nav entry ID is updated during transfers. The pending_nav_entry_id is now correct after transfers, and the handle is always present at commit (thanks to fixes to unit tests). This is part of a fix for matching the pending entry with the handle during navigations with replacement. BUG= 593153 TEST=See bug for repro steps. CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation Review URL: https://codereview.chromium.org/1777903003 Cr-Commit-Position: refs/heads/master@{#381081} [modify] https://crrev.com/b4dc9337cacb2152671ea1bcfbcbd236d6fbff20/content/browser/cross_site_transfer_browsertest.cc [modify] https://crrev.com/b4dc9337cacb2152671ea1bcfbcbd236d6fbff20/content/browser/frame_host/navigation_controller_impl.cc [modify] https://crrev.com/b4dc9337cacb2152671ea1bcfbcbd236d6fbff20/content/browser/frame_host/navigation_controller_impl.h [modify] https://crrev.com/b4dc9337cacb2152671ea1bcfbcbd236d6fbff20/content/browser/frame_host/navigation_controller_impl_unittest.cc [modify] https://crrev.com/b4dc9337cacb2152671ea1bcfbcbd236d6fbff20/content/browser/frame_host/navigation_handle_impl.h [modify] https://crrev.com/b4dc9337cacb2152671ea1bcfbcbd236d6fbff20/content/browser/frame_host/navigator_impl.cc [modify] https://crrev.com/b4dc9337cacb2152671ea1bcfbcbd236d6fbff20/content/browser/frame_host/navigator_impl.h [modify] https://crrev.com/b4dc9337cacb2152671ea1bcfbcbd236d6fbff20/content/browser/frame_host/render_frame_host_manager.cc [modify] https://crrev.com/b4dc9337cacb2152671ea1bcfbcbd236d6fbff20/content/browser/web_contents/web_contents_impl_unittest.cc
,
Mar 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/865ad44678b14ada619057ec2afb0d381f8c5ddd commit 865ad44678b14ada619057ec2afb0d381f8c5ddd Author: creis <creis@chromium.org> Date: Tue Mar 15 16:43:09 2016 Don't rely on the pending NavigationEntry for location.replace. This CL adds a should_replace_current_entry param to commit IPCs, and it ensures the WebDataSource is accurate on browser-initiated navigations. To keep the CL manageable, it does not yet change same-process location.replace navigations from EXISTING_PAGE to NEW_PAGE, though we want to do that as well. This goes further than the previous attempt and removes the old code that depended on the pending entry at commit time. This also corrects a bug where we were using the wrong pending entry to decide whether to replace in some cases. TBR=nasko (for IPC) BUG= 317872 , 593153 TEST=See bug 593153 for repro steps. CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation Review URL: https://codereview.chromium.org/1794513003 Cr-Commit-Position: refs/heads/master@{#381233} [modify] https://crrev.com/865ad44678b14ada619057ec2afb0d381f8c5ddd/content/browser/frame_host/navigation_controller_impl.cc [modify] https://crrev.com/865ad44678b14ada619057ec2afb0d381f8c5ddd/content/browser/frame_host/navigation_controller_impl.h [modify] https://crrev.com/865ad44678b14ada619057ec2afb0d381f8c5ddd/content/browser/frame_host/navigation_controller_impl_browsertest.cc [modify] https://crrev.com/865ad44678b14ada619057ec2afb0d381f8c5ddd/content/browser/frame_host/navigation_controller_impl_unittest.cc [modify] https://crrev.com/865ad44678b14ada619057ec2afb0d381f8c5ddd/content/common/frame_messages.h [modify] https://crrev.com/865ad44678b14ada619057ec2afb0d381f8c5ddd/content/renderer/render_frame_impl.cc [modify] https://crrev.com/865ad44678b14ada619057ec2afb0d381f8c5ddd/content/test/test_render_frame_host.cc [modify] https://crrev.com/865ad44678b14ada619057ec2afb0d381f8c5ddd/content/test/test_render_frame_host.h [modify] https://crrev.com/865ad44678b14ada619057ec2afb0d381f8c5ddd/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
,
Mar 15 2016
|
||
►
Sign in to add a comment |
||
Comment 1 by creis@chromium.org
, Mar 14 2016