New issue
Advanced search Search tips

Issue 902097 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Check failed: (*url == url::kAboutBlankURL) || !url->SchemeIs(url::kAboutScheme) with WKBasedNavigationManager

Project Member Reported by danyao@chromium.org, Nov 5

Issue description

All egtests started failing recently with --enable-features=SlimNavigaitonManager at the following check:

[1105/171238.001920:FATAL:browser_about_rewriter.cc(35)] Check failed: (*url == url::kAboutBlankURL) || !url->SchemeIs(url::kAboutScheme).
0   ios_chrome_web_egtests              0x000000010f6cf95d base::debug::StackTrace::StackTrace(unsigned long) + 157
1   ios_chrome_web_egtests              0x000000010f6cf99d base::debug::StackTrace::StackTrace(unsigned long) + 29
2   ios_chrome_web_egtests              0x000000010f39416a base::debug::StackTrace::StackTrace() + 26
3   ios_chrome_web_egtests              0x000000010f3da54d logging::LogMessage::~LogMessage() + 461
4   ios_chrome_web_egtests              0x000000010f3d81f5 logging::LogMessage::~LogMessage() + 21
5   ios_chrome_web_egtests              0x000000010c9feaa6 WillHandleWebBrowserAboutURL(GURL*, web::BrowserState*) + 646
6   ios_chrome_web_egtests              0x000000010d957a54 web::BrowserURLRewriter::RewriteURLWithWriters(GURL*, web::BrowserState*, std::__1::vector<bool (*)(GURL*, web::BrowserState*), std::__1::allocator<bool (*)(GURL*, web::BrowserState*)> > const&) + 420
7   ios_chrome_web_egtests              0x000000010d958639 web::BrowserURLRewriterImpl::RewriteURLIfNecessary(GURL*, web::BrowserState*) + 41
8   ios_chrome_web_egtests              0x000000010d987a28 web::NavigationManagerImpl::CreateNavigationItemWithRewriters(GURL const&, web::Referrer const&, ui::PageTransition, web::NavigationInitiationType, GURL const&, std::__1::vector<bool (*)(GURL*, web::BrowserState*), std::__1::allocator<bool (*)(GURL*, web::BrowserState*)> > const*) const + 376
9   ios_chrome_web_egtests              0x000000010dab70a5 web::WKBasedNavigationManagerImpl::WKWebViewCache::GetNavigationItemImplAtIndex(unsigned long, bool) const + 997
10  ios_chrome_web_egtests              0x000000010dabb2eb web::WKBasedNavigationManagerImpl::GetNavigationItemImplAtIndex(unsigned long) const + 171
11  ios_chrome_web_egtests              0x000000010dabb3a4 web::WKBasedNavigationManagerImpl::GetLastCommittedItemImpl() const + 164
12  ios_chrome_web_egtests              0x000000010d9878a9 web::NavigationManagerImpl::GetLastCommittedItem() const + 25
13  ios_chrome_web_egtests              0x000000010db4e57e web::WebStateImpl::GetTitle() const + 302
14  ios_chrome_web_egtests              0x000000010f298c5f (anonymous namespace)::CreateItem(web::WebState*) + 303
15  ios_chrome_web_egtests              0x000000010f29968e -[TabGridMediator snapshotCache:didUpdateSnapshotForIdentifier:] + 334
16  CoreFoundation                      0x000000011c048e1c __invoking___ + 140
17  CoreFoundation                      0x000000011c0462a5 -[NSInvocation invoke] + 325
18  CoreFoundation                      0x000000011c0466f6 -[NSInvocation invokeWithTarget:] + 54
19  ios_chrome_web_egtests              0x000000010f3c34b9 -[CRBProtocolObservers forwardInvocation:] + 521
20  CoreFoundation                      0x000000011c046a08 ___forwarding___ + 776
21  CoreFoundation                      0x000000011c048b88 _CF_forwarding_prep_0 + 120
22  ios_chrome_web_egtests              0x000000010e0e6963 -[SnapshotCache removeImageWithSessionID:] + 675
23  ios_chrome_web_egtests              0x000000010e0f56a9 -[SnapshotGenerator removeSnapshot] + 89
24  ios_chrome_web_egtests              0x000000010e0fa463 SnapshotTabHelper::RemoveSnapshot() + 259
25  ios_chrome_web_egtests              0x000000010e0fae73 SnapshotTabHelper::DidStartLoading(web::WebState*) + 51
26  ios_chrome_web_egtests              0x000000010db4c781 web::WebStateImpl::SetIsLoading(bool) + 305
27  ios_chrome_web_egtests              0x000000010da2c1f6 -[CRWWebController registerLoadRequestForURL:referrer:transition:sameDocumentNavigation:hasUserGesture:] + 2294
28  ios_chrome_web_egtests              0x000000010da30896 -[CRWWebController didLoadNativeContentForNavigationItem:] + 246
29  ios_chrome_web_egtests              0x000000010da5ddcf -[CRWWebController webView:didFinishNavigation:] + 4767


 
The failure is triggered from -[CRWWebController didLoadNativeContentForNavigationItem:], so it doesn't happen if "BrowserContainerContainsNTP" feature is turned on.

The root cause is the code sharing of -[CRWWebController didLoadNativeContentForNavigationItem:] between LegacyNavigationManager and WKBasedNavigationManager. Because this is the only place where we can trigger WebStateImpl::SetIsLoading() and commit pending item for a native content navigation, WKBasedNavigationManager defers committing pending item in |webView:didCommitNavigation| for the placeholder item until this point. Now if any WebStateObserver queries NavigationManager::GetLastCommittedItem() when it observes the loading event from -[CRWWebController didLoadNativeContentForNavigationItem:], it will not find a navigation item associated with WKBackForwardList.currentItem. WKBasedNavigationManager will attempt to synthesize a navigation item for WKBackForwardList.currentItem. This code path triggers the check.
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 6

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

commit bce15ee8327451d125431b5756fd1d6e440223f4
Author: Danyao Wang <danyao@chromium.org>
Date: Tue Nov 06 00:47:30 2018

[Nav Experiment] Don't trigger URL rewriter on placeholder URL.

This fixes a DCHECK in browser_about_rewriter.cc because it is triggered
on about:blank?for=chrome://newtab in all egtests with
SlimNavigationManager enabled. Although this is not the root cause, it
is still more correct to skip URL rewriting altogether on placeholder
URLs than reverting the rewrite.

Bug:  902097 
Change-Id: I50b0848f0589f24398fb01a8c625b899ab8336f7
Reviewed-on: https://chromium-review.googlesource.com/c/1318790
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605546}
[modify] https://crrev.com/bce15ee8327451d125431b5756fd1d6e440223f4/ios/web/navigation/navigation_manager_impl.mm

Status: Fixed (was: Assigned)

Sign in to add a comment