Only consider loads of the last committed URL for conversion to reloads |
|
Issue descriptionhttps://codereview.chromium.org/2580753002 changed the way we consider a load of the current URL a reload in order to fix an issue with the Inbox app (which uses the Android WebView). Prior to this CL, we would consider that a load of the last committed URL was a reload. However, this caused issue when an app tried to do the following: - navigation to url1 - start navigation to url2 - start a navigation to url1 before the navigation to url2 commmits. This one is classified as a reload. Inbox doesn't expect this. In order to avoid this, the CL introduced a mechanism where we would compare against the URL of the last pending navigation instead. In this case, it would be url2 so the last navigation isn't classified as a reload. However, this change introduces new bugs. For example, when we do the following: - navigation to url1 - start navigation to url2 - start a navigation to url2 again before the previous navigation to url2 commmits. This one is classified as a reload (since the last pending navigation was to url2). - the navigation to url2 commits but does not create a new entry (since it's a reload). Instead, the NavigationEntry for url1 is modified, but the navigation to url2 was not a reload of url1. So we should only be assigning navigations as reloads when their url match a committed NavigationEntry, otherwise we mess up the navigation history. Since the CL landed, there was another issue with this mechanism and Android WebView uncovered in issue 794020 . https://chromium-review.googlesource.com/824682 landed, which disables the convert to reload for navigations started from an Android WebView. This should fix the initial Inbox issue as well. We should go back to using only the last committed NavigationEntry in the convert-to-reload mechanism.
,
Feb 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0a656e429f0454c7ff10ce9eaa1d655aed94fd7c commit 0a656e429f0454c7ff10ce9eaa1d655aed94fd7c Author: clamy <clamy@chromium.org> Date: Tue Feb 06 18:18:28 2018 Fix conversion of Enter-in-omnibox to reload This CL fixes an issue where new navigations are wrongly considered a reload. Prior to this CL, a browser-initiated navigation would be converted to a reload if it matches the URL of the last attempted navigation. However, we should only compare to the URL of the last committed navigation, otherwise we will not create a new NavigationEntry when the navigation commits but will instead modify the last committed NavigationEntry. This is an issue since we did the comparison with the last pending NavigationEntry, not the last committed one, so it is quite likely that their URLs don't match. BUG=809040 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I86a3c2f9b933bc6eadd89a3e694e01b985ac4e91 Reviewed-on: https://chromium-review.googlesource.com/824663 Commit-Queue: Camille Lamy <clamy@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org> Cr-Commit-Position: refs/heads/master@{#534723} [modify] https://crrev.com/0a656e429f0454c7ff10ce9eaa1d655aed94fd7c/content/browser/frame_host/navigation_controller_impl.cc [modify] https://crrev.com/0a656e429f0454c7ff10ce9eaa1d655aed94fd7c/content/browser/frame_host/navigation_controller_impl.h [modify] https://crrev.com/0a656e429f0454c7ff10ce9eaa1d655aed94fd7c/content/browser/frame_host/navigation_controller_impl_unittest.cc [modify] https://crrev.com/0a656e429f0454c7ff10ce9eaa1d655aed94fd7c/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter |
|
►
Sign in to add a comment |
|
Comment 1 by creis@chromium.org
, Feb 5 2018Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Status: Started (was: Assigned)