[SEC] Fix metrics naming and reporting issues

Bug: 1001839
Change-Id: Id27380f95b5c06f938826d4ec9a4725577034718
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1861041Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706252}

chrome/android/java/src/org/chromium/chrome/browser/search_engines/SearchEngineChoiceMetrics.java

@@ -50,13 +50,13 @@ public class SearchEngineChoiceMetrics {
     public @interface EventsV2 {
         int CHOICE_REQUEST_RECEIVED = 0;
         int CHOICE_SKIPPED = 1;
-        int CHOICE_REQUEST_NO_DATA = 1;
-        int CHOICE_REQUEST_VALID = 2;
-        int CHOICE_REQUEST_METADATA_NULL = 3;
-        int CHOICE_REQUEST_PARSE_FAILED = 4;
-        int PREVIOUS_CHOICE_REQUEST_FAILED = 5;
-        int CHOICE_REQUEST_SUCCESS = 6;
-        int MAX = 7;
+        int CHOICE_REQUEST_NO_DATA = 2;
+        int CHOICE_REQUEST_VALID = 3;
+        int CHOICE_REQUEST_METADATA_NULL = 4;
+        int CHOICE_REQUEST_PARSE_FAILED = 5;
+        int PREVIOUS_CHOICE_REQUEST_FAILED = 6;
+        int CHOICE_REQUEST_SUCCESS = 7;
+        int MAX = 8;
     }
 
     /**
@@ -89,21 +89,25 @@ public class SearchEngineChoiceMetrics {
         setPreviousSearchEngineType(currentSearchEngineType);
     }
 
-    /** Records the search engine type after the user made a choice about which engine to use. */
-    public static void recordSearchEngineTypeAfterChoice() {
-        if (!isSearchEnginePossiblyDifferent()) return;
+    /**
+     * Records the search engine type after the user chooses a different search engine.
+     * @return Whether the search engine was changed.
+     **/
+    public static boolean recordSearchEngineTypeAfterChoice() {
+        if (!isSearchEnginePossiblyDifferent()) return false;
 
         @SearchEngineType
         int previousSearchEngineType = getPreviousSearchEngineType();
         @SearchEngineType
         int currentSearchEngineType = getDefaultSearchEngineType();
-        if (previousSearchEngineType != currentSearchEngineType) {
-            recordEvent(Events.SEARCH_ENGINE_CHANGED);
+        boolean didChangeEngine = previousSearchEngineType != currentSearchEngineType;
+        if (didChangeEngine) {
             RecordHistogram.recordEnumeratedHistogram(
                     "Android.SearchEngineChoice.ChosenSearchEngine", currentSearchEngineType,
                     SearchEngineType.SEARCH_ENGINE_MAX);
         }
         removePreviousSearchEngineType();
+        return didChangeEngine;
     }
 
     /** @return True if the current search engine is possibly different from the previous one. */

chrome/android/java/src/org/chromium/chrome/browser/search_engines/SearchEngineChoiceNotification.java

@@ -92,7 +92,10 @@ public final class SearchEngineChoiceNotification {
             updateSearchEngineChoicePresented();
             SearchEngineChoiceMetrics.recordEvent(SearchEngineChoiceMetrics.Events.SNACKBAR_SHOWN);
         } else {
-            SearchEngineChoiceMetrics.recordSearchEngineTypeAfterChoice();
+            if (SearchEngineChoiceMetrics.recordSearchEngineTypeAfterChoice()) {
+                SearchEngineChoiceMetrics.recordEvent(
+                        SearchEngineChoiceMetrics.Events.SEARCH_ENGINE_CHANGED);
+            }
         }
     }
 

chrome/android/junit/src/org/chromium/chrome/browser/search_engines/SearchEngineChoiceMetricsTest.java

@@ -119,6 +119,10 @@ public final class SearchEngineChoiceMetricsTest {
         assertEquals(0,
                 ShadowRecordHistogram.getHistogramValueCountForTesting(
                         HISTOGRAM_AFTER_CHOICE, SearchEngineType.SEARCH_ENGINE_GOOGLE));
+        assertEquals(0,
+                ShadowRecordHistogram.getHistogramValueCountForTesting(
+                        "Android.SearchEngineChoice.EventsV2",
+                        SearchEngineChoiceMetrics.Events.SEARCH_ENGINE_CHANGED));
     }
 
     @Test

tools/metrics/histograms/enums.xml

@@ -1744,7 +1744,7 @@ Unknown properties are collapsed to zero. -->
 
 <enum name="AndroidSearchEngineChoiceEventsV2">
   <summary>Events related to Search Engine Choice V2 feature.</summary>
-  <int value="0" label="Request recieved from play"/>
+  <int value="0" label="Request received from play"/>
   <int value="1"
       label="User skipped (didn't complete) Search Engine Choice flow"/>
   <int value="2" label="Play API returned empty list of providers"/>