Remove tracking of MobileTabClobbered in iOS.

Remove the MobileTabClobbered metric from iOS code, since it is unused. The user action will be obsoleted once a follow up CL removes the metric from Android. This CL also simplified an API which requires fewer parameters now that this metric is no longer collected, updated related imports and forward declarations, and fixed the resulting missing imports errors. Bug: 546401 Change-Id: I2387dab4d1376585c7d50a271245792d442a42b5 Reviewed-on: https://chromium-review.googlesource.com/630618 Commit-Queue: Gregory Chatzinoff <gchatz@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Cr-Commit-Position: refs/heads/master@{#497296}
cbchan · Aug 25, 2017 · 0c2d0a6 · 0c2d0a6
1 parent 772dc5f
commit 0c2d0a6
Show file tree

Hide file tree

Showing 16 changed files with 26 additions and 73 deletions.
diff --git a/ios/chrome/browser/prerender/preload_controller.mm b/ios/chrome/browser/prerender/preload_controller.mm
@@ -20,6 +20,7 @@
 #import "ios/chrome/browser/tabs/tab_helper_util.h"
 #import "ios/chrome/browser/tabs/tab_private.h"
 #include "ios/chrome/browser/ui/prerender_final_status.h"
+#import "ios/web/public/navigation_manager.h"
 #import "ios/web/public/web_state/ui/crw_native_content.h"
 #include "ios/web/public/web_thread.h"
 #import "ios/web/web_state/ui/crw_web_controller.h"

diff --git a/ios/chrome/browser/tabs/tab.mm b/ios/chrome/browser/tabs/tab.mm
@@ -864,11 +864,9 @@ - (void)commitCachedEntriesToHistoryDB {
   _addPageVector.clear();
 }
 
-- (void)webDidUpdateSessionForLoadWithParams:
-            (const web::NavigationManager::WebLoadParams&)params
-                        wasInitialNavigation:(BOOL)initialNavigation {
+- (void)webDidUpdateSessionForLoadWithURL:(const GURL&)URL {
   // After a crash the NTP is loaded by default.
-  if (params.url.host() != kChromeUINewTabHost) {
+  if (URL.host() != kChromeUINewTabHost) {
     static BOOL hasLoadedPage = NO;
     if (!hasLoadedPage) {
       // As soon as load is initialted, a crash shouldn't be counted as a
@@ -880,15 +878,6 @@ - (void)webDidUpdateSessionForLoadWithParams:
     }
   }
 
-  ui::PageTransition transition = params.transition_type;
-
-  // Record any explicit, non-redirect navigation as a clobber (as long as it's
-  // in a real tab).
-  if (!initialNavigation && !_isPrerenderTab &&
-      !PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_RELOAD) &&
-      (transition & ui::PAGE_TRANSITION_IS_REDIRECT_MASK) == 0) {
-    base::RecordAction(base::UserMetricsAction("MobileTabClobbered"));
-  }
   if ([_parentTabModel tabUsageRecorder])
     [_parentTabModel tabUsageRecorder]->RecordPageLoadStart(self.webState);
 
@@ -1277,16 +1266,13 @@ - (void)webWillAddPendingURL:(const GURL&)url
 
   BOOL isUserNavigationEvent =
       (transition & ui::PAGE_TRANSITION_IS_REDIRECT_MASK) == 0;
-  // Check for link-follow clobbers. These are changes where there is no
+  // Record start of page load when a user taps a link. A user initiated link
+  // tap is indicated when the page transition is not a redirect, there is no
   // pending entry (since that means the change wasn't caused by this class),
-  // and where the URL changes (to avoid counting page resurrection).
-  // TODO(crbug.com/546401): Consider moving this into NavigationManager, or
-  // into a NavigationManager observer callback, so it doesn't need to be
-  // checked in several places.
+  // and when the URL changes (to avoid counting page resurrection).
   if (isUserNavigationEvent && !_isPrerenderTab &&
       ![self navigationManager]->GetPendingItem() &&
       url != self.lastCommittedURL) {
-    base::RecordAction(base::UserMetricsAction("MobileTabClobbered"));
     if ([_parentTabModel tabUsageRecorder])
       [_parentTabModel tabUsageRecorder]->RecordPageLoadStart(self.webState);
   }

diff --git a/ios/chrome/browser/ui/contextual_search/contextual_search_results_view.mm b/ios/chrome/browser/ui/contextual_search/contextual_search_results_view.mm
@@ -20,6 +20,7 @@
 #import "ios/chrome/browser/web_state_list/web_state_opener.h"
 #import "ios/chrome/common/material_timing.h"
 #include "ios/web/public/load_committed_details.h"
+#import "ios/web/public/navigation_manager.h"
 #import "ios/web/public/serializable_user_data_manager.h"
 #import "ios/web/public/web_state/ui/crw_native_content_provider.h"
 #import "ios/web/public/web_state/ui/crw_web_view_proxy.h"

diff --git a/ios/chrome/browser/ui/toolbar/toolbar_model_impl_ios.mm b/ios/chrome/browser/ui/toolbar/toolbar_model_impl_ios.mm
@@ -12,6 +12,7 @@
 #import "ios/chrome/browser/tabs/legacy_tab_helper.h"
 #include "ios/chrome/browser/tabs/tab.h"
 #include "ios/chrome/browser/ui/toolbar/toolbar_model_delegate_ios.h"
+#import "ios/web/public/navigation_manager.h"
 #import "ios/web/public/web_state/web_state.h"
 #import "ios/web/web_state/ui/crw_web_controller.h"
 

diff --git a/ios/chrome/browser/web_state_list/web_state_list_metrics_observer.h b/ios/chrome/browser/web_state_list/web_state_list_metrics_observer.h
@@ -21,10 +21,6 @@ class WebStateListMetricsObserver : public WebStateListObserver {
                           web::WebState* web_state,
                           int index,
                           bool activating) override;
-  void WebStateReplacedAt(WebStateList* web_state_list,
-                          web::WebState* old_web_state,
-                          web::WebState* new_web_state,
-                          int index) override;
   void WebStateDetachedAt(WebStateList* web_state_list,
                           web::WebState* web_state,
                           int index) override;

diff --git a/ios/chrome/browser/web_state_list/web_state_list_metrics_observer.mm b/ios/chrome/browser/web_state_list/web_state_list_metrics_observer.mm
@@ -42,16 +42,6 @@
   ++inserted_web_state_counter_;
 }
 
-void WebStateListMetricsObserver::WebStateReplacedAt(
-    WebStateList* web_state_list,
-    web::WebState* old_web_state,
-    web::WebState* new_web_state,
-    int index) {
-  // Record a tab clobber, since swapping tabs bypasses the Tab code that would
-  // normally log clobbers.
-  base::RecordAction(base::UserMetricsAction("MobileTabClobbered"));
-}
-
 void WebStateListMetricsObserver::WebStateDetachedAt(
     WebStateList* web_state_list,
     web::WebState* web_state,

diff --git a/ios/web/navigation/navigation_manager_delegate.h b/ios/web/navigation/navigation_manager_delegate.h
@@ -7,9 +7,9 @@
 
 #include <stddef.h>
 
-#import "ios/web/public/navigation_manager.h"
 
 @protocol CRWWebViewNavigationProxy;
+class GURL;
 
 namespace web {
 
@@ -40,9 +40,7 @@ class NavigationManagerDelegate {
 
   // Instructs the delegate to notify its delegates that the current navigation
   // item will be loaded.
-  virtual void WillLoadCurrentItemWithParams(
-      const NavigationManager::WebLoadParams&,
-      bool is_initial_navigation) = 0;
+  virtual void WillLoadCurrentItemWithUrl(const GURL&) = 0;
 
   // Instructs the delegate to load the current navigation item.
   virtual void LoadCurrentItem() = 0;

diff --git a/ios/web/navigation/navigation_manager_impl.mm b/ios/web/navigation/navigation_manager_impl.mm
@@ -206,8 +206,6 @@
   delegate_->ClearTransientContent();
   delegate_->RecordPageStateInNavigationItem();
 
-  bool is_initial_navigation = !GetItemCount();
-
   NavigationInitiationType initiation_type =
       params.is_renderer_initiated
           ? NavigationInitiationType::RENDERER_INITIATED
@@ -249,7 +247,7 @@
     added_item->SetShouldSkipRepostFormConfirmation(true);
   }
 
-  delegate_->WillLoadCurrentItemWithParams(params, is_initial_navigation);
+  delegate_->WillLoadCurrentItemWithUrl(params.url);
   delegate_->LoadCurrentItem();
 }
 

diff --git a/ios/web/navigation/navigation_manager_impl_unittest.mm b/ios/web/navigation/navigation_manager_impl_unittest.mm
@@ -73,8 +73,7 @@ void SetSessionController(CRWSessionController* session_controller) {
   MOCK_METHOD0(ClearTransientContent, void());
   MOCK_METHOD0(RecordPageStateInNavigationItem, void());
   MOCK_METHOD0(UpdateHtml5HistoryState, void());
-  MOCK_METHOD2(WillLoadCurrentItemWithParams,
-               void(const NavigationManager::WebLoadParams&, bool));
+  MOCK_METHOD1(WillLoadCurrentItemWithUrl, void(const GURL&));
   MOCK_METHOD0(WillChangeUserAgentType, void());
   MOCK_METHOD0(LoadCurrentItem, void());
   MOCK_METHOD0(LoadIfNecessary, void());
@@ -2137,8 +2136,7 @@ GURL rewritten_url4(
       .Times(1);
   EXPECT_CALL(navigation_manager_delegate(), ClearTransientContent()).Times(1);
   EXPECT_CALL(navigation_manager_delegate(),
-              WillLoadCurrentItemWithParams(::testing::Ref(params),
-                                            true /* is_initial_navigation */));
+              WillLoadCurrentItemWithUrl(::testing::Ref(params.url)));
   EXPECT_CALL(navigation_manager_delegate(), LoadCurrentItem()).Times(1);
 
   navigation_manager()->LoadURLWithParams(params);
@@ -2171,8 +2169,7 @@ GURL rewritten_url4(
       .Times(1);
   EXPECT_CALL(navigation_manager_delegate(), ClearTransientContent()).Times(1);
   EXPECT_CALL(navigation_manager_delegate(),
-              WillLoadCurrentItemWithParams(::testing::Ref(params),
-                                            false /* is_initial_navigation */));
+              WillLoadCurrentItemWithUrl(::testing::Ref(params.url)));
   EXPECT_CALL(navigation_manager_delegate(), LoadCurrentItem()).Times(1);
 
   navigation_manager()->LoadURLWithParams(params);

diff --git a/ios/web/public/web_state/ui/crw_web_delegate.h b/ios/web/public/web_state/ui/crw_web_delegate.h
@@ -11,7 +11,6 @@
 
 #import "base/ios/block_types.h"
 #include "ios/web/public/favicon_url.h"
-#import "ios/web/public/navigation_manager.h"
 #include "ios/web/public/ssl_status.h"
 #import "ios/web/public/web_state/ui/crw_native_content.h"
 #import "ios/web/public/web_state/web_state.h"
@@ -51,9 +50,7 @@ class GURL;
 // less, then remove any delegate method that becomes unnecessary as a result.
 
 // Called when a page is loaded using loadWithParams.
-- (void)webDidUpdateSessionForLoadWithParams:
-            (const web::NavigationManager::WebLoadParams&)params
-                        wasInitialNavigation:(BOOL)initialNavigation;
+- (void)webDidUpdateSessionForLoadWithURL:(const GURL&)URL;
 
 @optional
 

diff --git a/ios/web/test/fakes/test_navigation_manager_delegate.h b/ios/web/test/fakes/test_navigation_manager_delegate.h
@@ -7,6 +7,7 @@
 
 #import "ios/web/navigation/navigation_manager_delegate.h"
 
+class GURL;
 @protocol CRWWebViewNavigationProxy;
 
 namespace web {
@@ -16,8 +17,7 @@ class TestNavigationManagerDelegate : public NavigationManagerDelegate {
   void ClearTransientContent() override;
   void RecordPageStateInNavigationItem() override;
   void UpdateHtml5HistoryState() override;
-  void WillLoadCurrentItemWithParams(const NavigationManager::WebLoadParams&,
-                                     bool is_initial_navigation) override;
+  void WillLoadCurrentItemWithUrl(const GURL&) override;
   void WillChangeUserAgentType() override;
   void LoadCurrentItem() override;
   void LoadIfNecessary() override;

diff --git a/ios/web/test/fakes/test_navigation_manager_delegate.mm b/ios/web/test/fakes/test_navigation_manager_delegate.mm
@@ -4,6 +4,7 @@
 
 #import "ios/web/test/fakes/test_navigation_manager_delegate.h"
 #import "ios/web/web_state/ui/crw_web_view_navigation_proxy.h"
+#include "url/gurl.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -14,9 +15,7 @@
 void TestNavigationManagerDelegate::ClearTransientContent() {}
 void TestNavigationManagerDelegate::RecordPageStateInNavigationItem() {}
 void TestNavigationManagerDelegate::UpdateHtml5HistoryState() {}
-void TestNavigationManagerDelegate::WillLoadCurrentItemWithParams(
-    const NavigationManager::WebLoadParams&,
-    bool is_initial_navigation) {}
+void TestNavigationManagerDelegate::WillLoadCurrentItemWithUrl(const GURL&) {}
 void TestNavigationManagerDelegate::WillChangeUserAgentType() {}
 void TestNavigationManagerDelegate::LoadCurrentItem() {}
 void TestNavigationManagerDelegate::LoadIfNecessary() {}

diff --git a/ios/web/web_state/ui/crw_web_controller.h b/ios/web/web_state/ui/crw_web_controller.h
@@ -8,7 +8,6 @@
 #import <UIKit/UIKit.h>
 
 #import "ios/web/net/crw_request_tracker_delegate.h"
-#import "ios/web/public/navigation_manager.h"
 #import "ios/web/public/web_state/js/crw_js_injection_evaluator.h"
 #import "ios/web/public/web_state/ui/crw_web_delegate.h"
 #include "ios/web/public/web_state/url_verification_constants.h"
@@ -155,13 +154,10 @@ class WebStateImpl;
 // to generate an overlay placeholder view.
 - (BOOL)canUseViewForGeneratingOverlayPlaceholderView;
 
-// Notifies delegate that |currentNavItem| will be loaded with |params|. If this
-// is the first navigation, |isInitialNavigation| is YES.
+// Notifies delegate that |currentNavItem| will be loaded with |url|.
 // TODO(crbug.com/674991): Remove this method when CRWWebDelegate is no longer
 // used.
-- (void)willLoadCurrentItemWithParams:
-            (const web::NavigationManager::WebLoadParams&)params
-                  isInitialNavigation:(BOOL)isInitialNavigation;
+- (void)willLoadCurrentItemWithURL:(const GURL&)URL;
 
 // Loads the URL indicated by current session state.
 - (void)loadCurrentURL;

diff --git a/ios/web/web_state/ui/crw_web_controller.mm b/ios/web/web_state/ui/crw_web_controller.mm
@@ -1755,11 +1755,8 @@ - (void)loadCurrentURLInNativeView {
                 navigationContext:navigationContext.get()];
 }
 
-- (void)willLoadCurrentItemWithParams:
-            (const NavigationManager::WebLoadParams&)params
-                  isInitialNavigation:(BOOL)isInitialNavigation {
-  [_delegate webDidUpdateSessionForLoadWithParams:params
-                             wasInitialNavigation:isInitialNavigation];
+- (void)willLoadCurrentItemWithURL:(const GURL&)URL {
+  [_delegate webDidUpdateSessionForLoadWithURL:URL];
 }
 
 - (void)loadCurrentURL {

diff --git a/ios/web/web_state/web_state_impl.h b/ios/web/web_state/web_state_impl.h
@@ -264,8 +264,7 @@ class WebStateImpl : public WebState, public NavigationManagerDelegate {
   void RecordPageStateInNavigationItem() override;
   void UpdateHtml5HistoryState() override;
   void WillChangeUserAgentType() override;
-  void WillLoadCurrentItemWithParams(const NavigationManager::WebLoadParams&,
-                                     bool is_initial_navigation) override;
+  void WillLoadCurrentItemWithUrl(const GURL&) override;
   void LoadCurrentItem() override;
   void LoadIfNecessary() override;
   void Reload() override;

diff --git a/ios/web/web_state/web_state_impl.mm b/ios/web/web_state/web_state_impl.mm
@@ -749,11 +749,8 @@
   [web_controller_ requirePageReconstruction];
 }
 
-void WebStateImpl::WillLoadCurrentItemWithParams(
-    const NavigationManager::WebLoadParams& params,
-    bool is_initial_navigation) {
-  [web_controller_ willLoadCurrentItemWithParams:params
-                             isInitialNavigation:is_initial_navigation];
+void WebStateImpl::WillLoadCurrentItemWithUrl(const GURL& url) {
+  [web_controller_ willLoadCurrentItemWithURL:url];
 }
 
 void WebStateImpl::LoadCurrentItem() {