[waffle]Add histograms to record the choice location

Add histograms to record whether the search engine choice was made
in the choice screen or in the settings pages.

Fixed: b:306580764
Change-Id: I4dfe1638638056c467408533a64ba8a22fb6aae6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4964800
Commit-Queue: Jack Yammine <jyammine@google.com>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org>
Auto-Submit: Jack Yammine <jyammine@google.com>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216893}

chrome/browser/ui/webui/settings/search_engines_handler.h

@@ -50,9 +50,6 @@ class SearchEnginesHandler : public SettingsPageUIHandler,
 
  private:
   friend class SearchEnginesHandlerTestBase;
-  FRIEND_TEST_ALL_PREFIXES(
-      SearchEnginesHandlerTestWithSearchEngineChoiceEnabled,
-      ModifyingSearchEngineSetsSearchEngineChoiceTimestamp);
 
   // Retrieves all search engines and returns them to WebUI.
   void HandleGetSearchEnginesList(const base::Value::List& args);

chrome/browser/ui/webui/settings/search_engines_handler_unittest.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/webui/settings/search_engines_handler.h"
 #include "base/functional/bind.h"
 #include "base/memory/raw_ptr.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/time/time.h"
 #include "chrome/browser/profiles/profile.h"
@@ -58,15 +59,18 @@ class SearchEnginesHandlerTestBase : public testing::Test {
     web_ui_.set_web_contents(web_contents_factory_.CreateWebContents(profile_));
     handler_->set_web_ui(&web_ui_);
     handler()->AllowJavascript();
+    handler()->RegisterMessages();
     web_ui()->ClearTrackedCalls();
   }
 
   content::TestWebUI* web_ui() { return &web_ui_; }
   Profile* profile() const { return profile_; }
   SearchEnginesHandler* handler() const { return handler_.get(); }
   base::test::ScopedFeatureList* feature_list() { return &feature_list_; }
+  base::HistogramTester& histogram_tester() { return histogram_tester_; }
 
  private:
+  base::HistogramTester histogram_tester_;
   base::test::ScopedFeatureList feature_list_;
   content::BrowserTaskEnvironment task_environment_;
   TestingProfileManager profile_manager_;
@@ -118,6 +122,31 @@ TEST_P(SearchEnginesHandlerParametrizedTest,
   EXPECT_EQ("search-engines-changed", second_call_data.arg1()->GetString());
 }
 
+TEST_P(SearchEnginesHandlerParametrizedTest,
+       SettingTheDefaultSearchEngineRecordsHistogram) {
+  base::Value::List first_call_args;
+  // Search engine model id.
+  first_call_args.Append(1);
+  first_call_args.Append(static_cast<int>(
+      search_engines::ChoiceMadeLocation::kSearchEngineSettings));
+  web_ui()->HandleReceivedMessage("setDefaultSearchEngine", first_call_args);
+
+  histogram_tester().ExpectUniqueSample(
+      search_engines::kDefaultSearchEngineChoiceLocationHistogram,
+      search_engines::ChoiceMadeLocation::kSearchEngineSettings, 1);
+
+  base::Value::List second_call_args;
+  // Search engine model id.
+  second_call_args.Append(1);
+  second_call_args.Append(
+      static_cast<int>(search_engines::ChoiceMadeLocation::kSearchSettings));
+  web_ui()->HandleReceivedMessage("setDefaultSearchEngine", second_call_args);
+
+  histogram_tester().ExpectBucketCount(
+      search_engines::kDefaultSearchEngineChoiceLocationHistogram,
+      search_engines::ChoiceMadeLocation::kSearchSettings, 1);
+}
+
 class SearchEnginesHandlerTestWithSearchEngineChoiceEnabled
     : public SearchEnginesHandlerTestBase {
  public:
@@ -143,11 +172,15 @@ TEST_F(SearchEnginesHandlerTestWithSearchEngineChoiceEnabled,
   args.Append(1);
   args.Append(static_cast<int>(
       search_engines::ChoiceMadeLocation::kSearchEngineSettings));
-  handler()->HandleSetDefaultSearchEngine(args);
+  web_ui()->HandleReceivedMessage("setDefaultSearchEngine", args);
 
   EXPECT_NEAR(pref_service->GetInt64(
                   prefs::kDefaultSearchProviderChoiceScreenCompletionTimestamp),
               base::Time::Now().ToDeltaSinceWindowsEpoch().InSeconds(),
               /*abs_error=*/2);
+
+  histogram_tester().ExpectUniqueSample(
+      search_engines::kDefaultSearchEngineChoiceLocationHistogram,
+      search_engines::ChoiceMadeLocation::kSearchEngineSettings, 1);
 }
 }  // namespace settings

components/search_engines/search_engine_choice_utils.cc

@@ -114,6 +114,9 @@ const char kSearchEngineChoiceScreenProfileInitConditionsHistogram[] =
 const char kSearchEngineChoiceScreenEventsHistogram[] =
     "Search.ChoiceScreenEvents";
 
+const char kDefaultSearchEngineChoiceLocationHistogram[] =
+    "Search.DefaultSearchEngineChoiceLocation";
+
 // Returns whether the choice screen flag is generally enabled for the specific
 // user flow.
 bool IsChoiceScreenFlagEnabled(ChoicePromo promo) {
@@ -235,6 +238,11 @@ void RecordChoiceScreenEvent(SearchEngineChoiceScreenEvents event) {
 
 void RecordChoiceMade(PrefService* profile_prefs,
                       ChoiceMadeLocation choice_location) {
+  // Record the histogram even if the feature is not enabled.
+  base::UmaHistogramEnumeration(
+      search_engines::kDefaultSearchEngineChoiceLocationHistogram,
+      choice_location);
+
   if (!IsChoiceScreenFlagEnabled(ChoicePromo::kAny)) {
     return;
   }

components/search_engines/search_engine_choice_utils.h

@@ -19,6 +19,7 @@ namespace search_engines {
 extern const char kSearchEngineChoiceScreenProfileInitConditionsHistogram[];
 extern const char kSearchEngineChoiceScreenNavigationConditionsHistogram[];
 extern const char kSearchEngineChoiceScreenEventsHistogram[];
+extern const char kDefaultSearchEngineChoiceLocationHistogram[];
 
 // These values are persisted to logs. Entries should not be renumbered and
 // numeric values should never be reused.
@@ -81,7 +82,7 @@ enum class ChoicePromo {
   kFre = 2,
 };
 
-//  The location from which the search engine choice was made.
+//  The location from which the default search engine was set.
 //  These values are persisted to logs. Entries should not be renumbered and
 //  numeric values should never be reused.
 //  Must be kept in sync with the ChoiceMadeLocation enum in
@@ -95,6 +96,7 @@ enum class ChoiceMadeLocation {
   // The search engine choice dialog for existing users or the profile picker
   // for new users.
   kChoiceScreen = 2,
+  kMaxValue = kChoiceScreen,
 };
 
 // Whether the choice screen flag is generally enabled for the specific flow.
@@ -124,8 +126,6 @@ bool IsEeaChoiceCountry(int country_id);
 
 // Records that the choice was made by settings the timestamp if applicable.
 // Records the location from which the choice was made.
-// TODO(b/306580764): Use `choice_location` to record histograms of the location
-// where the choice was made from.
 void RecordChoiceMade(PrefService* profile_prefs,
                       ChoiceMadeLocation choice_location);
 

components/search_engines/search_engine_choice_utils_unittest.cc

@@ -373,6 +373,10 @@ TEST_F(SearchEngineChoiceUtilsTest, RecordChoiceMade) {
   EXPECT_FALSE(pref_service()->HasPrefPath(
       prefs::kDefaultSearchProviderChoiceScreenCompletionTimestamp));
 
+  histogram_tester_.ExpectUniqueSample(
+      search_engines::kDefaultSearchEngineChoiceLocationHistogram,
+      search_engines::ChoiceMadeLocation::kChoiceScreen, 1);
+
   // Revert to an EEA region country.
   const int kBelgiumCountryId =
       country_codes::CountryCharsToCountryID('B', 'E');
@@ -388,6 +392,10 @@ TEST_F(SearchEngineChoiceUtilsTest, RecordChoiceMade) {
               base::Time::Now().ToDeltaSinceWindowsEpoch().InSeconds(),
               /*abs_error=*/2);
 
+  histogram_tester_.ExpectUniqueSample(
+      search_engines::kDefaultSearchEngineChoiceLocationHistogram,
+      search_engines::ChoiceMadeLocation::kChoiceScreen, 2);
+
   // Set the pref to 5 so that we can know if it gets modified.
   const int kModifiedTimestamp = 5;
   pref_service()->SetInt64(
@@ -400,4 +408,8 @@ TEST_F(SearchEngineChoiceUtilsTest, RecordChoiceMade) {
   EXPECT_EQ(pref_service()->GetInt64(
                 prefs::kDefaultSearchProviderChoiceScreenCompletionTimestamp),
             kModifiedTimestamp);
+
+  histogram_tester_.ExpectUniqueSample(
+      search_engines::kDefaultSearchEngineChoiceLocationHistogram,
+      search_engines::ChoiceMadeLocation::kChoiceScreen, 3);
 }

tools/metrics/histograms/enums.xml

@@ -16098,6 +16098,16 @@ Called by update_protocol_commands_enum.py.-->
   <int value="6" label="Notification Action Click"/>
 </enum>
 
+<enum name="ChoiceMadeLocation">
+  <int value="0" label="Search settings page (Desktop only)"/>
+  <int value="1"
+      label="Manage search engines and site search page (desktop) OR search
+             engine settings screen (mobile)"/>
+  <int value="2"
+      label="Choice screen (rendered in dialog, profile picker, promo page
+             etc.., not accessed through settings)"/>
+</enum>
+
 <enum name="ChromeActivityType">
   <int value="0" label="Tabbed Chrome"/>
   <int value="1" label="Custom Tab"/>

tools/metrics/histograms/metadata/search/histograms.xml

@@ -1387,6 +1387,17 @@ chromium-metrics-reviews@google.com.
   </summary>
 </histogram>
 
+<histogram name="Search.DefaultSearchEngineChoiceLocation"
+    enum="ChoiceMadeLocation" expires_after="2024-03-31">
+  <owner>dgn@chromium.org</owner>
+  <owner>chrome-waffle-eng@google.com</owner>
+  <summary>
+    Records the location from which the default search engine was set. Emitted
+    when the default search engine is changed in the settings page or in the
+    search engine choice screen.
+  </summary>
+</histogram>
+
 <histogram name="Search.DefaultSearchProviderType"
     enum="OmniboxSearchEngineType" expires_after="2023-02-12">
   <obsolete>