Reland "[Privacy Hub] Add triggers for HaTS survey"

This is a reland of commit d5dbc7f.

This fixes the crash from crbug.com/1417002 by enabling the handler all
the time and moving the feature check into the TS side only calling the
handlers when the feature is enabled.

Original change's description:
> [Privacy Hub] Add triggers for HaTS survey
>
> Add the new triggers for the survey. This requires the trigger class
> to handle the delay part between user interaction and showing the
> survey.
>
> Also removes the UMA as the new question setup makes it impossible.
>
> Bug: b:264971230
> Change-Id: I67228643e16938ccc246f81097bc8a78e5282d05
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4188890
> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
> Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
> Commit-Queue: Christoph Schlosser <cschlosser@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1106171}

Bug: b:264971230
Change-Id: I6be9e876f3980c1d6c7ad5801a12083b35bfc194
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4264089
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Christoph Schlosser <cschlosser@chromium.org>
Auto-Submit: Christoph Schlosser <cschlosser@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1108388}

chrome/browser/ash/hats/hats_config.cc

@@ -179,7 +179,7 @@ const HatsConfig kHatsBluetoothRevampSurvey = {
 };
 
 // Privacy Hub Baseline experience survey -- shown 40 seconds after the user
-// stayed for more than 5 seconds on the Security and Privacy page.
+// leaves the Security and Privacy page.
 const HatsConfig kPrivacyHubBaselineSurvey = {
     ::features::kHappinessTrackingPrivacyHubBaseline,  // feature
     base::Days(1),                                     // new_device_threshold

chrome/browser/ash/privacy_hub/privacy_hub_hats_trigger.cc

@@ -12,18 +12,31 @@
 namespace ash {
 namespace {
 const HatsConfig& kHatsConfig = kPrivacyHubBaselineSurvey;
+constexpr base::TimeDelta kShowSurveyDelay = base::Seconds(40);
 }
 
-PrivacyHubHatsTrigger::PrivacyHubHatsTrigger() = default;
-PrivacyHubHatsTrigger::~PrivacyHubHatsTrigger() = default;
+PrivacyHubHatsTrigger& PrivacyHubHatsTrigger::Get() {
+  static base::NoDestructor<PrivacyHubHatsTrigger> instance;
+  return *instance;
+}
 
-void PrivacyHubHatsTrigger::ShowSurveyIfSelected() {
-  // The user has already seen a survey.
-  if (hats_controller_) {
+void PrivacyHubHatsTrigger::ShowSurveyAfterDelayElapsed() {
+  // The user has already seen a survey or we're about to show them one.
+  if (hats_controller_ || show_notification_timer_.IsRunning()) {
     return;
   }
 
-  // We only show the survey if the current session is active.
+  show_notification_timer_.Start(
+      FROM_HERE, kShowSurveyDelay,
+      base::BindOnce(&PrivacyHubHatsTrigger::ShowSurveyIfSelected,
+                     base::Unretained(this)));
+}
+
+PrivacyHubHatsTrigger::PrivacyHubHatsTrigger() = default;
+PrivacyHubHatsTrigger::~PrivacyHubHatsTrigger() = default;
+
+void PrivacyHubHatsTrigger::ShowSurveyIfSelected() {
+  // We only show the survey if the current session is still active.
   if (session_manager::SessionManager::Get()->IsUserSessionBlocked()) {
     return;
   }
@@ -50,6 +63,10 @@ PrivacyHubHatsTrigger::GetHatsNotificationControllerForTesting() const {
   return hats_controller_.get();
 }
 
+base::OneShotTimer& PrivacyHubHatsTrigger::GetTimerForTesting() {
+  return show_notification_timer_;
+}
+
 Profile* PrivacyHubHatsTrigger::GetProfile() const {
   if (no_profile_for_testing_) {
     return nullptr;

chrome/browser/ash/privacy_hub/privacy_hub_hats_trigger.h

@@ -6,10 +6,15 @@
 #define CHROME_BROWSER_ASH_PRIVACY_HUB_PRIVACY_HUB_HATS_TRIGGER_H_
 
 #include "base/memory/scoped_refptr.h"
+#include "base/no_destructor.h"
+#include "base/timer/timer.h"
 
 class Profile;
 
 namespace ash {
+namespace settings {
+class PrivacyHubHandlerHatsTest;
+}
 
 class HatsNotificationController;
 
@@ -18,23 +23,35 @@ class HatsNotificationController;
 // survey within the PrivacyHub specific limits.
 class PrivacyHubHatsTrigger {
  public:
-  PrivacyHubHatsTrigger();
+  static PrivacyHubHatsTrigger& Get();
+
   PrivacyHubHatsTrigger(const PrivacyHubHatsTrigger&) = delete;
   PrivacyHubHatsTrigger& operator=(const PrivacyHubHatsTrigger&) = delete;
-  ~PrivacyHubHatsTrigger();
 
-  // Show the survey to the current primary user if they are selected. If they
-  // aren't or any of the surveys preconditions aren't met this does nothing.
-  void ShowSurveyIfSelected();
+  // Start the delay to show the survey to the user if they are selected. If the
+  // user has already seen a survey or this is called while the delay hasn't
+  // elapsed yet nothing will happen.
+  void ShowSurveyAfterDelayElapsed();
 
  private:
+  friend class base::NoDestructor<PrivacyHubHatsTrigger>;
   friend class PrivacyHubHatsTriggerTest;
+  friend class settings::PrivacyHubHandlerHatsTest;
+
+  PrivacyHubHatsTrigger();
+  ~PrivacyHubHatsTrigger();
 
   const HatsNotificationController* GetHatsNotificationControllerForTesting()
       const;
+  base::OneShotTimer& GetTimerForTesting();
   void SetNoProfileForTesting(bool no_profile);
   Profile* GetProfile() const;
 
+  // Show the survey to the current primary user if they are selected. If they
+  // aren't or any of the surveys preconditions aren't met this does nothing.
+  void ShowSurveyIfSelected();
+
+  base::OneShotTimer show_notification_timer_;
   scoped_refptr<HatsNotificationController> hats_controller_;
   bool no_profile_for_testing_ = false;
 };

chrome/browser/ash/privacy_hub/privacy_hub_hats_trigger_browsertest.cc

@@ -51,10 +51,31 @@ class PrivacyHubHatsTriggerTest : public InProcessBrowserTest {
     return privacy_hub_trigger_.GetHatsNotificationControllerForTesting();
   }
 
+  base::OneShotTimer& GetTimer() {
+    return privacy_hub_trigger_.GetTimerForTesting();
+  }
+
   void SetNoProfileForTesting() {
     privacy_hub_trigger_.SetNoProfileForTesting(true);
   }
 
+  void ExpectTimerIsRunningThenTrigger() {
+    EXPECT_TRUE(GetTimer().IsRunning());
+    EXPECT_FALSE(IsHatsNotificationActive());
+    EXPECT_FALSE(GetHatsNotificationController());
+
+    GetTimer().FireNow();
+  }
+
+  void RunThenTriggerTimer(base::TimeDelta delay) {
+    base::SequencedTaskRunner::GetCurrentDefault()->PostDelayedTask(
+        FROM_HERE,
+        base::BindOnce(
+            &PrivacyHubHatsTriggerTest::ExpectTimerIsRunningThenTrigger,
+            base::Unretained(this)),
+        delay);
+  }
+
   base::test::ScopedFeatureList scoped_feature_list_;
   PrivacyHubHatsTrigger privacy_hub_trigger_;
 
@@ -66,12 +87,13 @@ IN_PROC_BROWSER_TEST_F(PrivacyHubHatsTriggerTest, ShowSurveySuccess) {
 
   base::RunLoop loop;
   display_service_->SetNotificationAddedClosure(loop.QuitClosure());
-  privacy_hub_trigger_.ShowSurveyIfSelected();
+  privacy_hub_trigger_.ShowSurveyAfterDelayElapsed();
 
-  EXPECT_TRUE(GetHatsNotificationController());
+  RunThenTriggerTimer(base::Seconds(2));
 
   loop.Run();
 
+  EXPECT_TRUE(GetHatsNotificationController());
   EXPECT_TRUE(IsHatsNotificationActive());
 }
 
@@ -81,15 +103,19 @@ IN_PROC_BROWSER_TEST_F(PrivacyHubHatsTriggerTest, ShowSurveyOnlyOnce) {
   // Show survey once
   base::RunLoop loop;
   display_service_->SetNotificationAddedClosure(loop.QuitClosure());
-  privacy_hub_trigger_.ShowSurveyIfSelected();
+  privacy_hub_trigger_.ShowSurveyAfterDelayElapsed();
+
+  RunThenTriggerTimer(base::Seconds(2));
+
+  loop.Run();
+
   const HatsNotificationController* hats_notification_controller =
       GetHatsNotificationController();
-  EXPECT_TRUE(hats_notification_controller);
-  loop.Run();
+  EXPECT_NE(hats_notification_controller, nullptr);
   EXPECT_TRUE(IsHatsNotificationActive());
 
   // Trigger survey again but the controller shouldn't be a new instance.
-  privacy_hub_trigger_.ShowSurveyIfSelected();
+  privacy_hub_trigger_.ShowSurveyAfterDelayElapsed();
 
   EXPECT_EQ(hats_notification_controller, GetHatsNotificationController());
 }
@@ -98,7 +124,7 @@ IN_PROC_BROWSER_TEST_F(PrivacyHubHatsTriggerTest, NoActiveProfile) {
   SetNoProfileForTesting();
   EXPECT_FALSE(IsHatsNotificationActive());
 
-  privacy_hub_trigger_.ShowSurveyIfSelected();
+  privacy_hub_trigger_.ShowSurveyAfterDelayElapsed();
 
   EXPECT_FALSE(GetHatsNotificationController());
 }
@@ -108,7 +134,7 @@ IN_PROC_BROWSER_TEST_F(PrivacyHubHatsTriggerTest, NoSurveyIfSessionNotActive) {
       session_manager::SessionState::LOCKED);
   EXPECT_FALSE(IsHatsNotificationActive());
 
-  privacy_hub_trigger_.ShowSurveyIfSelected();
+  privacy_hub_trigger_.ShowSurveyAfterDelayElapsed();
 
   EXPECT_FALSE(GetHatsNotificationController());
 }

chrome/browser/resources/settings/chromeos/os_privacy_page/os_privacy_page.ts

@@ -36,6 +36,7 @@ import {Route, Router} from '../router.js';
 
 import {getTemplate} from './os_privacy_page.html.js';
 import {PeripheralDataAccessBrowserProxy, PeripheralDataAccessBrowserProxyImpl} from './peripheral_data_access_browser_proxy.js';
+import {PrivacyHubBrowserProxy, PrivacyHubBrowserProxyImpl} from './privacy_hub_browser_proxy.js';
 import {PrivacyHubNavigationOrigin} from './privacy_hub_page.js';
 
 const OsSettingsPrivacyPageElementBase = PrefsMixin(
@@ -191,6 +192,14 @@ class OsSettingsPrivacyPageElement extends OsSettingsPrivacyPageElementBase {
               !loadTimeData.getBoolean('isGuest');
         },
       },
+
+      isHatsSurveyEnabled_: {
+        type: Boolean,
+        readOnly: true,
+        value: function() {
+          return loadTimeData.getBoolean('isPrivacyHubHatsEnabled');
+        },
+      },
     };
   }
 
@@ -200,6 +209,7 @@ class OsSettingsPrivacyPageElement extends OsSettingsPrivacyPageElementBase {
 
   private authToken_: chrome.quickUnlockPrivate.TokenInfo|undefined;
   private browserProxy_: PeripheralDataAccessBrowserProxy;
+  private privacyHubBrowserProxy_: PrivacyHubBrowserProxy;
 
   /**
    * The timeout ID to pass to clearTimeout() to cancel auth token
@@ -211,6 +221,7 @@ class OsSettingsPrivacyPageElement extends OsSettingsPrivacyPageElementBase {
   private fingerprintUnlockEnabled_: boolean;
   private focusConfig_: Map<string, string>;
   private isGuestMode_: boolean;
+  private isHatsSurveyEnabled_: boolean;
   private isRevenBranding_: boolean;
   private isSmartPrivacyEnabled_: boolean;
   private isThunderboltSupported_: boolean;
@@ -232,6 +243,8 @@ class OsSettingsPrivacyPageElement extends OsSettingsPrivacyPageElementBase {
         this.supportedSettingIds.add(Setting.kPeripheralDataAccessProtection);
       }
     });
+
+    this.privacyHubBrowserProxy_ = PrivacyHubBrowserProxyImpl.getInstance();
   }
 
   override ready(): void {
@@ -243,9 +256,14 @@ class OsSettingsPrivacyPageElement extends OsSettingsPrivacyPageElementBase {
   override currentRouteChanged(route: Route): void {
     // Does not apply to this page.
     if (route !== routes.OS_PRIVACY) {
+      if (this.isHatsSurveyEnabled_) {
+        this.privacyHubBrowserProxy_.sendLeftOsPrivacyPage();
+      }
       return;
     }
-
+    if (this.isHatsSurveyEnabled_) {
+      this.privacyHubBrowserProxy_.sendOpenedOsPrivacyPage();
+    }
     this.attemptDeepLink();
   }
 

chrome/browser/resources/settings/chromeos/os_privacy_page/privacy_hub_browser_proxy.ts

@@ -6,6 +6,8 @@ import {sendWithPromise} from 'chrome://resources/js/cr.js';
 
 export interface PrivacyHubBrowserProxy {
   getInitialMicrophoneHardwareToggleState(): Promise<boolean>;
+  sendLeftOsPrivacyPage(): void;
+  sendOpenedOsPrivacyPage(): void;
 }
 
 let instance: PrivacyHubBrowserProxy|null = null;
@@ -15,6 +17,14 @@ export class PrivacyHubBrowserProxyImpl implements PrivacyHubBrowserProxy {
     return sendWithPromise('getInitialMicrophoneHardwareToggleState');
   }
 
+  sendLeftOsPrivacyPage(): void {
+    chrome.send('leftOsPrivacyPage');
+  }
+
+  sendOpenedOsPrivacyPage(): void {
+    chrome.send('osPrivacyPageWasOpened');
+  }
+
   static getInstance(): PrivacyHubBrowserProxy {
     return instance || (instance = new PrivacyHubBrowserProxyImpl());
   }

chrome/browser/ui/webui/settings/ash/privacy_hub_handler.cc

@@ -4,25 +4,43 @@
 
 #include "chrome/browser/ui/webui/settings/ash/privacy_hub_handler.h"
 
+#include "ash/constants/ash_features.h"
 #include "base/functional/bind.h"
 #include "base/logging.h"
+#include "chrome/browser/ash/privacy_hub/privacy_hub_hats_trigger.h"
 #include "chrome/browser/ash/privacy_hub/privacy_hub_util.h"
+#include "chrome/common/chrome_features.h"
 
 namespace ash::settings {
 
 PrivacyHubHandler::PrivacyHubHandler() = default;
 
 PrivacyHubHandler::~PrivacyHubHandler() {
+  TriggerHatsIfPageWasOpened();
   privacy_hub_util::SetFrontend(nullptr);
 }
 
 void PrivacyHubHandler::RegisterMessages() {
-  privacy_hub_util::SetFrontend(this);
-  web_ui()->RegisterMessageCallback(
-      "getInitialMicrophoneHardwareToggleState",
-      base::BindRepeating(
-          &PrivacyHubHandler::HandleInitialMicrophoneSwitchState,
-          base::Unretained(this)));
+  if (ash::features::IsCrosPrivacyHubEnabled()) {
+    privacy_hub_util::SetFrontend(this);
+    web_ui()->RegisterMessageCallback(
+        "getInitialMicrophoneHardwareToggleState",
+        base::BindRepeating(
+            &PrivacyHubHandler::HandleInitialMicrophoneSwitchState,
+            base::Unretained(this)));
+  }
+
+  if (base::FeatureList::IsEnabled(
+          ::features::kHappinessTrackingPrivacyHubBaseline)) {
+    web_ui()->RegisterMessageCallback(
+        "osPrivacyPageWasOpened",
+        base::BindRepeating(&PrivacyHubHandler::HandlePrivacyPageOpened,
+                            base::Unretained(this)));
+    web_ui()->RegisterMessageCallback(
+        "leftOsPrivacyPage",
+        base::BindRepeating(&PrivacyHubHandler::HandlePrivacyPageClosed,
+                            base::Unretained(this)));
+  }
 }
 
 void PrivacyHubHandler::NotifyJS(const std::string& event_name,
@@ -37,6 +55,7 @@ void PrivacyHubHandler::NotifyJS(const std::string& event_name,
 
 void PrivacyHubHandler::HandleInitialMicrophoneSwitchState(
     const base::Value::List& args) {
+  DCHECK(ash::features::IsCrosPrivacyHubEnabled());
   AllowJavascript();
 
   DCHECK_GE(1U, args.size()) << ": Did not expect arguments";
@@ -49,7 +68,34 @@ void PrivacyHubHandler::HandleInitialMicrophoneSwitchState(
 }
 
 void PrivacyHubHandler::MicrophoneHardwareToggleChanged(bool muted) {
+  DCHECK(ash::features::IsCrosPrivacyHubEnabled());
   NotifyJS("microphone-hardware-toggle-changed", base::Value(muted));
 }
 
+void PrivacyHubHandler::HandlePrivacyPageOpened(const base::Value::List& args) {
+  DCHECK(args.empty());
+  DCHECK(base::FeatureList::IsEnabled(
+      ::features::kHappinessTrackingPrivacyHubBaseline));
+
+  AllowJavascript();
+
+  privacy_page_was_opened_ = true;
+}
+
+void PrivacyHubHandler::HandlePrivacyPageClosed(const base::Value::List& args) {
+  DCHECK(args.empty());
+  DCHECK(base::FeatureList::IsEnabled(
+      ::features::kHappinessTrackingPrivacyHubBaseline));
+
+  AllowJavascript();
+
+  TriggerHatsIfPageWasOpened();
+}
+
+void PrivacyHubHandler::TriggerHatsIfPageWasOpened() {
+  if (privacy_page_was_opened_) {
+    PrivacyHubHatsTrigger::Get().ShowSurveyAfterDelayElapsed();
+  }
+}
+
 }  // namespace ash::settings