Fixed Navigation callbacks for failed provisional navigation.

Verified callbacks correctness with
NavigationCallbacksTest.FailedNavigation integration test.

Before this CL DidStartNavigation was called twice (second time
for presenting error in Native Content). DidFinishNavigation was
called once (also for presenting NativeContent). First
DidStartNavigation did not have a matching DidStartNavigation
call.

This CL removes DidStartNavigation/DidFinishNavigation callbacks
for for presenting error in Native Content and adds balanced
DidFinishNavigation callback in handleLoadingError:.

Bug: 725241
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Id15ef8c9422e571f74f9c00bf88ea98ccbf6607c
Reviewed-on: https://chromium-review.googlesource.com/758718
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517420}

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

@@ -27,17 +27,14 @@
 - (id<CRWNativeContent>)controllerForURL:(const GURL&)URL
                                 webState:(web::WebState*)webState {
   auto nativeContent = _nativeContent.find(URL);
-  if (nativeContent == _nativeContent.end()) {
-    return nil;
-  }
-  return nativeContent->second;
+  return nativeContent == _nativeContent.end() ? nil : nativeContent->second;
 }
 
 - (id<CRWNativeContent>)controllerForURL:(const GURL&)URL
                                withError:(NSError*)error
                                   isPost:(BOOL)isPost {
-  NOTREACHED();
-  return nil;
+  auto nativeContent = _nativeContent.find(URL);
+  return nativeContent == _nativeContent.end() ? nil : nativeContent->second;
 }
 
 @end

ios/web/web_state/navigation_callbacks_inttest.mm

@@ -8,12 +8,14 @@
 #include "base/memory/ptr_util.h"
 #include "base/scoped_observer.h"
 #include "base/strings/stringprintf.h"
+#import "ios/testing/wait_util.h"
 #import "ios/web/public/navigation_item.h"
 #import "ios/web/public/navigation_manager.h"
 #import "ios/web/public/test/fakes/test_native_content.h"
 #import "ios/web/public/test/fakes/test_native_content_provider.h"
 #import "ios/web/public/test/http_server/http_server.h"
 #include "ios/web/public/test/http_server/http_server_util.h"
+#import "ios/web/public/test/navigation_test_util.h"
 #import "ios/web/public/test/web_view_content_test_util.h"
 #import "ios/web/public/test/web_view_interaction_test_util.h"
 #import "ios/web/public/web_client.h"
@@ -92,6 +94,30 @@ ACTION_P3(VerifyNewPageFinishedContext, web_state, url, context) {
   EXPECT_EQ(url, item->GetURL());
 }
 
+// Verifies correctness of |NavigationContext| (|arg1|) for failed navigation
+// passed to |DidFinishNavigation|. Asserts that |NavigationContext| the same as
+// |context|.
+ACTION_P3(VerifyErrorFinishedContext, web_state, url, context) {
+  ASSERT_EQ(*context, arg1);
+  EXPECT_EQ(web_state, arg0);
+  ASSERT_TRUE((*context));
+  EXPECT_EQ(web_state, (*context)->GetWebState());
+  EXPECT_EQ(url, (*context)->GetUrl());
+  EXPECT_TRUE(
+      PageTransitionCoreTypeIs(ui::PageTransition::PAGE_TRANSITION_TYPED,
+                               (*context)->GetPageTransition()));
+  EXPECT_FALSE((*context)->IsSameDocument());
+  EXPECT_FALSE((*context)->IsPost());
+  EXPECT_EQ(NSURLErrorCannotFindHost, (*context)->GetError().code);
+  EXPECT_FALSE((*context)->IsRendererInitiated());
+  EXPECT_FALSE((*context)->GetResponseHeaders());
+  ASSERT_FALSE(web_state->IsLoading());
+  NavigationManager* navigation_manager = web_state->GetNavigationManager();
+  NavigationItem* item = navigation_manager->GetLastCommittedItem();
+  EXPECT_FALSE(item->GetTimestamp().is_null());
+  EXPECT_EQ(url, item->GetURL());
+}
+
 // Verifies correctness of |NavigationContext| (|arg1|) for navigations via POST
 // HTTP methods passed to |DidStartNavigation|. Stores |NavigationContext| in
 // |context| pointer.
@@ -331,6 +357,7 @@ using test::HttpServer;
 using testing::Return;
 using testing::StrictMock;
 using testing::_;
+using testing::WaitUntilConditionOrTimeout;
 using web::test::WaitForWebViewContainingText;
 
 // Test fixture for WebStateDelegate::ProvisionalNavigationStarted,
@@ -391,6 +418,26 @@ TEST_F(NavigationCallbacksTest, NewPageNavigation) {
   LoadUrl(url);
 }
 
+// Tests failed navigation to a new page.
+TEST_F(NavigationCallbacksTest, FailedNavigation) {
+  const GURL url = HttpServer::MakeUrl("unsupported://chromium.test");
+
+  // Perform a navigation to url with unsupported scheme, which will fail.
+  NavigationContext* context = nullptr;
+  EXPECT_CALL(observer_, DidStartLoading(web_state()));
+  EXPECT_CALL(*decider_, ShouldAllowRequest(_, _)).WillOnce(Return(true));
+  EXPECT_CALL(observer_, DidStartNavigation(web_state(), _))
+      .WillOnce(VerifyNewPageStartedContext(web_state(), url, &context));
+  EXPECT_CALL(observer_, DidStopLoading(web_state()));
+  EXPECT_CALL(observer_, DidFinishNavigation(web_state(), _))
+      .WillOnce(VerifyErrorFinishedContext(web_state(), url, &context));
+  // LoadUrl() expects sucessfull load and can not be used here.
+  web::test::LoadUrl(web_state(), url);
+  EXPECT_TRUE(WaitUntilConditionOrTimeout(testing::kWaitForPageLoadTimeout, ^{
+    return !web_state()->IsLoading();
+  }));
+}
+
 // Tests web page reload navigation.
 TEST_F(NavigationCallbacksTest, WebPageReloadNavigation) {
   const GURL url = HttpServer::MakeUrl("http://chromium.test");

ios/web/web_state/ui/crw_web_controller.mm

@@ -1686,12 +1686,18 @@ registerLoadRequestForURL:(const GURL&)requestURL
 
 - (void)loadNativeViewWithSuccess:(BOOL)loadSuccess
                 navigationContext:(web::NavigationContextImpl*)context {
-  _webStateImpl->OnNavigationStarted(context);
+  if (loadSuccess) {
+    // No DidStartNavigation callback for displaying error page.
+    _webStateImpl->OnNavigationStarted(context);
+  }
   const GURL currentURL([self currentURL]);
   [self didStartLoading];
   self.navigationManagerImpl->CommitPendingItem();
   _loadPhase = web::PAGE_LOADED;
-  _webStateImpl->OnNavigationFinished(context);
+  if (loadSuccess) {
+    // No DidFinishNavigation callback for displaying error page.
+    _webStateImpl->OnNavigationFinished(context);
+  }
 
   // Perform post-load-finished updates.
   [self didFinishWithURL:currentURL loadSuccess:loadSuccess];
@@ -2956,6 +2962,10 @@ registerLoadRequestForURL:(const GURL&)requestURL
 
   [self loadCompleteWithSuccess:NO forNavigation:navigation];
   [self loadErrorInNativeView:error navigationContext:navigationContext];
+  if ([_navigationStates stateForNavigation:navigation] ==
+      web::WKNavigationState::PROVISIONALY_FAILED) {
+    _webStateImpl->OnNavigationFinished(navigationContext);
+  }
 }
 
 - (void)handleCancelledError:(NSError*)error {