Gracefully handle null pending item inside webView:didCommitNavigation:.

webView:didCommitNavigation: used to assume that pending item is
always valid and crashed on dereferencing null pointer. Pending item
should never be null inside webView:didCommitNavigation: but existing
ownership model is incorrect, so pending item could be distroyed before
committing.

This CL adds checks before dereferencing pending item to avoid the
crash. The real fix could be implemented by storing pending item in
NavigationContext object (crbug.com/925304).

Bug: 676458
Change-Id: Idf60e60cbe98111e8fed5d2903ae8bc8b8df3d90
Reviewed-on: https://chromium-review.googlesource.com/c/1444953
Reviewed-by: Justin Cohen <[email protected]>
Commit-Queue: Eugene But <[email protected]>
Cr-Commit-Position: refs/heads/master@{#627509}

Author: Eugene But

ios/web/web_state/ui/crw_web_controller.mm

@@ -1718,6 +1718,12 @@ - (void)updateCurrentBackForwardListItemHolder {
   if (_webUIManager)
     return;
 
+  if (!self.currentNavItem) {
+    // TODO(crbug.com/925304): Pending item (which stores the holder) should be
+    // owned by NavigationContext object. Pending item should never be null.
+    return;
+  }
+
   web::WKBackForwardListItemHolder* holder =
       [self currentBackForwardListItemHolder];
 
@@ -2933,7 +2939,10 @@ - (void)webPageChangedWithContext:(const web::NavigationContextImpl*)context {
   // keep it since it will have a more accurate URL and policy than what can
   // be extracted from the landing page.)
   web::NavigationItem* currentItem = self.currentNavItem;
-  if (!currentItem->GetReferrer().url.is_valid()) {
+
+  // TODO(crbug.com/925304): Pending item (which should be used here) should be
+  // owned by NavigationContext object. Pending item should never be null.
+  if (currentItem && !currentItem->GetReferrer().url.is_valid()) {
     currentItem->SetReferrer(referrer);
   }
 
@@ -4935,15 +4944,21 @@ - (void)webView:(WKWebView*)webView
   }
 
   [self commitPendingNavigationInfo];
-  if ([self currentBackForwardListItemHolder]->navigation_type() ==
-      WKNavigationTypeBackForward) {
-    // A fast back/forward won't call decidePolicyForNavigationResponse, so
-    // the MIME type needs to be updated explicitly.
-    NSString* storedMIMEType =
-        [self currentBackForwardListItemHolder]->mime_type();
-    if (storedMIMEType) {
-      self.webStateImpl->SetContentsMimeType(
-          base::SysNSStringToUTF8(storedMIMEType));
+
+  // TODO(crbug.com/925304): Pending item (which is stores
+  // currentBackForwardListItemHolder) should be owned by NavigationContext.
+  // Pending item should never be null.
+  if (self.currentNavItem) {
+    if ([self currentBackForwardListItemHolder]
+        -> navigation_type() == WKNavigationTypeBackForward) {
+      // A fast back/forward won't call decidePolicyForNavigationResponse, so
+      // the MIME type needs to be updated explicitly.
+      NSString* storedMIMEType =
+          [self currentBackForwardListItemHolder] -> mime_type();
+      if (storedMIMEType) {
+        self.webStateImpl->SetContentsMimeType(
+            base::SysNSStringToUTF8(storedMIMEType));
+      }
     }
   }