1166860 - Crash in BrowserViewController.webView(, didFinishNavigation:)

Status: RESOLVED FIXED

(Firefox for iOS :: Browser, defect)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

func webView(webView: WKWebView, didFinishNavigation navigation: WKNavigation!) {
        let tab: Browser! = tabManager[webView]

        tab.expireSnackbars()

        let notificationCenter = NSNotificationCenter.defaultCenter()
        var info = [NSObject: AnyObject]()
        info["url"] = tab.displayURL
        info["title"] = tab.title
>>>     if let visitType = self.getVisitTypeForTab(tab, navigation: navigation)?.rawValue {
            info["visitType"] = visitType
        }
        notificationCenter.postNotificationName("LocationChange", object: self, userInfo: info)

Because navigation is nil. This is interesting because I don't think that is documented to have nil as a value.

Comment 1

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

STR taken from bug 1165745

Steps to Reproduce:
1. Open cnn.com
2. Click the first article you see
3. Verify it has reader mode available (Reader mode icon appears in URL bar)
4. Tap the "Back" button
5. Tap the "Forward" button, verify Reader mode is still available (be patient, it appears ~5 seconds after the webpage loads)
6. Tap the "Back" button
7. Tap the "Forward" button

First time going back is ok. Second time crashes.

Comment 2

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

This is the WebKit code. See the FIXME comment. Means we can expect nil.

void NavigationState::LoaderClient::didFinishLoadForFrame(WebPageProxy& page, WebFrameProxy& webFrameProxy, API::Navigation* navigation, API::Object*)
{
    if (!webFrameProxy.isMainFrame())
        return;

    if (!m_navigationState.m_navigationDelegateMethods.webViewDidFinishNavigation)
        return;

    auto navigationDelegate = m_navigationState.m_navigationDelegate.get();
    if (!navigationDelegate)
        return;

    // FIXME: We should assert that navigation is not null here, but it's currently null for some navigations through the page cache.
    WKNavigation *wkNavigation = nil;
    if (navigation)
        wkNavigation = wrapper(*navigation);

    [navigationDelegate webView:m_navigationState.m_webView didFinishNavigation:wkNavigation];
}

Comment 3

[Richard Newman [:rnewman]]

Ugh. Yay for other people's bugs! Still, should be an easy fix.

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

https://github.com/mozilla/firefox-ios/commit/1310214db4afd8fb26ad3acb68ff14630d8023f6