New issue
Advanced search Search tips

Issue 776378 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 734150



Sign in to add a comment

CommonJsTest fails assert when using WKBasedNavigationManager

Project Member Reported by danyao@chromium.org, Oct 19 2017

Issue description

This test fails the assumption that WKBackForwardList.currentItem is not nil in WKNavigationDelegate's |-webView:didCommitNavigation| callback. It uses WebTestWebState::LoadHTML(), which calls WKWebView's |-loadHTMLString:baseURL|, which does not generate a WKBackForwardListItem.

This is a flaw in our assumption about how WKWebView navigation works.

A workaround is to load about:blank into the web view as part of test setup. But finding a better solution can simplify WebUI handling as well.

Callstack:
FATAL:wk_based_navigation_manager_impl.mm(38)] Check failed: wk_item.
0   ios_web_unittests                   0x0000000100e8282d base::debug::StackTrace::StackTrace(unsigned long) + 157
1   ios_web_unittests                   0x0000000100e8286d base::debug::StackTrace::StackTrace(unsigned long) + 29
2   ios_web_unittests                   0x0000000100e80d8c base::debug::StackTrace::StackTrace() + 28
3   ios_web_unittests                   0x0000000100edb91f logging::LogMessage::~LogMessage() + 479
4   ios_web_unittests                   0x0000000100ed9475 logging::LogMessage::~LogMessage() + 21
5   ios_web_unittests                   0x0000000100a02cc4 (anonymous namespace)::SetNavigationItemInWKItem(WKBackForwardListItem*, std::__1::unique_ptr<web::NavigationItemImpl, std::__1::default_delete<web::NavigationItemImpl> >) + 276
6   ios_web_unittests                   0x0000000100a034de web::WKBasedNavigationManagerImpl::CommitPendingItem() + 830
7   ios_web_unittests                   0x0000000100a838a4 -[CRWWebController didStartLoadingURL:] + 196
8   ios_web_unittests                   0x0000000100a8368d -[CRWWebController webPageChanged] + 621
9   ios_web_unittests                   0x0000000100a9a77d -[CRWWebController webView:didCommitNavigation:] + 3453
10  WebKit                              0x0000000108822509 WebKit::NavigationState::NavigationClient::didCommitNavigation(WebKit::WebPageProxy&, API::Navigation*, API::Object*) + 91
 

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

 Issue 776043  has been merged into this issue.

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

Also affects these tests:

ContextMenuJsTest.*
ScriptExecutionTest.*
CRWWebControllerJSExecutionTest.*
WebStateTest.LoadingProgress
WebStateTest.OverridingWebKitObject
WebStateTest.ScriptExecution
WebStateTest.Snapshot
WebStateTest.UserScriptExecution

Comment 3 by danyao@chromium.org, Oct 23 2017

Also:

JsInjectionManagerTest.*
WindowOpenByDomTest*
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 26 2017

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

commit a965ace552fb3a97de878b53573e17d96ea5eb36
Author: Danyao Wang <danyao@chromium.org>
Date: Thu Oct 26 20:29:34 2017

[Nav Experiment] Add about:blank workaround to fix WebTestWithWebState.

These unit tests call [CRWWebController -loadHTML] on an empty WKWebView
This breaks the collective assumption in CRWWebController and
WKBasedNavigationManager that -loadHTML is always called on an existing
WKBackForwardListItem. This CL works around it by loading about:blank
before calling -loadHTML if WKBackForwardList is empty.

The only other user of [CRWWebController -loadHTML] is WebUI, which
already has a similar hack implemented in
[CRWWebController -loadHTML:forAppSpecificURL].

Bug:  776378 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I4d529011b8a9eea906632c13edc1da20e31b52a9
Reviewed-on: https://chromium-review.googlesource.com/735128
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511945}
[modify] https://crrev.com/a965ace552fb3a97de878b53573e17d96ea5eb36/ios/web/public/test/web_test_with_web_state.mm

Comment 5 by danyao@chromium.org, Oct 27 2017

Status: Fixed (was: Available)

Sign in to add a comment