New issue
Advanced search Search tips

Issue 807102 link

Starred by 1 user
Status: Verified
Owner:
danyao@chromium.org
Closed: Jan 2018
Cc:
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Task

Blocking:
issue 734150



Sign in to add a comment

WKBasedNavigationManager only restores the last committed item

Project Member Reported by danyao@chromium.org, Jan 30 2018 
A regression in session restoration has been introduced in WKNavigationManager. When restoring session history, only the last committed item is restored.

Steps to reproduce:
1) In a new tab, load a few different URLs to generate history (e.g. amazon.ca -> wikipedia.org)
2) Double tab home button and swipe up to force kill Chrome
3) Restart Chrome, and click on "Restore" button in crash restore toast

Expected behavior: the full history is restored (i.e. NTP -> amazon.ca -> wikipedia.org)
Actual behavior: only wikipedia.org is restored.
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 30 2018 

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

commit b3b8f6c1ae8b8e080c83a6421dd9ef5541552927
Author: Danyao Wang <danyao@chromium.org>
Date: Tue Jan 30 16:01:14 2018

Use URL instead of virtual URL when creating NSURLRequest.

I believe this is a latent bug. NSURLReuest should reflect the actual
URL to be loaded in the web view, not the virtual URL that is meant to
be displayed to the user. Using virtual URL here breaks
WKBasedNavigationManagerImpl::Restore(), which creates a NavigationItem
whose URL is restore_session.html and whose virtual URL is the to-be-
restored page for display in omnibox.

The virtual URL usage was introduced in
https://codereview.chromium.org/2581193002. The CL description indicates
that virtual URL may not have been the correct value to use here in the
first place.

Bug:  807102 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I5d40be25ee26e370ffa960b6acb2fb7e74ee91d6
Reviewed-on: https://chromium-review.googlesource.com/892285
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532892}
[modify] https://crrev.com/b3b8f6c1ae8b8e080c83a6421dd9ef5541552927/ios/web/web_state/ui/crw_web_controller.mm
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 30 2018 

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

commit 8f2fad53cfd5423535130c3c14cf74241324c912
Author: Danyao Wang <danyao@chromium.org>
Date: Tue Jan 30 22:14:41 2018

[Nav Experiment] Restore app-specific URLs as placeholder URLs.

restore_session.html uses push/replaceState to restore URLs. This
technique doesn't work for app-specific URLs because they can't be
loaded natively by WKWebView. Restoring them as placeholder URLs reuses
the same code path for rendering WebUI and native view on back/forward
navigations.

Manually tested the session restore behavior and verified that history
with multiple entries are restored correctly.

Bug:  807102 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I9dd04f75dc464d645d30fe7c339e2ffe147b5fed
Reviewed-on: https://chromium-review.googlesource.com/894126
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533041}
[modify] https://crrev.com/8f2fad53cfd5423535130c3c14cf74241324c912/ios/web/navigation/navigation_item_impl.mm
[modify] https://crrev.com/8f2fad53cfd5423535130c3c14cf74241324c912/ios/web/navigation/session_storage_builder.mm
[modify] https://crrev.com/8f2fad53cfd5423535130c3c14cf74241324c912/ios/web/navigation/wk_based_navigation_manager_impl.mm
[modify] https://crrev.com/8f2fad53cfd5423535130c3c14cf74241324c912/ios/web/navigation/wk_based_navigation_manager_impl_unittest.mm
[modify] https://crrev.com/8f2fad53cfd5423535130c3c14cf74241324c912/ios/web/navigation/wk_based_restore_session_util.mm
[modify] https://crrev.com/8f2fad53cfd5423535130c3c14cf74241324c912/ios/web/navigation/wk_based_restore_session_util_unittest.mm
[modify] https://crrev.com/8f2fad53cfd5423535130c3c14cf74241324c912/ios/web/web_state/ui/crw_web_controller.mm

Comment 3 by danyao@chromium.org, Jan 30 2018

Status: Fixed (was: Started)

Comment 4 by shbarezer@chromium.org, Feb 6 2018

Status: Verified (was: Fixed)
Verified in 66.0.3340.0 Canary, iPhone 7 iOS11
Followed steps in comment #0.
Looks good.
►

Sign in to add a comment