New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 815248 link

Starred by 5 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

All tabs reset to about:blank with no history.

Project Member Reported by michaeldo@chromium.org, Feb 23 2018

Issue description

Chrome Version: 66.0.3352.0
OS: iOS 11.2.6

Precondition:
Enable "Use Slim Navigation Manager" at chrome://flags

Steps to reproduce:
I can't reproduce this yet on a dev build from Xcode, but I am actively trying to do so. I will provide exact steps if I can. So far, here are estimated steps:

(1) Open 5 different tabs with various URLs and available back history.
(2) Background Chrome and use the phone for a while until Chrome is killed by the system.
(3) Re-launch Chrome.
(4) Open tab switcher.
(5) Select a different tab.

Expected result:
The other tab should load the page as shown in the displayed preview image.

Actual result:
In 4, the title of pages have changed to about:blank
In 5, the tab loads with no content and has no history.

Notes:
I regularly use Canary as my daily browser and have noticed that when I come back to old tabs, they load "about:blank" without any history instead of the expected content. 
This also happens for tabs that were created after I flipped the feature flag.
I have many more than 100 tabs, but I don't know the exact count.
 
IMG-8372.JPG
813 KB View Download
IMG-8343.JPG
78.6 KB View Download
This actually looks like it may be  crbug.com/777945 .

Comment 2 by pkl@chromium.org, Feb 26 2018

Cc: eugene...@chromium.org
Status: Assigned (was: Untriaged)
Labels: -Restrict-View-Google ReleaseBlock-Stable M-67

Comment 4 by danyao@chromium.org, Mar 14 2018

Cc: danyao@chromium.org
 Issue 777945  has been merged into this issue.

Comment 5 by danyao@chromium.org, Mar 19 2018

 Issue 818790  has been merged into this issue.

Comment 6 by danyao@chromium.org, Mar 22 2018

Michael - can you help check if this is still a probably with the most recent Canary build? I was never able to reproduce this consistently in iOS11.2, but the symptoms seem very similar to  crbug.com/821752 , and that is fixed now.
I just hit this issue again after updating Canary a few days ago.

Is there anything I can do to help diagnose this? I just cleared all my tabs (I had 71) since they were broken to ensure I am in a clean state. I will report back if I see it again. I will also update Canary now but then now update to ensure that isn't triggering it.
Cc: michaeldo@chromium.org

Comment 9 by danyao@chromium.org, Mar 28 2018

Thanks Michael! If you can note down the exact circumstance when the bug manifests, it'll help me narrow down the issue. Important bits include: how many tabs are open at the time, is this the first cold restart after some amount of usage, are all tabs blank or just some of them.
I was just able to reproduce this! I only had 3 tabs.

Here were the steps:
- Entered search terms into the omnibox on Tab 3 that already has navigation history.
- Search triggered a crash (my specific crash report is linked from crbug.com/826992), it looks like the crash is a dup of crbug.com/797756
- Relaunched Chrome and Tapped on "Restore"
- All tabs are in "blank" state with no history except the last (currently active) tab.
Thanks! It looks like the issue is with crashed / evicted WKWebView. When the tab is revived, the navigation history is not restored into the web state. I was able to reproduce the behavior in iOS simulator by killing the WebContent process directly (kill -9).

This is also consistent with the observation that this tends to happen to people with lots of tabs, but doesn't reproduce easily when I tried, because I would create just a single new tab for testing. 

The solution is to call WebStateImpl::RestoreSessionStorage() when the evicted web state is restored. A naive hack to just call this method in WebStateImpl::GetView() if IsEvicted() is true loads the page correctly, but it doesn't fix "Untitled" in the tab switcher. Also killing the web process of the same web state again triggers a DCHECK in TabUsageRecorder::RendererTerminated():

DCHECK(!WebStateAlreadyEvicted(terminated_web_state));

Clearly there are some additional book keeping that needs to be sorted out.
 Issue 825824  has been merged into this issue.
Issue 825864 has been merged into this issue.
If its useful here are the consistent repro steps:

1. Launch Google Chrome
2. Open two tabs (say tab#1, facebook.com, tab#2: wikipedia.org)
3. Open tab#3 and navigate to browsingtest.appspot.com/venti.html
4. Open this venti.html in few more tabs so that previous tabs will be evicted.
5. Enter tab swither and observe that tab#1, tab#2 shows Untitled. and also opening these tabs will show about:blank
Cc: marq@chromium.org edchin@chromium.org ghendel@chromium.org
 Issue 824721  has been merged into this issue.
Project Member

Comment 16 by bugdroid1@chromium.org, Apr 11 2018

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

commit 0efc969a9f49f256af06e077c65feb193239f397
Author: Danyao Wang <danyao@chromium.org>
Date: Wed Apr 11 14:17:09 2018

[Nav Experiment] Add DetachFromWebView to WKBasedNavigationManagerImpl.

CRWWebController sometimes needs to remove WKWebView (e.g. crbug/770914)
and this obliterates the session history when WKBasedNavigationManager
is used. This CL adds the ability for WKBasedNavigationManager to
temporarily be detached from the web view.

Major parts to this CL:
1. NavigationManagerImpl::DetachFromWebView() is a new API that
   CRWWebController uses to notify the navigation manager before
   removing the web view.
2. WKBasedNavigationManager implements this API with the help of a new
   nested class WKWebViewShim. It knows how to cache the items when
   detaching from a web view and service getter calls in detached mode.
3. Added overrides for FinishReload(), FinishLoadURLWithParams() and
   LoadIfNecessary() in WKBasedNavigationManager so it can switch back
   to attached mode when a new navigation starts by restoring the cached
   navigation items.

Also cleaned up unneeded APIs from CRWWebViewNavigationProxy.

Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I2b1796731964f021df8c0489d3a6ef56ae390e04
Bug:  815248 
Reviewed-on: https://chromium-review.googlesource.com/999054
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549880}
[modify] https://crrev.com/0efc969a9f49f256af06e077c65feb193239f397/ios/web/navigation/navigation_manager_impl.h
[modify] https://crrev.com/0efc969a9f49f256af06e077c65feb193239f397/ios/web/navigation/navigation_manager_impl.mm
[modify] https://crrev.com/0efc969a9f49f256af06e077c65feb193239f397/ios/web/navigation/wk_based_navigation_manager_impl.h
[modify] https://crrev.com/0efc969a9f49f256af06e077c65feb193239f397/ios/web/navigation/wk_based_navigation_manager_impl.mm
[modify] https://crrev.com/0efc969a9f49f256af06e077c65feb193239f397/ios/web/navigation/wk_based_navigation_manager_impl_unittest.mm
[modify] https://crrev.com/0efc969a9f49f256af06e077c65feb193239f397/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/0efc969a9f49f256af06e077c65feb193239f397/ios/web/web_state/ui/crw_web_view_navigation_proxy.h

Project Member

Comment 17 by bugdroid1@chromium.org, Apr 12 2018

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

commit 8ed28accc8f5dbbb3e8f2de8bda853c2b7feaa5e
Author: Danyao Wang <danyao@chromium.org>
Date: Thu Apr 12 14:26:44 2018

[Nav Experiment] Set virtual URL for restore session URL.

This removes flicker in Omnibox that briefly exposes the internal URL
when restoring session history. Virtual URL must be set on the pending
item before calling |loadCurrentURLInWebView| because
WebStateObserver::DidStartLoading event fires during this call, which
triggers ToolbarModelDelegateIOS to update itself using the virtual
URL of the pending item.

Bug:  815248 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I260e251d90ad832120b908a61843fe30df261be3
Reviewed-on: https://chromium-review.googlesource.com/1003252
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550184}
[modify] https://crrev.com/8ed28accc8f5dbbb3e8f2de8bda853c2b7feaa5e/ios/web/navigation/navigation_manager_impl.mm
[modify] https://crrev.com/8ed28accc8f5dbbb3e8f2de8bda853c2b7feaa5e/ios/web/navigation/wk_based_navigation_manager_impl.mm
[modify] https://crrev.com/8ed28accc8f5dbbb3e8f2de8bda853c2b7feaa5e/ios/web/public/navigation_manager.h

Status: Fixed (was: Assigned)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-67; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-67 label, otherwise remove Merge-TBD label. Thanks.
Labels: -Merge-TBD Merge-Request-67
Project Member

Comment 21 by sheriffbot@chromium.org, Apr 12 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: We don't branch M67 until 2018-04-12.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-67
The cl landed last night for M67 branch. No merge approval needed. I am removing both merge review labels.

https://storage.googleapis.com/chromium-find-releases-static/0ef.html#0efc969a9f49f256af06e077c65feb193239f397
Status: Verified (was: Fixed)
Verified on chrome canary version 68.0.3398.0 on iPhone 8 plus with iOS 11.4, 11.3, following steps mentioned in comment #14. Tap title is displayed in tab switcher mode and webpages are loaded.  Looks good. 

Comment 25 Deleted

Please also verify on iOS 11.2.6. #slim-navigation-manager is force disabled on 11.3+.
Cc: srikanthg@chromium.org linds...@chromium.org
Status: Fixed (was: Verified)
Srikanth, please reassign this for verification
Project Member

Comment 28 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ed28accc8f5dbbb3e8f2de8bda853c2b7feaa5e

commit 8ed28accc8f5dbbb3e8f2de8bda853c2b7feaa5e
Author: Danyao Wang <danyao@chromium.org>
Date: Thu Apr 12 14:26:44 2018

[Nav Experiment] Set virtual URL for restore session URL.

This removes flicker in Omnibox that briefly exposes the internal URL
when restoring session history. Virtual URL must be set on the pending
item before calling |loadCurrentURLInWebView| because
WebStateObserver::DidStartLoading event fires during this call, which
triggers ToolbarModelDelegateIOS to update itself using the virtual
URL of the pending item.

Bug:  815248 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I260e251d90ad832120b908a61843fe30df261be3
Reviewed-on: https://chromium-review.googlesource.com/1003252
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550184}
[modify] https://crrev.com/8ed28accc8f5dbbb3e8f2de8bda853c2b7feaa5e/ios/web/navigation/navigation_manager_impl.mm
[modify] https://crrev.com/8ed28accc8f5dbbb3e8f2de8bda853c2b7feaa5e/ios/web/navigation/wk_based_navigation_manager_impl.mm
[modify] https://crrev.com/8ed28accc8f5dbbb3e8f2de8bda853c2b7feaa5e/ios/web/public/navigation_manager.h

Status: Verified (was: Fixed)
Verified on iOS11.2.6 iPhone8, iPhone7
based on my earlier steps to repro from comment#12. Tabs restores correctly after evicted.

Sign in to add a comment