Improve page transition type heuristic in WebController

ios/web/web_state/ui/crw_wk_web_view_web_controller.mm

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ios/web/web_state/ui/crw_wk_web_view_web_controller.h"
 
 #import <WebKit/WebKit.h>
 
 #include "base/ios/ios_util.h"
 #include "base/ios/weak_nsobject.h"
@@ skipping 126 matching lines @@
   // bad SSL cert, presenting SSL interstitials and determining SSL status for
   // Navigation Items.
   base::scoped_nsobject<CRWCertVerificationController>
       _certVerificationController;
 
   // Whether the pending navigation has been directly cancelled in
   // |decidePolicyForNavigationAction| or |decidePolicyForNavigationResponse|.
   // Cancelled navigations should be simply discarded without handling any
   // specific error.
   BOOL _pendingNavigationCancelled;
+
+  // YES if the user has touched the content area since the last URL change.
+  BOOL _touchedSinceLastURLChange;
 }
 
 // Response's MIME type of the last known navigation.
 @property(nonatomic, copy) NSString* documentMIMEType;
 
 // Dictionary where keys are the names of WKWebView properties and values are
 // selector names which should be called when a corresponding property has
 // changed. e.g. @{ @"URL" : @"webViewURLDidChange" } means that
 // -[self webViewURLDidChange] must be called every time when WKWebView.URL is
 // changed.
@@ skipping 204 matching lines @@
   // TODO(eugenebut): implement dialogs/windows suppression using
   // WKNavigationDelegate methods where possible.
   [super setPageDialogOpenPolicy:policy];
 }
 
 - (void)close {
   [_certVerificationController shutDown];
   [super close];
 }
 
+- (void)touched:(BOOL)touched {
+  [super touched:touched];
+  if (touched)
+    _touchedSinceLastURLChange = YES;
+}
+
 #pragma mark -
 #pragma mark Testing-Only Methods
 
 - (void)injectWebViewContentView:(CRWWebViewContentView*)webViewContentView {
   [super injectWebViewContentView:webViewContentView];
   [self setWebView:static_cast<WKWebView*>(webViewContentView.webView)];
 }
 
 #pragma mark - Protected property implementations
 
@@ skipping 626 matching lines @@
   }
 
   if (!previousSSLStatus.Equals(item->GetSSL())) {
     [self didUpdateSSLStatusForCurrentNavigationItem];
   }
 }
 
 #endif  // !defined(ENABLE_CHROME_NET_STACK_FOR_WKWEBVIEW)
 
 - (void)registerLoadRequest:(const GURL&)url {
-  // If load request is registered via WKWebViewWebController, assume transition
-  // is link or client redirect as other transitions will already be registered
-  // by web controller or delegates.
-  // TODO(stuartmorgan): Remove guesswork and replace with information from
-  // decidePolicyForNavigationAction:.
-  ui::PageTransition transition = self.userInteractionRegistered
-                                      ? ui::PAGE_TRANSITION_LINK
-                                      : ui::PAGE_TRANSITION_CLIENT_REDIRECT;
+  // Get the navigation type from the last main frame load request, and try to
+  // map that to a PageTransition.
+  WKNavigationType navigationType = _pendingNavigationTypeForMainFrame
+                                        ? *_pendingNavigationTypeForMainFrame
+                                        : WKNavigationTypeOther;
+  ui::PageTransition transition = ui::PAGE_TRANSITION_CLIENT_REDIRECT;
+  switch (navigationType) {
+    case WKNavigationTypeLinkActivated:
+      transition = ui::PAGE_TRANSITION_LINK;
+      break;
+    case WKNavigationTypeFormSubmitted:
+    case WKNavigationTypeFormResubmitted:
+      transition = ui::PAGE_TRANSITION_FORM_SUBMIT;
+      break;
+    case WKNavigationTypeBackForward:
+      transition = ui::PAGE_TRANSITION_FORWARD_BACK;
+      break;
+    case WKNavigationTypeReload:
+      transition = ui::PAGE_TRANSITION_RELOAD;
+      break;
+    case WKNavigationTypeOther:
+      // The "Other" type covers a variety of very different cases, which may
+      // or may not be the result of user actions. For now, guess based on
+      // whether there's been a touch since the last URL change.
+      // TODO(crbug.com/549301): See if this heuristic can be improved.
+      transition = _touchedSinceLastURLChange
+                       ? ui::PAGE_TRANSITION_LINK
+                       : ui::PAGE_TRANSITION_CLIENT_REDIRECT;
+      break;
+  }
   // The referrer is not known yet, and will be updated later.
   const web::Referrer emptyReferrer;
   [self registerLoadRequest:url referrer:emptyReferrer transition:transition];
 }
 
 - (void)URLDidChangeWithoutDocumentChange:(const GURL&)newURL {
   DCHECK(newURL == net::GURLWithNSURL([_wkWebView URL]));
   DCHECK_EQ(_documentURL.host(), newURL.host());
   _documentURL = newURL;
   // If called during window.history.pushState or window.history.replaceState
@@ skipping 184 matching lines @@
 }
 
 - (void)webViewURLDidChange {
   // TODO(stuartmorgan): Determine if there are any cases where this still
   // happens, and if so whether anything should be done when it does.
   if (![_wkWebView URL]) {
     DVLOG(1) << "Received nil URL callback";
     return;
   }
   GURL url(net::GURLWithNSURL([_wkWebView URL]));
+  if (url != _documentURL)
+    _touchedSinceLastURLChange = NO;

[Eugene But (OOO till 7-30), 2015/10/30 15:10:47]

This is not the only place where _documentURL is changed. How about adding
- (void)setDocumentURL: (const GURL&)newURL {
  if (_documentURL != newURL) {
    _documentURL = newURL;
    _touchedSinceLastURLChange = NO;
  }
}

and calling it everywhere instead of assigning value to ivar?

[stuartmorgan, 2015/10/30 16:00:01]

Originally I didn't do that because of the async code in URL change detection
below, but on further consideration keeping the relative ordering makes sense,
and this is a less fragile approach. Done.

   // URL changes happen at three points:
   // 1) When a load starts; at this point, the load is provisional, and
   //    it should be ignored until it's committed, since the document/window
   //    objects haven't changed yet.
   // 2) When a non-document-changing URL change happens (hash change,
   //    history.pushState, etc.). This URL change happens instantly, so should
   //    be reported.
   // 3) When a navigation error occurs after provisional navigation starts,
   //    the URL reverts to the previous URL without triggering a new navigation.
   //
@@ skipping 351 matching lines @@
                               placeholderText:defaultText
                                    requestURL:
             net::GURLWithNSURL(frame.request.URL)
                             completionHandler:completionHandler];
   } else if (completionHandler) {
     completionHandler(nil);
   }
 }
 
 @end