New issue
Advanced search Search tips

Issue 775129 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Task

Blocking:
issue 734150



Sign in to add a comment

WebViewRestorableStateTest.EncodeDecode fails with WKBasedNavigationManager

Project Member Reported by ajuma@chromium.org, Oct 16 2017

Issue description

This is failing because the restored URL isn't "about:blank" as expected, but rather the path to restore_session.html constructed in WKBasedNavigationManagerImpl::Restore. It looks like the way this is supposed t work is that loading restore_session.html actually restores the history rather than becoming part of the history. Maybe the test needs to wait longer for this to happen? Just waiting for |loading| to be false on |restored_web_view| isn't enough to make the test pass though.

Failure log:
../../ios/web_view/test/web_view_restorable_state_inttest.mm:59: Failure
      Expected: @"about:blank"
      Which is: about:blank
To be equal to: [restored_web_view lastCommittedURL].absoluteString
      Which is: (null)
../../ios/web_view/test/web_view_restorable_state_inttest.mm:60: Failure
      Expected: @"about:blank"
      Which is: about:blank
To be equal to: [restored_web_view visibleURL].absoluteString
      Which is: file:///Users/ajuma/Library/Developer/CoreSimulator/Devices/937C81D0-6D27-4F72-85B7-673682BD27EA/data/Containers/Bundle/Application/AD229AAB-BB0B-46FB-9130-15BEE0058278/ios_web_view_inttests.app/Frameworks/ChromeWebView.framework/restore_session.html?session=%7B%22offset%22%3A0%2C%22titles%22%3A%5B%22%22%2C%22%22%5D%2C%22urls%22%3A%5B%22about%3Anewtab%22%2C%22about%3Ablank%22%5D%7D
 

Comment 1 by danyao@chromium.org, Oct 16 2017

There are two issues:
1) restore_session.html has a name conflict between lines 69 and 72 that stopped the page to finish loading.
2) Even with the fix, CRWWebView's lastCommittedURL is updated in |-updateCurrentURLs|. This WebStateObserver call is not triggered when restoring session.

Comment 2 by danyao@chromium.org, Oct 16 2017

Found an additional problem before hitting (2):

In CRWWebController |-loadCurrentURL| (triggered by WKNavigationManager::Restore()),  early exit triggers because _containerView is nil. Normal code path for initializing _containerView does not seem to trigger in restore code path.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 18 2017

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

commit 1c42b7a0d9771845d4d6ec9ef985eb7d0fe54f8f
Author: Danyao Wang <danyao@google.com>
Date: Wed Oct 18 18:16:34 2017

[Nav Experiment] Fix name conflict in restore_session.html.

Name conflict was introduced in a code cleanup in a previous CL. It was
not immediately detected because --enable-slim-navigation-manager is not
turned on in inttests.

Bug: 734150,  775129 
Change-Id: I22a97c6a518ac70381dd08bc03fb817ea29953dc
Reviewed-on: https://chromium-review.googlesource.com/722125
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509815}
[modify] https://crrev.com/1c42b7a0d9771845d4d6ec9ef985eb7d0fe54f8f/ios/web/navigation/resources/restore_session.html

Comment 4 by danyao@chromium.org, Nov 27 2017

Owner: danyao@chromium.org
Status: Fixed (was: Available)
This test passes now after the recent KVO changes ( crbug.com/776373 )

Sign in to add a comment