Vertical tab P3A improvements as per feedback

brave · Jun 21, 2023 · 7c1f503 · 7c1f503
1 parent ec53193
commit 7c1f503
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 27 deletions.
diff --git a/browser/brave_browser_process_impl.cc b/browser/brave_browser_process_impl.cc
@@ -527,12 +527,9 @@ brave::BraveFarblingService* BraveBrowserProcessImpl::brave_farbling_service() {
 
 misc_metrics::ProcessMiscMetrics*
 BraveBrowserProcessImpl::process_misc_metrics() {
-#if !BUILDFLAG(IS_ANDROID)
   if (!process_misc_metrics_) {
     process_misc_metrics_ =
         std::make_unique<misc_metrics::ProcessMiscMetrics>(local_state());
   }
-#endif
   return process_misc_metrics_.get();
 }
-
diff --git a/browser/misc_metrics/privacy_hub_metrics_factory_android.cc b/browser/misc_metrics/privacy_hub_metrics_factory_android.cc
@@ -13,7 +13,9 @@ namespace chrome {
 namespace android {
 static jlong JNI_PrivacyHubMetricsFactory_GetInterfaceToPrivacyHubMetrics(
     JNIEnv* env) {
-  auto pending = g_brave_browser_process->process_misc_metrics()->privacy_hub_metrics()->MakeRemote();
+  auto pending = g_brave_browser_process->process_misc_metrics()
+                     ->privacy_hub_metrics()
+                     ->MakeRemote();
 
   return static_cast<jlong>(pending.PassPipe().release().value());
 }

diff --git a/browser/misc_metrics/process_misc_metrics.cc b/browser/misc_metrics/process_misc_metrics.cc
@@ -23,7 +23,7 @@ ProcessMiscMetrics::ProcessMiscMetrics(PrefService* local_state) {
   vertical_tab_metrics_ = std::make_unique<VerticalTabMetrics>(local_state);
 #else
   privacy_hub_metrics_ =
-      std::make_unique<misc_metrics::PrivacyHubMetrics>(local_state());
+      std::make_unique<misc_metrics::PrivacyHubMetrics>(local_state);
 #endif
 }
 

diff --git a/browser/misc_metrics/vertical_tab_metrics.cc b/browser/misc_metrics/vertical_tab_metrics.cc
@@ -101,11 +101,10 @@ void VerticalTabBrowserMetrics::OnTabStripModelChanged(
   change_callback_.Run();
 }
 
-bool VerticalTabBrowserMetrics::IsVerticalTabsEnabled() const {
-  return vertical_tabs_enabled_;
-}
-
 size_t VerticalTabBrowserMetrics::GetTabCount(TabCountType count_type) const {
+  if (!vertical_tabs_enabled_) {
+    return 0;
+  }
   return counts_.at(count_type);
 }
 
@@ -132,17 +131,6 @@ void VerticalTabMetrics::RegisterPrefs(PrefRegistrySimple* registry) {
 }
 
 void VerticalTabMetrics::UpdateMetrics() {
-  bool is_vertical_tabs_enabled = false;
-  for (const auto& [session_id, browser_metrics] : browser_metrics_) {
-    if (browser_metrics->IsVerticalTabsEnabled()) {
-      is_vertical_tabs_enabled = true;
-    }
-  }
-  if (!is_vertical_tabs_enabled) {
-    // Do not report if vertical tabs is not enabled on any of the
-    // active windows/profiles.
-    return;
-  }
   // Add up tab count totals from all windows
   base::flat_map<TabCountType, size_t> current_counts;
   for (const auto& [session_id, browser_metrics] : browser_metrics_) {
@@ -167,14 +155,12 @@ void VerticalTabMetrics::OnBrowserAdded(Browser* browser) {
     return;
   }
   Profile* profile = browser->profile();
-  if (!profile || profile->IsOffTheRecord()) {
+  if (!profile || profile->IsOffTheRecord() || !profile->IsRegularProfile()) {
     // Do not monitor incognito windows.
     return;
   }
   PrefService* profile_prefs = profile->GetPrefs();
-  if (!profile_prefs) {
-    return;
-  }
+  CHECK(profile_prefs);
   SessionID session_id = browser->session_id();
   browser_metrics_[session_id] = std::make_unique<VerticalTabBrowserMetrics>(
       profile_prefs, base::BindRepeating(&VerticalTabMetrics::UpdateMetrics,

diff --git a/browser/misc_metrics/vertical_tab_metrics.h b/browser/misc_metrics/vertical_tab_metrics.h
@@ -27,7 +27,11 @@ extern const char kVerticalOpenTabsHistogramName[];
 extern const char kVerticalGroupTabsHistogramName[];
 extern const char kVerticalPinnedTabsHistogramName[];
 
-enum class TabCountType { kOpen, kGroup, kPinned };
+enum class TabCountType {
+  kOpen,
+  kGroup,
+  kPinned,
+};
 
 const TabCountType kAllTabCountTypes[] = {
     TabCountType::kOpen, TabCountType::kGroup, TabCountType::kPinned};
@@ -42,12 +46,12 @@ class VerticalTabBrowserMetrics : public TabStripModelObserver {
   VerticalTabBrowserMetrics& operator=(const VerticalTabBrowserMetrics&) =
       delete;
 
+  // TabStripModelObserver:
   void OnTabStripModelChanged(
       TabStripModel* tab_strip_model,
       const TabStripModelChange& change,
       const TabStripSelectionChange& selection) override;
 
-  bool IsVerticalTabsEnabled() const;
   size_t GetTabCount(TabCountType count_type) const;
 
  private:
@@ -72,7 +76,7 @@ class VerticalTabMetrics : public BrowserListObserver {
 
   void UpdateMetrics();
 
-  // BrowserListObserver
+  // BrowserListObserver:
   void OnBrowserAdded(Browser* browser) override;
   void OnBrowserRemoved(Browser* browser) override;