Remove NOTIFICATION_TAB_PARENTED/CLOSING from metrics code.

BrowserActivityWatcher is now a TabStripModelObserver, and runs the
activity callback whenever a tab action is taken (to include parenting,
i.e. addition or replacement, as well as closing).

Note these notifications are sent by Browser, so they don't exist on
Android. BrowserActivityWatcher and TabStripModelObserver are also
unique to desktop, so this entire CL only affects desktop.

Bug: 268984
Change-Id: I668e22bf637c33266060cd27c1464118a8c6ed1c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1752912
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687298}

chrome/browser/metrics/browser_activity_watcher.cc

@@ -4,23 +4,43 @@
 
 #include "chrome/browser/metrics/browser_activity_watcher.h"
 
+#include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_list_observer.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 
 BrowserActivityWatcher::BrowserActivityWatcher(
-    const base::RepeatingClosure& on_browser_list_change)
-    : on_browser_list_change_(on_browser_list_change) {
+    const base::RepeatingClosure& on_browser_activity)
+    : on_browser_activity_(on_browser_activity) {
   BrowserList::AddObserver(this);
+
+  for (Browser* browser : *BrowserList::GetInstance()) {
+    if (browser->tab_strip_model())
+      browser->tab_strip_model()->AddObserver(this);
+  }
 }
 
 BrowserActivityWatcher::~BrowserActivityWatcher() {
   BrowserList::RemoveObserver(this);
 }
 
 void BrowserActivityWatcher::OnBrowserAdded(Browser* browser) {
-  on_browser_list_change_.Run();
+  if (browser->tab_strip_model())
+    browser->tab_strip_model()->AddObserver(this);
+
+  on_browser_activity_.Run();
 }
 
 void BrowserActivityWatcher::OnBrowserRemoved(Browser* browser) {
-  on_browser_list_change_.Run();
+  if (browser->tab_strip_model())
+    browser->tab_strip_model()->RemoveObserver(this);
+
+  on_browser_activity_.Run();
+}
+
+void BrowserActivityWatcher::OnTabStripModelChanged(
+    TabStripModel* tab_strip_model,
+    const TabStripModelChange& change,
+    const TabStripSelectionChange& selection) {
+  on_browser_activity_.Run();
 }

chrome/browser/metrics/browser_activity_watcher.h

@@ -8,23 +8,31 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "chrome/browser/ui/browser_list_observer.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 
 // Helper that fires a callback every time a Browser object is added or removed
-// from the BrowserList. This class primarily exists to encapsulate this
-// behavior and reduce the number of platform-specific macros as Android doesn't
-// use BrowserList.
-class BrowserActivityWatcher : public BrowserListObserver {
+// from the BrowserList, or a browser's tab strip model is modified. This class
+// primarily exists to encapsulate this behavior and reduce the number of
+// platform-specific macros as Android doesn't use BrowserList or TabStripModel.
+class BrowserActivityWatcher : public BrowserListObserver,
+                               public TabStripModelObserver {
  public:
   explicit BrowserActivityWatcher(
-      const base::RepeatingClosure& on_browser_list_change);
+      const base::RepeatingClosure& on_browser_activity);
   ~BrowserActivityWatcher() override;
 
   // BrowserListObserver:
   void OnBrowserAdded(Browser* browser) override;
   void OnBrowserRemoved(Browser* browser) override;
 
+  // TabStripModelObserver:
+  void OnTabStripModelChanged(
+      TabStripModel* tab_strip_model,
+      const TabStripModelChange& change,
+      const TabStripSelectionChange& selection) override;
+
  private:
-  base::RepeatingClosure on_browser_list_change_;
+  base::RepeatingClosure on_browser_activity_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserActivityWatcher);
 };

chrome/browser/metrics/chrome_metrics_service_client.cc

@@ -931,10 +931,6 @@ void ChromeMetricsServiceClient::RecordCommandLineMetrics() {
 }
 
 bool ChromeMetricsServiceClient::RegisterForNotifications() {
-  registrar_.Add(this, chrome::NOTIFICATION_TAB_PARENTED,
-                 content::NotificationService::AllSources());
-  registrar_.Add(this, chrome::NOTIFICATION_TAB_CLOSING,
-                 content::NotificationService::AllSources());
   registrar_.Add(this, content::NOTIFICATION_LOAD_START,
                  content::NotificationService::AllSources());
   registrar_.Add(this, content::NOTIFICATION_LOAD_STOP,
@@ -1009,8 +1005,6 @@ void ChromeMetricsServiceClient::Observe(
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   switch (type) {
-    case chrome::NOTIFICATION_TAB_PARENTED:
-    case chrome::NOTIFICATION_TAB_CLOSING:
     case content::NOTIFICATION_LOAD_STOP:
     case content::NOTIFICATION_LOAD_START:
     case content::NOTIFICATION_RENDERER_PROCESS_CLOSED:

chrome/browser/metrics/thread_watcher.cc

@@ -101,10 +101,6 @@ ThreadWatcherObserver::ThreadWatcherObserver(
                           base::Unretained(this)));
 #endif
 
-  registrar_.Add(this, chrome::NOTIFICATION_TAB_PARENTED,
-                 content::NotificationService::AllSources());
-  registrar_.Add(this, chrome::NOTIFICATION_TAB_CLOSING,
-                 content::NotificationService::AllSources());
   registrar_.Add(this, content::NOTIFICATION_LOAD_START,
                  content::NotificationService::AllSources());
   registrar_.Add(this, content::NOTIFICATION_LOAD_STOP,