Introducing a pref for hover card image previews

Adding a new pref for hover card image previews which will be controlled
from settings. This pref's default value will be the same as the
hover card image previews feature. This allows us to continue the
rollout on MacOS through Finch.

Change-Id: Ifb94a0e44df8cd219ba03b99c48f58c2e5488e44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4529188
Reviewed-by: Dana Fried <dfried@chromium.org>
Commit-Queue: Eshwar Stalin <estalin@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1152060}

diff --git a/chrome/browser/ui/browser_ui_prefs.cc b/chrome/browser/ui/browser_ui_prefs.cc
index 16ab809..31af7b4 100644
--- a/chrome/browser/ui/browser_ui_prefs.cc
+++ b/chrome/browser/ui/browser_ui_prefs.cc

@@ -56,6 +56,8 @@
   registry->RegisterIntegerPref(
       prefs::kMacRestoreLocationPermissionsExperimentCount, 0);
 #endif  // BUILDFLAG(IS_MAC)
+
+  registry->RegisterBooleanPref(prefs::kHoverCardImagesEnabled, true);
 }
 
 void RegisterBrowserUserPrefs(user_prefs::PrefRegistrySyncable* registry) {

diff --git a/chrome/browser/ui/tab_helpers.cc b/chrome/browser/ui/tab_helpers.cc
index 4e8f049..f895075 100644
--- a/chrome/browser/ui/tab_helpers.cc
+++ b/chrome/browser/ui/tab_helpers.cc

@@ -507,10 +507,8 @@
     performance_manager::user_tuning::UserPerformanceTuningManager::
         ResourceUsageTabHelper::CreateForWebContents(web_contents);
   }
-  if (base::FeatureList::IsEnabled(features::kTabHoverCardImages) ||
-      base::FeatureList::IsEnabled(features::kWebUITabStrip)) {
-    ThumbnailTabHelper::CreateForWebContents(web_contents);
-  }
+  ThumbnailTabHelper::CreateForWebContents(web_contents);
+
   web_modal::WebContentsModalDialogManager::CreateForWebContents(web_contents);
   if (OmniboxFieldTrial::IsZeroSuggestPrefetchingEnabled()) {
     ZeroSuggestPrefetchTabHelper::CreateForWebContents(web_contents);

diff --git a/chrome/browser/ui/views/tabs/tab_hover_card_controller.cc b/chrome/browser/ui/views/tabs/tab_hover_card_controller.cc
index 77f4beae..61da9c3 100644
--- a/chrome/browser/ui/views/tabs/tab_hover_card_controller.cc
+++ b/chrome/browser/ui/views/tabs/tab_hover_card_controller.cc

@@ -12,6 +12,7 @@
 #include "base/memory/raw_ptr.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/ui/tabs/tab_style.h"
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/chrome_widget_sublevel.h"
@@ -23,6 +24,7 @@
 #include "chrome/browser/ui/views/tabs/tab_hover_card_thumbnail_observer.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
 #include "chrome/browser/ui/views/tabs/tab_strip_controller.h"
+#include "chrome/common/pref_names.h"
 #include "components/omnibox/browser/omnibox_edit_model.h"
 #include "components/omnibox/browser/omnibox_popup_view.h"
 #include "components/user_education/common/help_bubble_factory_registry.h"
@@ -245,6 +247,23 @@
 
 TabHoverCardController::TabHoverCardController(TabStrip* tab_strip)
     : tab_strip_(tab_strip) {
+  if (PrefService* pref_service = g_browser_process->local_state()) {
+    // Hovercard image previews are still not fully rolled out to all platforms
+    // so we default the pref to the state of the feature rollout.
+    pref_service->SetDefaultPrefValue(prefs::kHoverCardImagesEnabled,
+                                      base::Value(base::FeatureList::IsEnabled(
+                                          features::kTabHoverCardImages)));
+
+    // Register for previews enabled pref change events.
+    hover_card_image_previews_enabled_ = AreHoverCardImagesEnabled();
+    pref_change_registrar_.Init(pref_service);
+    pref_change_registrar_.Add(
+        prefs::kHoverCardImagesEnabled,
+        base::BindRepeating(
+            &TabHoverCardController::OnHovercardImagesEnabledChanged,
+            base::Unretained(this)));
+  }
+
   // Possibly apply memory pressure override for testing.
   auto override = GetMemoryPressureOverride();
   if (override) {
@@ -261,7 +280,8 @@
 
 // static
 bool TabHoverCardController::AreHoverCardImagesEnabled() {
-  return base::FeatureList::IsEnabled(features::kTabHoverCardImages);
+  PrefService* pref_service = g_browser_process->local_state();
+  return pref_service->GetBoolean(prefs::kHoverCardImagesEnabled);
 }
 
 // static
@@ -525,10 +545,10 @@
       base::BindRepeating(&TabHoverCardController::OnFadeAnimationEnded,
                           weak_ptr_factory_.GetWeakPtr()));
 
-  if (!thumbnail_observer_ && AreHoverCardImagesEnabled()) {
+  if (!thumbnail_observer_ && hover_card_image_previews_enabled_) {
     thumbnail_observer_ = std::make_unique<TabHoverCardThumbnailObserver>();
     thumbnail_subscription_ = thumbnail_observer_->AddCallback(
-        base::BindRepeating(&TabHoverCardController::OnPreviewImageAvaialble,
+        base::BindRepeating(&TabHoverCardController::OnPreviewImageAvailable,
                             weak_ptr_factory_.GetWeakPtr()));
   }
 }
@@ -755,7 +775,7 @@
   OnCardFullyVisible();
 }
 
-void TabHoverCardController::OnPreviewImageAvaialble(
+void TabHoverCardController::OnPreviewImageAvailable(
     TabHoverCardThumbnailObserver* observer,
     gfx::ImageSkia thumbnail_image) {
   DCHECK_EQ(thumbnail_observer_.get(), observer);
@@ -791,3 +811,11 @@
   // hovering a card through an entire CRITICAL -> NORMAL transition so as a
   // simplification we simply don't care.
 }
+
+void TabHoverCardController::OnHovercardImagesEnabledChanged() {
+  hover_card_image_previews_enabled_ = AreHoverCardImagesEnabled();
+  if (!hover_card_image_previews_enabled_) {
+    thumbnail_subscription_ = base::CallbackListSubscription();
+    thumbnail_observer_.reset();
+  }
+}

diff --git a/chrome/browser/ui/views/tabs/tab_hover_card_controller.h b/chrome/browser/ui/views/tabs/tab_hover_card_controller.h
index aa321c6..31c56d8 100644
--- a/chrome/browser/ui/views/tabs/tab_hover_card_controller.h
+++ b/chrome/browser/ui/views/tabs/tab_hover_card_controller.h

@@ -16,6 +16,7 @@
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/ui/views/tabs/tab_slot_controller.h"
+#include "components/prefs/pref_change_registrar.h"
 #include "ui/events/event.h"
 #include "ui/views/animation/bubble_slide_animator.h"
 #include "ui/views/animation/widget_fade_animator.h"
@@ -65,6 +66,8 @@
   FRIEND_TEST_ALL_PREFIXES(TabHoverCardControllerTest, ShowWrongTabDoesntCrash);
   FRIEND_TEST_ALL_PREFIXES(TabHoverCardControllerTest,
                            SetPreviewWithNoHoverCardDoesntCrash);
+  FRIEND_TEST_ALL_PREFIXES(TabHoverCardControllerTest, ShowPreviewsForTab);
+  FRIEND_TEST_ALL_PREFIXES(TabHoverCardControllerTest, DisablePreviewsForTab);
   class EventSniffer;
 
   enum ThumbnailWaitState {
@@ -111,12 +114,14 @@
                                   double value);
   void OnSlideAnimationComplete(views::BubbleSlideAnimator* animator);
 
-  void OnPreviewImageAvaialble(TabHoverCardThumbnailObserver* observer,
+  void OnPreviewImageAvailable(TabHoverCardThumbnailObserver* observer,
                                gfx::ImageSkia thumbnail_image);
 
   void OnMemoryPressureChanged(
       base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level);
 
+  void OnHovercardImagesEnabledChanged();
+
   bool waiting_for_preview() const {
     return thumbnail_wait_state_ != ThumbnailWaitState::kNotWaiting;
   }
@@ -162,6 +167,10 @@
       base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE;
   std::unique_ptr<base::MemoryPressureListener> memory_pressure_listener_;
 
+  // Tracks changes to the hover card image previews preferences
+  PrefChangeRegistrar pref_change_registrar_;
+  bool hover_card_image_previews_enabled_ = false;
+
   // Ensure that this timer is destroyed before anything else is cleaned up.
   base::OneShotTimer delayed_show_timer_;
   base::WeakPtrFactory<TabHoverCardController> weak_ptr_factory_{this};

diff --git a/chrome/browser/ui/views/tabs/tab_hover_card_controller_unittest.cc b/chrome/browser/ui/views/tabs/tab_hover_card_controller_unittest.cc
index fbbab05..8da114dc 100644
--- a/chrome/browser/ui/views/tabs/tab_hover_card_controller_unittest.cc
+++ b/chrome/browser/ui/views/tabs/tab_hover_card_controller_unittest.cc

@@ -4,8 +4,12 @@
 
 #include "chrome/browser/ui/views/tabs/tab_hover_card_controller.h"
 
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/test_with_browser_view.h"
+#include "chrome/browser/ui/views/tabs/tab_strip.h"
+#include "chrome/common/pref_names.h"
 
 // These are regression tests for possible crashes.
 
@@ -29,6 +33,56 @@
       std::make_unique<TabHoverCardController>(browser_view()->tabstrip());
   // If the safeguard is not in place, this could crash in either metrics
   // collection *or* in trying to set the actual thumbnail image on the card.
-  controller->OnPreviewImageAvaialble(controller->thumbnail_observer_.get(),
+  controller->OnPreviewImageAvailable(controller->thumbnail_observer_.get(),
                                       gfx::ImageSkia());
 }
+
+TEST_F(TabHoverCardControllerTest, ShowPreviewsForTab) {
+  g_browser_process->local_state()->SetBoolean(prefs::kHoverCardImagesEnabled,
+                                               true);
+
+  AddTab(browser_view()->browser(), GURL("http://foo1.com"));
+  AddTab(browser_view()->browser(), GURL("http://foo2.com"));
+  browser_view()->browser()->tab_strip_model()->ActivateTabAt(0);
+
+  auto controller =
+      std::make_unique<TabHoverCardController>(browser_view()->tabstrip());
+
+  Tab* const target_tab = browser_view()->tabstrip()->tab_at(1);
+  controller->target_tab_ = target_tab;
+
+  controller->CreateHoverCard(target_tab);
+  EXPECT_TRUE(controller->ArePreviewsEnabled());
+}
+
+TEST_F(TabHoverCardControllerTest, DisablePreviewsForTab) {
+  g_browser_process->local_state()->SetBoolean(prefs::kHoverCardImagesEnabled,
+                                               false);
+
+  AddTab(browser_view()->browser(), GURL("http://foo1.com"));
+  AddTab(browser_view()->browser(), GURL("http://foo2.com"));
+  browser_view()->browser()->tab_strip_model()->ActivateTabAt(0);
+
+  auto controller =
+      std::make_unique<TabHoverCardController>(browser_view()->tabstrip());
+
+  Tab* const target_tab = browser_view()->tabstrip()->tab_at(1);
+  controller->target_tab_ = target_tab;
+
+  controller->CreateHoverCard(target_tab);
+  EXPECT_FALSE(controller->ArePreviewsEnabled());
+}
+
+class TabHoverCardPreviewsEnabledPrefTest : public TestWithBrowserView {
+ public:
+  TabHoverCardPreviewsEnabledPrefTest() {
+    feature_list_.InitAndDisableFeature(features::kTabHoverCardImages);
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+TEST_F(TabHoverCardPreviewsEnabledPrefTest, DefaultState) {
+  EXPECT_FALSE(TabHoverCardController::AreHoverCardImagesEnabled());
+}

diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index 5ee417ff2..858e4dd 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc

@@ -3684,4 +3684,7 @@
 // `HttpsUpgradesEnabled` enterprise policy.
 const char kHttpsUpgradesEnabled[] = "https_upgrades.policy.upgrades_enabled";
 
+// Whether the hovercard image previews is enabled
+const char kHoverCardImagesEnabled[] = "browser.hovercard_images_enabled";
+
 }  // namespace prefs

diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index 32ddec5..39ae4dc8 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h

@@ -1296,6 +1296,8 @@
 extern const char kHttpAllowlist[];
 extern const char kHttpsUpgradesEnabled[];
 
+extern const char kHoverCardImagesEnabled[];
+
 }  // namespace prefs
 
 #endif  // CHROME_COMMON_PREF_NAMES_H_