Small fixes for navigation callbacks.

This CL fixes NavigationContext::IsRendererInitiated for reloads and
WebStatePolicyDecider::RequestInfo::transition_type for
browser-initiated navigation.

Bug: 676129, 725239
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I608b208d204bc055fcfd6c34dd7bd2a04b9c232a
Reviewed-on: https://chromium-review.googlesource.com/c/1282448
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600460}

ios/chrome/browser/ui/payments/payment_request_journey_logger_egtest.mm

@@ -11,7 +11,6 @@
 #import "ios/chrome/test/app/histogram_test_util.h"
 #import "ios/chrome/test/earl_grey/chrome_earl_grey.h"
 #import "ios/chrome/test/earl_grey/chrome_matchers.h"
-#import "ios/web/public/web_client.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -778,19 +777,8 @@ using payments::JourneyLogger;
       buckets[0].min & JourneyLogger::EVENT_RECEIVED_INSTRUMENT_DETAILS, @"");
   GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_SKIPPED_SHOW, @"");
   GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_COMPLETED, @"");
-
-  // TODO(crbug.com/676129): LegacyNavigationManager has a bug that doesn't
-  // create a pending item when reloading the page. This is incorrectly causing
-  // the second navigation above to be considered a renderer-initiated
-  // navigation, and the abort reason is incorrectly logged as
-  // EVENT_OTHER_ABORTED.
-  if (!web::GetWebClient()->IsSlimNavigationManagerEnabled()) {
-    GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_USER_ABORTED, @"");
-    GREYAssertTrue(buckets[0].min & JourneyLogger::EVENT_OTHER_ABORTED, @"");
-  } else {
-    GREYAssertTrue(buckets[0].min & JourneyLogger::EVENT_USER_ABORTED, @"");
-    GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_OTHER_ABORTED, @"");
-  }
+  GREYAssertTrue(buckets[0].min & JourneyLogger::EVENT_USER_ABORTED, @"");
+  GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_OTHER_ABORTED, @"");
   GREYAssertTrue(
       buckets[0].min & JourneyLogger::EVENT_HAD_INITIAL_FORM_OF_PAYMENT, @"");
   GREYAssertTrue(
@@ -840,18 +828,8 @@ using payments::JourneyLogger;
       buckets[0].min & JourneyLogger::EVENT_RECEIVED_INSTRUMENT_DETAILS, @"");
   GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_SKIPPED_SHOW, @"");
   GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_COMPLETED, @"");
-  // TODO(crbug.com/676129): LegacyNavigationManager has a bug that doesn't
-  // create a pending item when reloading the page. This is incorrectly causing
-  // the second navigation above to be considered a renderer-initiated
-  // navigation, and the abort reason is incorrectly logged as
-  // EVENT_OTHER_ABORTED.
-  if (!web::GetWebClient()->IsSlimNavigationManagerEnabled()) {
-    GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_USER_ABORTED, @"");
-    GREYAssertTrue(buckets[0].min & JourneyLogger::EVENT_OTHER_ABORTED, @"");
-  } else {
-    GREYAssertTrue(buckets[0].min & JourneyLogger::EVENT_USER_ABORTED, @"");
-    GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_OTHER_ABORTED, @"");
-  }
+  GREYAssertTrue(buckets[0].min & JourneyLogger::EVENT_USER_ABORTED, @"");
+  GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_OTHER_ABORTED, @"");
   GREYAssertFalse(
       buckets[0].min & JourneyLogger::EVENT_HAD_INITIAL_FORM_OF_PAYMENT, @"");
   GREYAssertFalse(
@@ -902,18 +880,8 @@ using payments::JourneyLogger;
       buckets[0].min & JourneyLogger::EVENT_RECEIVED_INSTRUMENT_DETAILS, @"");
   GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_SKIPPED_SHOW, @"");
   GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_COMPLETED, @"");
-  // TODO(crbug.com/676129): LegacyNavigationManager has a bug that doesn't
-  // create a pending item when reloading the page. This is incorrectly causing
-  // the second navigation above to be considered a renderer-initiated
-  // navigation, and the abort reason is incorrectly logged as
-  // EVENT_OTHER_ABORTED.
-  if (!web::GetWebClient()->IsSlimNavigationManagerEnabled()) {
-    GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_USER_ABORTED, @"");
-    GREYAssertTrue(buckets[0].min & JourneyLogger::EVENT_OTHER_ABORTED, @"");
-  } else {
-    GREYAssertTrue(buckets[0].min & JourneyLogger::EVENT_USER_ABORTED, @"");
-    GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_OTHER_ABORTED, @"");
-  }
+  GREYAssertTrue(buckets[0].min & JourneyLogger::EVENT_USER_ABORTED, @"");
+  GREYAssertFalse(buckets[0].min & JourneyLogger::EVENT_OTHER_ABORTED, @"");
   GREYAssertFalse(
       buckets[0].min & JourneyLogger::EVENT_HAD_INITIAL_FORM_OF_PAYMENT, @"");
   GREYAssertFalse(

ios/web/web_state/ui/crw_web_controller.h

@@ -153,8 +153,11 @@ class WebStateImpl;
 // appropriate, as this method won't display any error to the user.
 - (GURL)currentURLWithTrustLevel:(web::URLVerificationTrustLevel*)trustLevel;
 
-// Methods for navigation and properties to interrogate state.
-- (void)reload;
+// Reloads web view. |isRendererInitiated| is YES for renderer-initiated
+// navigation. |isRendererInitiated| is NO for browser-initiated navigation.
+- (void)reloadWithRendererInitiatedNavigation:(BOOL)isRendererInitiated;
+
+// Stops web view loading.
 - (void)stopLoading;
 
 // Loads the URL indicated by current session state.

ios/web/web_state/ui/crw_web_controller.mm

@@ -2032,7 +2032,7 @@ registerLoadRequestForURL:(const GURL&)requestURL
          !_webStateImpl->HasWebUI();
 }
 
-- (void)reload {
+- (void)reloadWithRendererInitiatedNavigation:(BOOL)isRendererInitiated {
   // Clear last user interaction.
   // TODO(crbug.com/546337): Move to after the load commits, in the subclass
   // implementation. This will be inaccurate if the reload fails or is
@@ -2047,6 +2047,7 @@ registerLoadRequestForURL:(const GURL&)requestURL
                        transition:ui::PageTransition::PAGE_TRANSITION_RELOAD
            sameDocumentNavigation:NO
                    hasUserGesture:YES];
+    navigationContext->SetIsRendererInitiated(isRendererInitiated);
     _webStateImpl->OnNavigationStarted(navigationContext.get());
     [self didStartLoading];
     self.navigationManagerImpl->CommitPendingItem();
@@ -2221,7 +2222,7 @@ registerLoadRequestForURL:(const GURL&)requestURL
     return;
 
   if (delta == 0) {
-    [self reload];
+    [self reloadWithRendererInitiatedNavigation:YES];
     return;
   }
 
@@ -4191,6 +4192,7 @@ registerLoadRequestForURL:(const GURL&)requestURL
                        sameDocumentNavigation:NO
                                hasUserGesture:true];
   }
+  context->SetIsRendererInitiated(false);
   context->SetLoadingHtmlString(true);
   [_navigationStates setContext:std::move(context) forNavigation:navigation];
 }
@@ -4405,15 +4407,23 @@ registerLoadRequestForURL:(const GURL&)requestURL
     }
   }
 
+  ui::PageTransition transition =
+      [self pageTransitionFromNavigationType:action.navigationType];
   BOOL isMainFrameNavigationAction = [self isMainFrameNavigationAction:action];
-  if (base::FeatureList::IsEnabled(web::features::kWebErrorPages) &&
-      isMainFrameNavigationAction) {
+  if (isMainFrameNavigationAction) {
     web::NavigationContextImpl* context =
         [self contextForPendingMainFrameNavigationWithURL:requestURL];
-    if (context && context->IsLoadingErrorPage()) {
-      // loadHTMLString: navigation which loads error page into WKWebView.
-      decisionHandler(WKNavigationActionPolicyAllow);
-      return;
+    if (context) {
+      DCHECK(!context->IsRendererInitiated());
+      transition = context->GetPageTransition();
+
+      if (base::FeatureList::IsEnabled(web::features::kWebErrorPages)) {
+        if (context->IsLoadingErrorPage()) {
+          // loadHTMLString: navigation which loads error page into WKWebView.
+          decisionHandler(WKNavigationActionPolicyAllow);
+          return;
+        }
+      }
     }
   }
 
@@ -4457,8 +4467,6 @@ registerLoadRequestForURL:(const GURL&)requestURL
     return;
   }
 
-  ui::PageTransition transition =
-      [self pageTransitionFromNavigationType:action.navigationType];
   BOOL userInteractedWithRequestMainFrame =
       [self userClickedRecently] &&
       net::GURLWithNSURL(action.request.mainDocumentURL) ==
@@ -5622,6 +5630,8 @@ registerLoadRequestForURL:(const GURL&)requestURL
       WKNavigation* navigation = [self loadPOSTRequest:request];
       [_navigationStates setContext:std::move(navigationContext)
                       forNavigation:navigation];
+      [_navigationStates setState:web::WKNavigationState::REQUESTED
+                    forNavigation:navigation];
       return;
     }
   }
@@ -5635,6 +5645,7 @@ registerLoadRequestForURL:(const GURL&)requestURL
                              transition:self.currentTransition
                  sameDocumentNavigation:sameDocumentNavigation
                          hasUserGesture:YES];
+    navigationContext->SetIsRendererInitiated(false);
     WKNavigation* navigation = [self loadRequest:request];
     [_navigationStates setContext:std::move(navigationContext)
                     forNavigation:navigation];
@@ -5677,6 +5688,7 @@ registerLoadRequestForURL:(const GURL&)requestURL
                              transition:self.currentTransition
                  sameDocumentNavigation:sameDocumentNavigation
                          hasUserGesture:YES];
+    navigationContext->SetIsRendererInitiated(false);
     WKNavigation* navigation = nil;
     if (navigationURL == net::GURLWithNSURL([_webView URL])) {
       navigation = [_webView reload];

ios/web/web_state/web_state_impl.mm

@@ -825,7 +825,7 @@ void WebStateImpl::LoadIfNecessary() {
 }
 
 void WebStateImpl::Reload() {
-  [web_controller_ reload];
+  [web_controller_ reloadWithRendererInitiatedNavigation:NO];
 }
 
 void WebStateImpl::OnNavigationItemsPruned(size_t pruned_item_count) {

ios/web_view/test/navigation_delegate_inttest.mm

@@ -53,10 +53,10 @@ class NavigationDelegateTest : public ios_web_view::WebViewInttestBase {
 
 // Tests that expected delegate methods are called for a successful request.
 TEST_F(NavigationDelegateTest, RequestSucceeds) {
-  // A request made with -loadRequest: has type CWVNavigationTypeClientRedirect.
+  // A request made with -loadRequest: has type CWVNavigationTypeTyped.
   OCMExpect([mock_delegate_ webView:web_view_
                 shouldStartLoadWithRequest:ArgWithURL(GetEchoURL())
-                            navigationType:CWVNavigationTypeClientRedirect])
+                            navigationType:CWVNavigationTypeTyped])
       .andReturn(YES);
   OCMExpect([mock_delegate_ webViewDidStartProvisionalNavigation:web_view_]);
   OCMExpect([mock_delegate_ webView:web_view_
@@ -74,7 +74,7 @@ TEST_F(NavigationDelegateTest, RequestSucceeds) {
 TEST_F(NavigationDelegateTest, RequestFails) {
   OCMExpect([mock_delegate_ webView:web_view_
                 shouldStartLoadWithRequest:ArgWithURL(GetCloseSocketURL())
-                            navigationType:CWVNavigationTypeClientRedirect])
+                            navigationType:CWVNavigationTypeTyped])
       .andReturn(YES);
   OCMExpect([mock_delegate_ webViewDidStartProvisionalNavigation:web_view_]);
   OCMExpect([mock_delegate_ webViewDidCommitNavigation:web_view_]);
@@ -92,7 +92,7 @@ TEST_F(NavigationDelegateTest, RequestFails) {
 TEST_F(NavigationDelegateTest, CancelRequest) {
   OCMExpect([mock_delegate_ webView:web_view_
                 shouldStartLoadWithRequest:ArgWithURL(GetEchoURL())
-                            navigationType:CWVNavigationTypeClientRedirect])
+                            navigationType:CWVNavigationTypeTyped])
       .andReturn(NO);
 
   ASSERT_TRUE(test::LoadUrl(web_view_, GetEchoURL()));
@@ -104,7 +104,7 @@ TEST_F(NavigationDelegateTest, CancelRequest) {
 TEST_F(NavigationDelegateTest, CancelResponse) {
   OCMExpect([mock_delegate_ webView:web_view_
                 shouldStartLoadWithRequest:ArgWithURL(GetEchoURL())
-                            navigationType:CWVNavigationTypeClientRedirect])
+                            navigationType:CWVNavigationTypeTyped])
       .andReturn(YES);
   OCMExpect([mock_delegate_ webViewDidStartProvisionalNavigation:web_view_]);
   OCMExpect([mock_delegate_ webView:web_view_