New issue
Advanced search Search tips

Issue 818796 link

Starred by 0 users

Issue metadata

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



Sign in to add a comment

NewTabPage is not displayed correctly after navigating back from plus.google.com

Project Member Reported by srikanthg@chromium.org, Mar 5 2018

Issue description

App Version: 67.0.3362.0 canary
iOS Version: 11.2.6
Device: iPad only
URL: any

Precondition: Enable #slim-navigation-manager from about://flags
Make sure you are signed into Chrome

Steps to reproduce:
  1. Launch Google Chrome
  2. Type plus.google.com in omnibox and Go
  3. Tap on Back arrow from toolbar

Observed results: Observe that NTP is displayed blank.

Expected results: NTP contents should be displayed

Note: Works fine on iPhones

Number of times you were able to reproduce: 5/5
Bug reproducible after clean install: Yes
Bug reproducible after clearing cache and cookies: Yes
Bug reproducible on Chrome Mobile on Android: NA
Bug reproducible on Safari/Firefox: Firefox: NA, Safari: NA
Bug reproducible on current stable build (App Version, iOS Version): M64 NO (New Feature)
Bug reproducible on the current beta channel build (App Version, iOS Version): M65 NO (New Feature)

Link to video/image: https://drive.google.com/file/d/1zO0HfuvByJ1kDLPrwZHB7BScsH9UDh4P/view 
 

Comment 1 by pkl@chromium.org, Mar 5 2018

Owner: danyao@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by danyao@chromium.org, Mar 23 2018

Two strange things are happening here:

1. When navigating back from plus.google.com to NTP, |webView:didFinishNavigation| is called before |webView:didCommitNavigation|
2. webView.URL changed from plus.google.com to NTP placeholder to plus.google.com again before coming back to NPT placeholder again.

This sequence of events are observed after hitting the back button:

1. about:blank?for=chrome%3A%2F%2Fnewtab%2F | loading: 1
2. decidePolicy: about:blank?for=chrome%3A%2F%2Fnewtab%2F
3. didStart [<WKNavigation: 0x7fee75d17760>]: about:blank?for=chrome%3A%2F%2Fnewtab%2F
4. URLDidChange: https://plus.google.com/ | loading: 0
5. didFinish [<WKNavigation: 0x7fee75d17760>]: https://plus.google.com/
6. URLDidChange: about:blank?for=chrome%3A%2F%2Fnewtab%2F | loading: 1
7. didCommit [<WKNavigation: 0x7fee75d17760>]: about:blank?for=chrome%3A%2F%2Fnewtab%2F

#4 is unexpected. It's not clear why URL changed back to plus.google.com with webView.loading = false before the navigation is committed.
#5 is expected to be called after #7

I can reproduce this out-of-order behavior reasonably consistently in Chrome. However, I can't reproduce it at all in a vanilla WKWebView, which make me think something in Chrome is causing this behavior.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 29 2018

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

commit 12d074a88884b24439b82982960aa4b895ee4946
Author: Danyao Wang <danyao@chromium.org>
Date: Thu Mar 29 20:56:18 2018

[Nav Experiment] Handle out-of-order WKNavigationDelegate callback.

For some reason, plus.google.com triggers a WKWebView bug that triggers
|webView:didFinishNavigation| before |webView:didCommitNavigation|
when navigation back to NTP. With WKBasedNavigationManager,
|webView:didFinishNavigation| triggers the presentation of native
content which relies on states updated in |webView:didCommitNavigation|.

Finish-before-commit has been observed before (crbug.com/727289). The
original fix made it possible to call didCommit without crashes after
didFinish has already been called. This is not sufficient for the
WKBasedNavigationManager use case because native controller needs last
committed navigation item to have been updated.

This CL calls didCommit in didFinish if a finish-before-commit scenario
is detected. UMA counters are added to track the frequency to ensure
this is indeed an exceptional case.

Bug:  818796 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I274cc37b494478882b4d8f2a773c33df5b29c703
Reviewed-on: https://chromium-review.googlesource.com/981299
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546950}
[modify] https://crrev.com/12d074a88884b24439b82982960aa4b895ee4946/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/12d074a88884b24439b82982960aa4b895ee4946/tools/metrics/histograms/histograms.xml

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

Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
Verified on chrome canary version 67.0.3387.0 on iPad Air with iOS 11.3 beta 6, following steps mentioned in comment #0.  NTP contents are displayed.  Looks good.

Sign in to add a comment