ErrorPageTestCase tests fail if SlimNavigationManager flag is enabled |
||
Issue description
App Version (from "Chrome Settings > About Chrome"): ToT
iOS Version: 11.4
Device: iPhone Simulator
Steps to reproduce:
1.) Enable SlimNavigationManager
2.) Run ErrorPageTestCase test suite
Observed behavior:
Test crash.
* thread #1, name = 'CrWebMain', queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
* frame #0: 0x000000010344393a ios_chrome_web_egtests`::-[CRWWebController webView:didFinishNavigation:](self=0x00007ffd77856a80, _cmd="webView:didFinishNavigation:", webView=0x00007ffd7a0b2c00, navigation=0x00007ffd77876830) at crw_web_controller.mm:4813
frame #1: 0x0000000116596fe3 WebKit`WebKit::NavigationState::NavigationClient::didFinishNavigation(WebKit::WebPageProxy&, API::Navigation*, API::Object*) + 93
frame #2: 0x000000011674bde7 WebKit`WebKit::WebPageProxy::didFinishLoadForFrame(unsigned long long, unsigned long long, WebKit::UserData const&) + 325
frame #3: 0x000000011676ceb4 WebKit`void IPC::handleMessage<Messages::WebPageProxy::DidFinishLoadForFrame, WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long long, unsigned long long, WebKit::UserData const&)>(IPC::Decoder&, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(unsigned long long, unsigned long long, WebKit::UserData const&)) + 100
frame #4: 0x000000011659333b WebKit`IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) + 127
frame #5: 0x00000001167c9646 WebKit`WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 24
frame #6: 0x000000011655b5ed WebKit`IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 119
frame #7: 0x000000011655de8f WebKit`IPC::Connection::dispatchOneMessage() + 177
frame #8: 0x000000011efd3489 JavaScriptCore`WTF::RunLoop::performWork() + 329
,
Jun 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c119f150878b8678c457e859cccb7c2501f61677 commit c119f150878b8678c457e859cccb7c2501f61677 Author: Danyao Wang <danyao@google.com> Date: Tue Jun 12 21:01:07 2018 [Nav Experiment] Fix crash in ErrorPageTestCase. If a new navigation has started before |webView:didFinishNavigation| arrives, early exit and stop processing. Bug: 851708 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: Iea6ed9be60a547bc3fc2ee5bad6104b3a9eaef40 Reviewed-on: https://chromium-review.googlesource.com/1097564 Commit-Queue: Danyao Wang <danyao@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Cr-Commit-Position: refs/heads/master@{#566566} [modify] https://crrev.com/c119f150878b8678c457e859cccb7c2501f61677/ios/web/web_state/ui/crw_web_controller.mm
,
Jun 12 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by danyao@chromium.org
, Jun 12 2018This is a tricky one because it doesn't always reproduce. The cause is this line returning nullptr: web::NavigationItemImpl* item = web::GetItemWithUniqueID( self.navigationManagerImpl, context->GetNavigationItemUniqueID()); Before https://chromium-review.googlesource.com/c/chromium/src/+/1087640, self.currentNavItem here. However, I noticed that self.currentNavItem is not always the right item associated with the WKNavigation*, and it caused the error retry state of the wrong item to be updated. When I'm able to reproduce the failure, the followings are always true: 1. webView.URL and webView.backForwardList.currentItem.URL are both NTP placeholder URL. 2. webView.backForwardList contains only a single item, which is the NTP placeholder item 3. NavigationContext's URL is also NTP placeholder URL 4. context->GetNavigationItemUniqueID() is not -1, and always smaller than self.currentNavItem->GetUniqueID(). This means that a navigation item existed at one point. 5. self.currentNavItem.URL is the first URL being loaded into the test, not NTP 6. [[CRWNavigationItemHolder holderForBackForwardListItem:webView.backForwardList.currentItem] navigationItem] returns an item with a unique ID greater than context->GetNavigationItemUniqueID() and self.currentNavItem->GetUniqueID() I think my change revived the bug that was fixed in https://chromium-review.googlesource.com/788173. Basically if self.currentNavItem->GetURL() is different from the URL encoded in the placeholder URL, it means that this |webView:didFinishNavigation| arrived late. We can just abort. I'll send a fix soon.