New issue
Advanced search Search tips

Issue 700571 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug

Blocking:
issue 678047
issue 704721



Sign in to add a comment

Implement the logic for ORIGINAL_REQUEST_URL in |Reload| function in NavigationManager.

Project Member Reported by liaoyuke@chromium.org, Mar 10 2017

Issue description

When ORIGINAL_REQUEST_URL is specified as the reload type for reload function in NavigationManager, we should set the URL of the visible item as the URL of the original URL of the last item (including pending item) that is not a redirect, and then call |Reload| of WebController.
 
Blocking: 704721
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d1e82b93ec0328ec947f387a84fdf2fd42f61f8

commit 6d1e82b93ec0328ec947f387a84fdf2fd42f61f8
Author: liaoyuke <liaoyuke@chromium.org>
Date: Tue Mar 28 17:50:23 2017

Implement Reload for ORIGINAL_REQUEST_URL in NavigationManagerImpl.

This CL implements logic for |Reload| in NavigationManagerImpl when
reload type is ORIGINAL_REQUEST_URL for reloading a page with updated
user agent.

BUG= 700571 

Review-Url: https://codereview.chromium.org/2766453002
Cr-Commit-Position: refs/heads/master@{#460161}

[modify] https://crrev.com/6d1e82b93ec0328ec947f387a84fdf2fd42f61f8/ios/web/navigation/crw_session_controller.mm
[modify] https://crrev.com/6d1e82b93ec0328ec947f387a84fdf2fd42f61f8/ios/web/navigation/navigation_manager_impl.mm
[modify] https://crrev.com/6d1e82b93ec0328ec947f387a84fdf2fd42f61f8/ios/web/navigation/navigation_manager_impl_unittest.mm
[modify] https://crrev.com/6d1e82b93ec0328ec947f387a84fdf2fd42f61f8/ios/web/public/reload_type.h

When reload with ORIGINAL_REQUEST_URL, the original plan was to reload with the original url of the last user item, but one problem with this approach is that: because web controller blindly reloads the "current item", we will need to discard the redirect pending item and those redirect committed items between the last user item and the last committed item in order to make the last user item our new "current item".

Quote from Eugene: "Discarding items looks like putting bandages around broken system :(", so we shouldn't implement this workaround.

Now that I've verified that a server redirect will not create a new item: https://codereview.chromium.org/2775023002/, and some experiments show that most popular websites won't issue a client side redirect to redirect the user from www.website.com to m.website.com, which means that reloading with the original url of "current item" should work for most websites.

The plan is that we implement "current item" first, and if it turns out that this approach is not working reliably, I'll fix the client side redirects logic to make it not creating new items.

The aforementioned "current item" is defined as:

if (transitent_item)
  return transient_item;
else if (pending_item)
  return pending_item;
else
  return last_committed_item;
Status: Fixed (was: Started)

Sign in to add a comment