[Nav Experiment] Abort native URL navigation if new navigation starts

With LegacyNavigationManagerImpl, native URLs are loaded synchronously
during the call to CRWWebController.loadCurrentURL, so it's impossible
for another navigation to start before the native URL navigation finishes.

With WKBasedNavigationManager, the call to loadCurrentURL triggers an
placeholder navigation on the WKWebView, and the actual work to load the
native URL happens when that placeholder navigation is finished. Since
the placeholder navigation happens asynchronously, a new navigation
(creating a new pending navigation item) can start before the placeholder
navigation finishes. In this situation, when the placeholder navigation
finishes, running its completion handler to load the native URL results
in incorrect behavior and a DCHECK, since the current navigation item
(the new one) no longer corresponds to that native URL navigation.

This CL skips running the completion handler for a placeholder navigation
when another navigation has started. This means that a WebStateObserver
sees neither a DidStartNavigation call nor a DidFinishNavigation call
for the native URL navigation.

This fixes crashes in several tests when loading a URL after opening a
new tab, when using WKBasedNavigationManager.

Bug: 775645
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I88ce3f524ca4298ad5905430c12dca759a9b7202
Reviewed-on: https://chromium-review.googlesource.com/788173
Commit-Queue: Ali Juma <ajuma@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519382}

ios/web/public/test/fakes/test_native_content_provider.mm

@@ -6,6 +6,7 @@
 
 #include <map>
 
+#include "ios/web/public/web_client.h"
 #include "url/gurl.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -26,6 +27,7 @@
 
 - (id<CRWNativeContent>)controllerForURL:(const GURL&)URL
                                 webState:(web::WebState*)webState {
+  DCHECK(web::GetWebClient()->IsAppSpecificURL(URL));
   auto nativeContent = _nativeContent.find(URL);
   return nativeContent == _nativeContent.end() ? nil : nativeContent->second;
 }

ios/web/web_state/ui/crw_web_controller.mm

@@ -4616,6 +4616,11 @@ registerLoadRequestForURL:(const GURL&)requestURL
       CRWPlaceholderNavigationInfo* placeholderNavigationInfo =
           [CRWPlaceholderNavigationInfo infoForNavigation:navigation];
       if (placeholderNavigationInfo) {
+        web::NavigationItem* item = self.currentNavItem;
+        // The |didFinishNavigation| callback can arrive after another
+        // navigation has started. Abort in this case.
+        if (CreatePlaceholderUrlForUrl(item->GetVirtualURL()) != webViewURL)
+          return;
         [placeholderNavigationInfo runCompletionHandler];
       }
       return;

ios/web/web_state/ui/crw_web_controller_unittest.mm

@@ -41,6 +41,7 @@
 #include "ios/web/public/web_state/web_state_observer.h"
 #import "ios/web/test/fakes/crw_fake_back_forward_list.h"
 #import "ios/web/test/fakes/crw_fake_wk_navigation_response.h"
+#include "ios/web/test/test_url_constants.h"
 #import "ios/web/test/web_test_with_web_controller.h"
 #import "ios/web/test/wk_web_view_crash_utils.h"
 #import "ios/web/web_state/ui/crw_web_controller_container_view.h"
@@ -58,6 +59,7 @@
 #include "third_party/ocmock/gtest_support.h"
 #include "third_party/ocmock/ocmock_extensions.h"
 #import "ui/base/test/ios/ui_view_test_utils.h"
+#include "url/scheme_host_port.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -120,6 +122,7 @@ class CRWWebControllerTest : public WebTestWithWebController {
     WebTestWithWebController::SetUp();
     mock_web_view_ = CreateMockWebView();
     scroll_view_ = [[UIScrollView alloc] init];
+    url_string_ = @(kTestURLString);
     [[[mock_web_view_ stub] andReturn:scroll_view_] scrollView];
 
     TestWebViewContentView* web_view_content_view =
@@ -142,6 +145,8 @@ class CRWWebControllerTest : public WebTestWithWebController {
     return {CGPointZero, container_view_size};
   }
 
+  void SetWebViewURL(NSString* url_string) { url_string_ = url_string; }
+
   // Creates WebView mock.
   UIView* CreateMockWebView() {
     id result = [OCMockObject mockForClass:[WKWebView class]];
@@ -157,7 +162,12 @@ class CRWWebControllerTest : public WebTestWithWebController {
 
     mock_wk_list_ = [[CRWFakeBackForwardList alloc] init];
     OCMStub([result backForwardList]).andReturn(mock_wk_list_);
-    [[[result stub] andReturn:[NSURL URLWithString:@(kTestURLString)]] URL];
+    // This uses |andDo| rather than |andReturn| since the URL it returns needs
+    // to change when |url_string_| changes.
+    OCMStub([result URL]).andDo(^(NSInvocation* invocation) {
+      NSURL* url = [NSURL URLWithString:url_string_];
+      [invocation setReturnValue:&url];
+    });
     [[result stub] setNavigationDelegate:[OCMArg checkWithBlock:^(id delegate) {
                      navigation_delegate_ = delegate;
                      return YES;
@@ -177,6 +187,7 @@ class CRWWebControllerTest : public WebTestWithWebController {
   UIScrollView* scroll_view_;
   id mock_web_view_;
   CRWFakeBackForwardList* mock_wk_list_;
+  NSString* url_string_;
 };
 
 // Tests that AllowCertificateError is called with correct arguments if
@@ -644,6 +655,46 @@ TEST_F(CRWWebControllerTest, CurrentUrlWithTrustLevel) {
   EXPECT_EQ(kAbsolute, trust_level);
 }
 
+// Tests that when a placeholder navigation is preempted by another navigation,
+// WebStateObservers get neither a DidStartNavigation nor a DidFinishNavigation
+// call for the corresponding native URL navigation.
+TEST_F(CRWWebControllerTest, AbortNativeUrlNavigation) {
+  // The legacy navigation manager doesn't have the concept of placeholder
+  // navigations.
+  if (!GetWebClient()->IsSlimNavigationManagerEnabled())
+    return;
+  GURL native_url(
+      url::SchemeHostPort(kTestNativeContentScheme, "ui", 0).Serialize());
+  NSString* placeholder_url = [NSString
+      stringWithFormat:@"%s%s", "about:blank?for=", native_url.spec().c_str()];
+  TestWebStateObserver observer(web_state());
+
+  WKNavigation* navigation =
+      static_cast<WKNavigation*>([[NSObject alloc] init]);
+  [static_cast<WKWebView*>([[mock_web_view_ stub] andReturn:navigation])
+      loadRequest:OCMOCK_ANY];
+  TestNativeContentProvider* mock_native_provider =
+      [[TestNativeContentProvider alloc] init];
+  [web_controller() setNativeProvider:mock_native_provider];
+
+  AddPendingItem(native_url, ui::PAGE_TRANSITION_TYPED);
+
+  // Trigger a placeholder navigation.
+  [web_controller() loadCurrentURL];
+
+  // Simulate the WKNavigationDelegate callbacks for the placeholder navigation
+  // arriving after another pending item has already been created.
+  AddPendingItem(GURL(kTestURLString), ui::PAGE_TRANSITION_TYPED);
+  SetWebViewURL(placeholder_url);
+  [navigation_delegate_ webView:mock_web_view_
+      didStartProvisionalNavigation:navigation];
+  [navigation_delegate_ webView:mock_web_view_ didCommitNavigation:navigation];
+  [navigation_delegate_ webView:mock_web_view_ didFinishNavigation:navigation];
+
+  EXPECT_FALSE(observer.did_start_navigation_info());
+  EXPECT_FALSE(observer.did_finish_navigation_info());
+}
+
 // Test fixture for testing CRWWebController presenting native content.
 class CRWWebControllerNativeContentTest : public WebTestWithWebController {
  protected: