Move didReceiveWebViewNavigationDelegateCallback into

CRWWKNavigationHandler.

CRWWebController.didReceiveWebViewNavigationDelegateCallback is called
when it receives any callback from WKWebView. This CL moves it into
CRWWKNavigationHandler and delegates all WKNavigationDelegate callbacks
to CRWWKNavigationHandler. This CL also makes
CRWWebController._isBeingDestroyed visible to CRWWKNavigationHandler
through CRWWKNavigationHandlerDelegate protocol.

Bug: 956511
Change-Id: Iecb377c603eb62e3a7265149b9394c8a2310c3ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1599013
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659422}

ios/web/navigation/crw_wk_navigation_handler.h

@@ -10,6 +10,7 @@
 
 @class CRWPendingNavigationInfo;
 @class CRWWKNavigationStates;
+@class CRWWKNavigationHandler;
 
 namespace base {
 class RepeatingTimer;
@@ -18,12 +19,18 @@ class RepeatingTimer;
 // CRWWKNavigationHandler uses this protocol to interact with its owner.
 @protocol CRWWKNavigationHandlerDelegate <NSObject>
 
+// Returns YES if WKWebView was deallocated or is being deallocated.
+- (BOOL)navigationHandlerWebViewBeingDestroyed:
+    (CRWWKNavigationHandler*)navigationHandler;
+
 @end
 
 // Handler class for WKNavigationDelegate, deals with navigation callbacks from
 // WKWebView and maintains page loading state.
 @interface CRWWKNavigationHandler : NSObject <WKNavigationDelegate>
 
+@property(nonatomic, weak) id<CRWWKNavigationHandlerDelegate> delegate;
+
 // Pending information for an in-progress page navigation. The lifetime of
 // this object starts at |decidePolicyForNavigationAction| where the info is
 // extracted from the request, and ends at either |didCommitNavigation| or

ios/web/navigation/crw_wk_navigation_handler.mm

@@ -4,6 +4,7 @@
 
 #import "ios/web/navigation/crw_wk_navigation_handler.h"
 
+#include "base/metrics/histogram_macros.h"
 #include "base/timer/timer.h"
 #import "ios/web/navigation/crw_pending_navigation_info.h"
 #import "ios/web/navigation/crw_wk_navigation_states.h"
@@ -27,6 +28,80 @@
   return self;
 }
 
+#pragma mark - WKNavigationDelegate
+
+- (void)webView:(WKWebView*)webView
+    decidePolicyForNavigationAction:(WKNavigationAction*)action
+                    decisionHandler:
+                        (void (^)(WKNavigationActionPolicy))decisionHandler {
+  [self didReceiveWKNavigationDelegateCallback];
+}
+
+- (void)webView:(WKWebView*)webView
+    decidePolicyForNavigationResponse:(WKNavigationResponse*)WKResponse
+                      decisionHandler:
+                          (void (^)(WKNavigationResponsePolicy))handler {
+  [self didReceiveWKNavigationDelegateCallback];
+}
+
+- (void)webView:(WKWebView*)webView
+    didStartProvisionalNavigation:(WKNavigation*)navigation {
+  [self didReceiveWKNavigationDelegateCallback];
+}
+
+- (void)webView:(WKWebView*)webView
+    didReceiveServerRedirectForProvisionalNavigation:(WKNavigation*)navigation {
+  [self didReceiveWKNavigationDelegateCallback];
+}
+
+- (void)webView:(WKWebView*)webView
+    didFailProvisionalNavigation:(WKNavigation*)navigation
+                       withError:(NSError*)error {
+  [self didReceiveWKNavigationDelegateCallback];
+}
+
+- (void)webView:(WKWebView*)webView
+    didCommitNavigation:(WKNavigation*)navigation {
+  [self didReceiveWKNavigationDelegateCallback];
+}
+
+- (void)webView:(WKWebView*)webView
+    didFinishNavigation:(WKNavigation*)navigation {
+  [self didReceiveWKNavigationDelegateCallback];
+}
+
+- (void)webView:(WKWebView*)webView
+    didFailNavigation:(WKNavigation*)navigation
+            withError:(NSError*)error {
+  [self didReceiveWKNavigationDelegateCallback];
+}
+
+- (void)webView:(WKWebView*)webView
+    didReceiveAuthenticationChallenge:(NSURLAuthenticationChallenge*)challenge
+                    completionHandler:
+                        (void (^)(NSURLSessionAuthChallengeDisposition,
+                                  NSURLCredential*))completionHandler {
+  [self didReceiveWKNavigationDelegateCallback];
+}
+
+- (void)webViewWebContentProcessDidTerminate:(WKWebView*)webView {
+  [self didReceiveWKNavigationDelegateCallback];
+}
+
+#pragma mark - Private methods
+
+// This method should be called on receiving WKNavigationDelegate callbacks. It
+// will log a metric if the callback occurs after the reciever has already been
+// closed. It also stops the SafeBrowsing warning detection timer, since after
+// this point it's too late for a SafeBrowsing warning to be displayed for the
+// navigation for which the timer was started.
+- (void)didReceiveWKNavigationDelegateCallback {
+  if ([self.delegate navigationHandlerWebViewBeingDestroyed:self]) {
+    UMA_HISTOGRAM_BOOLEAN("Renderer.WKWebViewCallbackAfterDestroy", true);
+  }
+  _safeBrowsingWarningDetectionTimer.Stop();
+}
+
 #pragma mark - Public methods
 
 - (void)stopLoading {

ios/web/web_state/ui/crw_web_controller.mm

@@ -229,6 +229,7 @@ bool RequiresContentFilterBlockingWorkaround() {
 }  // namespace
 
 @interface CRWWebController () <BrowsingDataRemoverObserver,
+                                CRWWKNavigationHandlerDelegate,
                                 CRWContextMenuDelegate,
                                 CRWNativeContentDelegate,
                                 CRWSSLStatusUpdaterDataSource,
@@ -331,6 +332,8 @@ bool RequiresContentFilterBlockingWorkaround() {
 // The WKNavigationDelegate handler class.
 @property(nonatomic, readonly, strong)
     CRWWKNavigationHandler* navigationHandler;
+// YES if in the process of closing.
+@property(nonatomic, readwrite, assign) BOOL beingDestroyed;
 
 // If |contentView_| contains a web view, this is the web view it contains.
 // If not, it's nil. When setting the property, it performs basic setup.
@@ -617,6 +620,7 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
              object:nil];
 
     _navigationHandler = [[CRWWKNavigationHandler alloc] init];
+    _navigationHandler.delegate = self;
   }
   return self;
 }
@@ -4039,7 +4043,9 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
     decidePolicyForNavigationAction:(WKNavigationAction*)action
                     decisionHandler:
                         (void (^)(WKNavigationActionPolicy))decisionHandler {
-  [self didReceiveWebViewNavigationDelegateCallback];
+  [self.navigationHandler webView:webView
+      decidePolicyForNavigationAction:action
+                      decisionHandler:decisionHandler];
 
   _webProcessCrashed = NO;
   if (_isBeingDestroyed) {
@@ -4354,7 +4360,9 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
     decidePolicyForNavigationResponse:(WKNavigationResponse*)WKResponse
                       decisionHandler:
                           (void (^)(WKNavigationResponsePolicy))handler {
-  [self didReceiveWebViewNavigationDelegateCallback];
+  [self.navigationHandler webView:webView
+      decidePolicyForNavigationResponse:WKResponse
+                        decisionHandler:handler];
 
   // If this is a placeholder navigation, pass through.
   GURL responseURL = net::GURLWithNSURL(WKResponse.response.URL);
@@ -4416,7 +4424,8 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
 
 - (void)webView:(WKWebView*)webView
     didStartProvisionalNavigation:(WKNavigation*)navigation {
-  [self didReceiveWebViewNavigationDelegateCallback];
+  [self.navigationHandler webView:webView
+      didStartProvisionalNavigation:navigation];
 
   GURL webViewURL = net::GURLWithNSURL(webView.URL);
 
@@ -4534,7 +4543,8 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
 
 - (void)webView:(WKWebView*)webView
     didReceiveServerRedirectForProvisionalNavigation:(WKNavigation*)navigation {
-  [self didReceiveWebViewNavigationDelegateCallback];
+  [self.navigationHandler webView:webView
+      didReceiveServerRedirectForProvisionalNavigation:navigation];
 
   GURL webViewURL = net::GURLWithNSURL(webView.URL);
 
@@ -4554,7 +4564,10 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
 - (void)webView:(WKWebView*)webView
     didFailProvisionalNavigation:(WKNavigation*)navigation
                        withError:(NSError*)error {
-  [self didReceiveWebViewNavigationDelegateCallback];
+  [self.navigationHandler webView:webView
+      didFailProvisionalNavigation:navigation
+                         withError:error];
+
   [self.navigationHandler.navigationStates
            setState:web::WKNavigationState::PROVISIONALY_FAILED
       forNavigation:navigation];
@@ -4622,7 +4635,7 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
 
 - (void)webView:(WKWebView*)webView
     didCommitNavigation:(WKNavigation*)navigation {
-  [self didReceiveWebViewNavigationDelegateCallback];
+  [self.navigationHandler webView:webView didCommitNavigation:navigation];
 
   // For reasons not yet fully understood, sometimes WKWebView triggers
   // |webView:didFinishNavigation| before |webView:didCommitNavigation|. If a
@@ -4844,7 +4857,7 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
 
 - (void)webView:(WKWebView*)webView
     didFinishNavigation:(WKNavigation*)navigation {
-  [self didReceiveWebViewNavigationDelegateCallback];
+  [self.navigationHandler webView:webView didFinishNavigation:navigation];
 
   // Sometimes |webView:didFinishNavigation| arrives before
   // |webView:didCommitNavigation|. Explicitly trigger post-commit processing.
@@ -4996,7 +5009,9 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
 - (void)webView:(WKWebView*)webView
     didFailNavigation:(WKNavigation*)navigation
             withError:(NSError*)error {
-  [self didReceiveWebViewNavigationDelegateCallback];
+  [self.navigationHandler webView:webView
+                didFailNavigation:navigation
+                        withError:error];
 
   [self.navigationHandler.navigationStates
            setState:web::WKNavigationState::FAILED
@@ -5014,7 +5029,9 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
                     completionHandler:
                         (void (^)(NSURLSessionAuthChallengeDisposition,
                                   NSURLCredential*))completionHandler {
-  [self didReceiveWebViewNavigationDelegateCallback];
+  [self.navigationHandler webView:webView
+      didReceiveAuthenticationChallenge:challenge
+                      completionHandler:completionHandler];
 
   NSString* authMethod = challenge.protectionSpace.authenticationMethod;
   if ([authMethod isEqual:NSURLAuthenticationMethodHTTPBasic] ||
@@ -5053,7 +5070,7 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
 }
 
 - (void)webViewWebContentProcessDidTerminate:(WKWebView*)webView {
-  [self didReceiveWebViewNavigationDelegateCallback];
+  [self.navigationHandler webViewWebContentProcessDidTerminate:webView];
 
   _certVerificationErrors->Clear();
   [self removeAllWebFrames];
@@ -5387,18 +5404,6 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
   return provisionalLoad;
 }
 
-// This method should be called on receiving WKNavigationDelegate callbacks. It
-// will log a metric if the callback occurs after the reciever has already been
-// closed. It also stops the SafeBrowsing warning detection timer, since after
-// this point it's too late for a SafeBrowsing warning to be displayed for the
-// navigation for which the timer was started.
-- (void)didReceiveWebViewNavigationDelegateCallback {
-  if (_isBeingDestroyed) {
-    UMA_HISTOGRAM_BOOLEAN("Renderer.WKWebViewCallbackAfterDestroy", true);
-  }
-  self.navigationHandler.safeBrowsingWarningDetectionTimer->Stop();
-}
-
 // Returns YES if response should be rendered in WKWebView.
 - (BOOL)shouldRenderResponse:(WKNavigationResponse*)WKResponse {
   if (!WKResponse.canShowMIMEType) {
@@ -6097,6 +6102,13 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
   }
 }
 
+#pragma mark - CRWWKNavigationHandlerDelegate
+
+- (BOOL)navigationHandlerWebViewBeingDestroyed:
+    (CRWWKNavigationHandler*)navigationHandler {
+  return _beingDestroyed;
+}
+
 #pragma mark - Testing-Only Methods
 
 - (void)injectWebViewContentView:(CRWWebViewContentView*)webViewContentView {