[iOS][tabmodel] Stop observing webstates.

As a tabmodel needs to be deprecated, this CL moves all
CRWWebStateObserver functions into a web state list metrics browser agent
and omnibox geolocation tab helper created here to move them out of
TabModel.

Bug: 1050116,1045575
Change-Id: I9b0f14c6fc89b5d7304a2cd2e5b5bd0af44838b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2289856
Commit-Queue: Nazerke Kalidolda <nazerke@google.com>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801855}

ios/chrome/browser/geolocation/BUILD.gn

@@ -18,6 +18,7 @@ source_set("geolocation") {
     "omnibox_geolocation_config.mm",
     "omnibox_geolocation_local_state.h",
     "omnibox_geolocation_local_state.mm",
+    "omnibox_geolocation_tab_helper.h",
   ]
   deps = [
     "//base",
@@ -28,6 +29,7 @@ source_set("geolocation") {
     "//ios/chrome/browser",
     "//ios/chrome/browser/ui/util",
     "//ios/public/provider/chrome/browser",
+    "//ios/web/public",
     "//ui/base",
     "//url",
   ]
@@ -72,6 +74,7 @@ source_set("geolocation_internal") {
     "omnibox_geolocation_controller+Testing.h",
     "omnibox_geolocation_controller.h",
     "omnibox_geolocation_controller.mm",
+    "omnibox_geolocation_tab_helper.mm",
   ]
   deps = [
     ":geolocation",
@@ -79,8 +82,10 @@ source_set("geolocation_internal") {
     "//components/google/core/common",
     "//components/version_info",
     "//ios/chrome/browser/browser_state",
+    "//ios/chrome/browser/browser_state",
     "//ios/chrome/browser/tabs",
     "//ios/web",
+    "//ios/web/public/navigation",
     "//ui/base",
     "//url",
   ]

ios/chrome/browser/geolocation/omnibox_geolocation_tab_helper.h

+// Copyright 2020 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.
+
+#ifndef IOS_CHROME_BROWSER_GEOLOCATION_OMNIBOX_GEOLOCATION_TAB_HELPER_H_
+#define IOS_CHROME_BROWSER_GEOLOCATION_OMNIBOX_GEOLOCATION_TAB_HELPER_H_
+
+#include "ios/web/public/web_state_observer.h"
+#import "ios/web/public/web_state_user_data.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+// A tab helper which handles the current device location for omnibox search
+// queries.
+class OmniboxGeolocationTabHelper
+    : public web::WebStateObserver,
+      public web::WebStateUserData<OmniboxGeolocationTabHelper> {
+ public:
+  // Not copyable or moveable
+  OmniboxGeolocationTabHelper(const OmniboxGeolocationTabHelper&) = delete;
+  OmniboxGeolocationTabHelper& operator=(const OmniboxGeolocationTabHelper&) =
+      delete;
+  ~OmniboxGeolocationTabHelper() override;
+
+ private:
+  friend class web::WebStateUserData<OmniboxGeolocationTabHelper>;
+
+  explicit OmniboxGeolocationTabHelper(web::WebState* web_state);
+
+  // web::WebStateObserver
+  void DidStartNavigation(web::WebState* web_state,
+                          web::NavigationContext* navigation_context) override;
+  void PageLoaded(
+      web::WebState* web_state,
+      web::PageLoadCompletionStatus load_completion_status) override;
+  void WebStateDestroyed(web::WebState* web_state) override;
+
+  web::WebState* web_state_ = nullptr;
+
+  WEB_STATE_USER_DATA_KEY_DECL();
+};
+
+#endif  // IOS_CHROME_BROWSER_GEOLOCATION_OMNIBOX_GEOLOCATION_TAB_HELPER_H_

ios/chrome/browser/geolocation/omnibox_geolocation_tab_helper.mm

+// Copyright 2020 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/chrome/browser/geolocation/omnibox_geolocation_tab_helper.h"
+
+#import "ios/chrome/browser/browser_state/chrome_browser_state.h"
+#import "ios/chrome/browser/geolocation/omnibox_geolocation_controller.h"
+#import "ios/web/public/navigation/navigation_context.h"
+#import "ios/web/public/navigation/navigation_item.h"
+#import "ios/web/public/navigation/navigation_manager.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+OmniboxGeolocationTabHelper::OmniboxGeolocationTabHelper(
+    web::WebState* web_state)
+    : web_state_(web_state) {
+  web_state_->AddObserver(this);
+}
+
+OmniboxGeolocationTabHelper::~OmniboxGeolocationTabHelper() {
+  if (web_state_) {
+    web_state_->RemoveObserver(this);
+    web_state_ = nullptr;
+  }
+}
+
+void OmniboxGeolocationTabHelper::DidStartNavigation(
+    web::WebState* web_state,
+    web::NavigationContext* navigation_context) {
+  DCHECK(web_state->GetNavigationManager());
+  web::NavigationItem* navigation_item =
+      web_state->GetNavigationManager()->GetPendingItem();
+
+  if (!navigation_item) {
+    // Pending item may not exist due to the bug in //ios/web layer.
+    // TODO(crbug.com/899827): remove this early return once GetPendingItem()
+    // always return valid object inside WebStateObserver::DidStartNavigation()
+    // callback.
+    //
+    // Note that GetLastCommittedItem() returns null if navigation manager does
+    // not have committed items (which is normal situation).
+    return;
+  }
+
+  [[OmniboxGeolocationController sharedInstance]
+      addLocationToNavigationItem:navigation_item
+                     browserState:ChromeBrowserState::FromBrowserState(
+                                      web_state->GetBrowserState())];
+}
+
+void OmniboxGeolocationTabHelper::PageLoaded(
+    web::WebState* web_state,
+    web::PageLoadCompletionStatus load_completion_status) {
+  [[OmniboxGeolocationController sharedInstance]
+      finishPageLoadForWebState:web_state
+                    loadSuccess:(load_completion_status ==
+                                 web::PageLoadCompletionStatus::SUCCESS)];
+}
+
+void OmniboxGeolocationTabHelper::WebStateDestroyed(web::WebState* web_state) {
+  web_state->RemoveObserver(this);
+  web_state_ = nullptr;
+}
+
+WEB_STATE_USER_DATA_KEY_IMPL(OmniboxGeolocationTabHelper)

ios/chrome/browser/tabs/BUILD.gn

@@ -69,6 +69,7 @@ source_set("tabs_internal") {
     "//ios/chrome/browser/download",
     "//ios/chrome/browser/favicon",
     "//ios/chrome/browser/find_in_page",
+    "//ios/chrome/browser/geolocation",
     "//ios/chrome/browser/geolocation:geolocation_internal",
     "//ios/chrome/browser/history",
     "//ios/chrome/browser/history:tab_helper",

ios/chrome/browser/tabs/tab_helper_util.mm

@@ -30,6 +30,7 @@
 #import "ios/chrome/browser/download/ar_quick_look_tab_helper.h"
 #include "ios/chrome/browser/favicon/favicon_service_factory.h"
 #import "ios/chrome/browser/find_in_page/find_tab_helper.h"
+#import "ios/chrome/browser/geolocation/omnibox_geolocation_tab_helper.h"
 #include "ios/chrome/browser/history/history_service_factory.h"
 #include "ios/chrome/browser/history/history_tab_helper.h"
 #include "ios/chrome/browser/history/top_sites_factory.h"
@@ -216,6 +217,8 @@ void AttachTabHelpers(web::WebState* web_state, bool for_prerender) {
     InfobarBadgeTabHelper::CreateForWebState(web_state);
   }
 
+  OmniboxGeolocationTabHelper::CreateForWebState(web_state);
+
   // Allow the embedder to attach tab helpers.
   ios::GetChromeBrowserProvider()->AttachTabHelpers(web_state);
 }

ios/chrome/browser/tabs/tab_model.mm

@@ -97,27 +97,9 @@ void CleanCertificatePolicyCache(
                  base::Unretained(web_state_list)));
 }
 
-// Records metrics for the interface's orientation.
-void RecordInterfaceOrientationMetric() {
-  switch ([[UIApplication sharedApplication] statusBarOrientation]) {
-    case UIInterfaceOrientationPortrait:
-    case UIInterfaceOrientationPortraitUpsideDown:
-      UMA_HISTOGRAM_BOOLEAN("Tab.PageLoadInPortrait", YES);
-      break;
-    case UIInterfaceOrientationLandscapeLeft:
-    case UIInterfaceOrientationLandscapeRight:
-      UMA_HISTOGRAM_BOOLEAN("Tab.PageLoadInPortrait", NO);
-      break;
-    case UIInterfaceOrientationUnknown:
-      // TODO(crbug.com/228832): Convert from a boolean histogram to an
-      // enumerated histogram and log this case as well.
-      break;
-  }
-}
-
 }  // anonymous namespace
 
-@interface TabModel () <CRWWebStateObserver> {
+@interface TabModel () {
   // Weak reference to the underlying shared model implementation.
   WebStateList* _webStateList;
 
@@ -262,81 +244,4 @@ void RecordInterfaceOrientationMetric() {
   }
 }
 
-#pragma mark - CRWWebStateObserver
-
-- (void)webState:(web::WebState*)webState
-    didFinishNavigation:(web::NavigationContext*)navigation {
-  if (!navigation->HasCommitted())
-    return;
-
-  if (!navigation->IsSameDocument() && !self.browserState->IsOffTheRecord()) {
-    int tabCount = static_cast<int>(self.count);
-    UMA_HISTOGRAM_CUSTOM_COUNTS("Tabs.TabCountPerLoad", tabCount, 1, 200, 50);
-  }
-
-  web::NavigationItem* item =
-      webState->GetNavigationManager()->GetLastCommittedItem();
-  navigation_metrics::RecordMainFrameNavigation(
-      item ? item->GetVirtualURL() : GURL::EmptyGURL(),
-      navigation->IsSameDocument(), self.browserState->IsOffTheRecord(),
-      GetBrowserStateType(webState->GetBrowserState()));
-}
-
-- (void)webState:(web::WebState*)webState
-    didStartNavigation:(web::NavigationContext*)navigation {
-
-  // In order to avoid false positive in the crash loop detection, disable the
-  // counter as soon as an URL is loaded. This requires an user action and is a
-  // significant source of crashes. Ignore NTP as it is loaded by default after
-  // a crash.
-  if (navigation->GetUrl().host_piece() != kChromeUINewTabHost) {
-    static dispatch_once_t dispatch_once_token;
-    dispatch_once(&dispatch_once_token, ^{
-      crash_util::ResetFailedStartupAttemptCount();
-    });
-  }
-
-  DCHECK(webState->GetNavigationManager());
-  web::NavigationItem* navigationItem =
-      webState->GetNavigationManager()->GetPendingItem();
-
-  // TODO(crbug.com/676129): the pending item is not correctly set when the
-  // page is reloading, use the last committed item if pending item is null.
-  // Remove this once tracking bug is fixed.
-  if (!navigationItem) {
-    navigationItem = webState->GetNavigationManager()->GetLastCommittedItem();
-  }
-
-  if (!navigationItem) {
-    // Pending item may not exist due to the bug in //ios/web layer.
-    // TODO(crbug.com/899827): remove this early return once GetPendingItem()
-    // always return valid object inside WebStateObserver::DidStartNavigation()
-    // callback.
-    //
-    // Note that GetLastCommittedItem() returns null if navigation manager does
-    // not have committed items (which is normal situation).
-    return;
-  }
-
-  [[OmniboxGeolocationController sharedInstance]
-      addLocationToNavigationItem:navigationItem
-                     browserState:ChromeBrowserState::FromBrowserState(
-                                      webState->GetBrowserState())];
-}
-
-- (void)webState:(web::WebState*)webState didLoadPageWithSuccess:(BOOL)success {
-  RecordInterfaceOrientationMetric();
-
-  [[OmniboxGeolocationController sharedInstance]
-      finishPageLoadForWebState:webState
-                    loadSuccess:success];
-}
-
-- (void)webStateDestroyed:(web::WebState*)webState {
-  // The TabModel is removed from WebState's observer when the WebState is
-  // detached from WebStateList which happens before WebState destructor,
-  // so this method should never be called.
-  NOTREACHED();
-}
-
 @end

ios/chrome/browser/web_state_list/BUILD.gn

@@ -47,7 +47,12 @@ source_set("agents") {
   ]
   deps = [
     ":web_state_list",
+    "//components/navigation_metrics",
+    "//components/profile_metrics",
+    "//ios/chrome/browser:chrome_url_constants",
     "//ios/chrome/browser/browser_state",
+    "//ios/chrome/browser/browser_state_metrics",
+    "//ios/chrome/browser/crash_report",
     "//ios/chrome/browser/main:public",
     "//ios/chrome/browser/sessions:restoration_agent",
     "//ios/chrome/browser/sessions:restoration_observer",

ios/chrome/browser/web_state_list/web_state_list_metrics_browser_agent.h

@@ -10,11 +10,13 @@
 #import "ios/chrome/browser/main/browser_user_data.h"
 #include "ios/chrome/browser/sessions/session_restoration_observer.h"
 #import "ios/chrome/browser/web_state_list/web_state_list_observer.h"
+#import "ios/web/public/web_state_observer.h"
 
 class WebStateListMetricsBrowserAgent
     : BrowserObserver,
       public WebStateListObserver,
       public SessionRestorationObserver,
+      public web::WebStateObserver,
       public BrowserUserData<WebStateListMetricsBrowserAgent> {
  public:
   WebStateListMetricsBrowserAgent();
@@ -52,6 +54,15 @@ class WebStateListMetricsBrowserAgent
   void SessionRestorationFinished(
       const std::vector<web::WebState*>& restored_web_states) override;
 
+  // web::WebStateObserver
+  void DidStartNavigation(web::WebState* web_state,
+                          web::NavigationContext* navigation_context) override;
+  void DidFinishNavigation(web::WebState* web_state,
+                           web::NavigationContext* navigation_context) override;
+  void PageLoaded(
+      web::WebState* web_state,
+      web::PageLoadCompletionStatus load_completion_status) override;
+
   // The WebStateList containing all the monitored tabs.
   WebStateList* web_state_list_;  // weak
 

ios/chrome/browser/web_state_list/web_state_list_metrics_browser_agent.mm

@@ -7,8 +7,19 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
 #include "base/metrics/user_metrics_action.h"
+#include "components/navigation_metrics/navigation_metrics.h"
+#include "components/profile_metrics/browser_profile_type.h"
+#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
+#include "ios/chrome/browser/browser_state_metrics/browser_state_metrics.h"
+#include "ios/chrome/browser/chrome_url_constants.h"
+#include "ios/chrome/browser/crash_report/crash_loop_detection_util.h"
 #import "ios/chrome/browser/sessions/session_restoration_browser_agent.h"
 #import "ios/chrome/browser/web_state_list/web_state_list.h"
+#include "ios/web/public/browser_state.h"
+#include "ios/web/public/navigation/navigation_context.h"
+#include "ios/web/public/navigation/navigation_item.h"
+#include "ios/web/public/navigation/navigation_manager.h"
+#import "ios/web/public/web_state.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -96,6 +107,84 @@ void WebStateListMetricsBrowserAgent::ResetSessionMetrics() {
   metric_collection_paused_ = false;
 }
 
+// web::WebStateObserver
+void WebStateListMetricsBrowserAgent::DidStartNavigation(
+    web::WebState* web_state,
+    web::NavigationContext* navigation_context) {
+  // In order to avoid false positive in the crash loop detection, disable the
+  // counter as soon as an URL is loaded. This requires an user action and is a
+  // significant source of crashes. Ignore NTP as it is loaded by default after
+  // a crash.
+  if (navigation_context->GetUrl().host_piece() != kChromeUINewTabHost) {
+    static dispatch_once_t dispatch_once_token;
+    dispatch_once(&dispatch_once_token, ^{
+      crash_util::ResetFailedStartupAttemptCount();
+    });
+  }
+
+  DCHECK(web_state->GetNavigationManager());
+  web::NavigationItem* navigation_item =
+      web_state->GetNavigationManager()->GetPendingItem();
+
+  // TODO(crbug.com/676129): the pending item is not correctly set when the
+  // page is reloading, use the last committed item if pending item is null.
+  // Remove this once tracking bug is fixed.
+  if (!navigation_item) {
+    navigation_item = web_state->GetNavigationManager()->GetLastCommittedItem();
+  }
+
+  if (!navigation_item) {
+    // Pending item may not exist due to the bug in //ios/web layer.
+    // TODO(crbug.com/899827): remove this early return once GetPendingItem()
+    // always return valid object inside WebStateObserver::DidStartNavigation()
+    // callback.
+    //
+    // Note that GetLastCommittedItem() returns null if navigation manager does
+    // not have committed items (which is normal situation).
+    return;
+  }
+}
+
+void WebStateListMetricsBrowserAgent::DidFinishNavigation(
+    web::WebState* web_state,
+    web::NavigationContext* navigation_context) {
+  if (!navigation_context->HasCommitted())
+    return;
+
+  if (!navigation_context->IsSameDocument() &&
+      !web_state->GetBrowserState()->IsOffTheRecord()) {
+    int tab_count = static_cast<int>(web_state_list_->count());
+    UMA_HISTOGRAM_CUSTOM_COUNTS("Tabs.TabCountPerLoad", tab_count, 1, 200, 50);
+  }
+
+  web::NavigationItem* item =
+      web_state->GetNavigationManager()->GetLastCommittedItem();
+  navigation_metrics::RecordMainFrameNavigation(
+      item ? item->GetVirtualURL() : GURL::EmptyGURL(),
+      navigation_context->IsSameDocument(),
+      web_state->GetBrowserState()->IsOffTheRecord(),
+      GetBrowserStateType(web_state->GetBrowserState()));
+}
+
+void WebStateListMetricsBrowserAgent::PageLoaded(
+    web::WebState* web_state,
+    web::PageLoadCompletionStatus load_completion_status) {
+  switch ([[UIApplication sharedApplication] statusBarOrientation]) {
+    case UIInterfaceOrientationPortrait:
+    case UIInterfaceOrientationPortraitUpsideDown:
+      UMA_HISTOGRAM_BOOLEAN("Tab.PageLoadInPortrait", YES);
+      break;
+    case UIInterfaceOrientationLandscapeLeft:
+    case UIInterfaceOrientationLandscapeRight:
+      UMA_HISTOGRAM_BOOLEAN("Tab.PageLoadInPortrait", NO);
+      break;
+    case UIInterfaceOrientationUnknown:
+      // TODO(crbug.com/228832): Convert from a boolean histogram to an
+      // enumerated histogram and log this case as well.
+      break;
+  }
+}
+
 void WebStateListMetricsBrowserAgent::BrowserDestroyed(Browser* browser) {
   DCHECK_EQ(browser->GetWebStateList(), web_state_list_);