New issue
Advanced search Search tips

Issue 820484 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug

Blocking:
issue 820111



Sign in to add a comment

WebState::NavigationManager::LoadIfNecessary() should work if called after WebState::GetView()

Project Member Reported by sdefresne@chromium.org, Mar 9 2018

Issue description

When initialising a WebState from a saved state, it ends up in a state where CRWWebController _containerView is nil but potentially with a current navigation item.

In that case, calling WebState::NavigationManager::LoadIfNecessary() after WebState::GetView() does not result in the navigation starting, while calling them the other way around works. This is because GetView() force CRWWebController to create _containerView but does not start the navigation, while calling LoadIfNecessary() does nothing if the _containerView is non-nil.

See https://bugs.chromium.org/p/chromium/issues/detail?id=820111 for more details.

eugenebut: can you triage?
 
Components: -Internals>Mobile>Web Mobile>WebView>Glue
I'm the right owner for the bug. But it will not be trivial to fix and given that ios/web don't have good testing coverage the fix is also risky for a branch. 

Is there a safe workaround in ios/chrome for this? 
Project Member

Comment 2 by bugdroid1@chromium.org, May 22 2018

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

commit d53dd34a2524944370ffc9577969a56b13aa4032
Author: Eugene But <eugenebut@google.com>
Date: Tue May 22 17:58:16 2018

Make LoadIfNecessary work after WebState::GetView() call.

LoadIfNecessary was no-op if _containerView already exists. This is
incorrect, because WebState::GetView() creates _containerView, but
LoadIfNecessary should still work after WebState::GetView() call.

This CL introduces _currentURLLoadWasTrigerred variable which is used
inside LoadIfNecessary to prevent extra subsequent loads.

Bug:  820484 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Id199789094a25fe414015dd586bf1411f2e81bea
Reviewed-on: https://chromium-review.googlesource.com/1067440
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560698}
[modify] https://crrev.com/d53dd34a2524944370ffc9577969a56b13aa4032/ios/web/web_state/ui/crw_web_controller.mm

Sylvain, did we land any workarounds which we can now remove?
No workaround to remove. I found this while investigating a regression, but the fix is not a workaround (i.e. the regression triggered this bug but required a proper fix elsewhere too).
Status: Fixed (was: Assigned)

Sign in to add a comment