Update and add UMA metrics for assistive features.

- Record coverage metrics only when we show the suggestions
- Change current coverage metrics to match metrics.
- Add disabled metrics

Previously we were recording coverage metrics for personal info in a
wrong place. Now we moved it to the correct place and added a new metric
for all the match scenarios.

Add disabled metrics for cases where we could show suggestion but the
feature is turned off.

For personal info suggester, we only record coverage metrics when we
first show a suggestion so that we won't record dup metrics when the
user keeps typing with suggestion shown.

Test: tested with Chrome on Linux
Bug: 1042084
Change-Id: I6559d81f8d3fc5fa2f9385a4df617f0ac06c5848
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2208577Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Jing Wang <jiwan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770857}

chrome/browser/chromeos/input_method/assistive_suggester.cc

@@ -22,6 +22,14 @@ namespace {
 const char kMaxTextBeforeCursorLength = 50;
 const char kKeydown[] = "keydown";
 
+void RecordAssistiveMatch(AssistiveType type) {
+  base::UmaHistogramEnumeration("InputMethod.Assistive.Match", type);
+}
+
+void RecordAssistiveDisabled(AssistiveType type) {
+  base::UmaHistogramEnumeration("InputMethod.Assistive.Disabled", type);
+}
+
 void RecordAssistiveCoverage(AssistiveType type) {
   base::UmaHistogramEnumeration("InputMethod.Assistive.Coverage", type);
 }
@@ -58,6 +66,23 @@ bool AssistiveSuggester::IsEmojiSuggestAdditionEnabled() {
   return enabled.value();
 }
 
+bool AssistiveSuggester::IsActionEnabled(AssistiveType action) {
+  switch (action) {
+    case AssistiveType::kPersonalEmail:
+    case AssistiveType::kPersonalAddress:
+    case AssistiveType::kPersonalPhoneNumber:
+    case AssistiveType::kPersonalName:
+      // TODO: Use value from settings when crbug/1068457 is done.
+      return IsAssistPersonalInfoEnabled();
+      break;
+    case AssistiveType::kEmoji:
+      return IsEmojiSuggestAdditionEnabled();
+    default:
+      break;
+  }
+  return false;
+}
+
 void AssistiveSuggester::OnFocus(int context_id) {
   context_id_ = context_id;
   personal_info_suggester_.OnFocus(context_id_);
@@ -98,10 +123,9 @@ bool AssistiveSuggester::OnKeyEvent(
   return false;
 }
 
-void AssistiveSuggester::RecordAssistiveCoverageMetrics(
-    const base::string16& text,
-    int cursor_pos,
-    int anchor_pos) {
+void AssistiveSuggester::RecordAssistiveMatchMetrics(const base::string16& text,
+                                                     int cursor_pos,
+                                                     int anchor_pos) {
   int len = static_cast<int>(text.length());
   if (cursor_pos > 0 && cursor_pos <= len && cursor_pos == anchor_pos &&
       (cursor_pos == len || base::IsAsciiWhitespace(text[cursor_pos]))) {
@@ -109,8 +133,11 @@ void AssistiveSuggester::RecordAssistiveCoverageMetrics(
     base::string16 text_before_cursor =
         text.substr(start_pos, cursor_pos - start_pos);
     AssistiveType action = ProposeAssistiveAction(text_before_cursor);
-    if (action != AssistiveType::kGenericAction)
-      RecordAssistiveCoverage(action);
+    if (action != AssistiveType::kGenericAction) {
+      RecordAssistiveMatch(action);
+      if (!IsActionEnabled(action))
+        RecordAssistiveDisabled(action);
+    }
   }
 }
 
@@ -145,6 +172,9 @@ bool AssistiveSuggester::Suggest(const base::string16& text,
     if (IsAssistPersonalInfoEnabled() &&
         personal_info_suggester_.Suggest(text_before_cursor)) {
       current_suggester_ = &personal_info_suggester_;
+      if (personal_info_suggester_.IsFirstShown()) {
+        RecordAssistiveCoverage(current_suggester_->GetProposeActionType());
+      }
       return true;
     } else if (IsEmojiSuggestAdditionEnabled() &&
                emoji_suggester_.Suggest(text_before_cursor)) {

chrome/browser/chromeos/input_method/assistive_suggester.h

@@ -35,9 +35,9 @@ class AssistiveSuggester {
 
   // Checks the text before cursor, emits metric if any assistive prefix is
   // matched.
-  void RecordAssistiveCoverageMetrics(const base::string16& text,
-                                      int cursor_pos,
-                                      int anchor_pos);
+  void RecordAssistiveMatchMetrics(const base::string16& text,
+                                   int cursor_pos,
+                                   int anchor_pos);
 
   // Called when a surrounding text is changed.
   // Returns true if it changes the surrounding text, e.g. a suggestion is
@@ -63,6 +63,8 @@ class AssistiveSuggester {
 
   bool IsEmojiSuggestAdditionEnabled();
 
+  bool IsActionEnabled(AssistiveType action);
+
   Profile* profile_;
   PersonalInfoSuggester personal_info_suggester_;
   EmojiSuggester emoji_suggester_;

chrome/browser/chromeos/input_method/native_input_method_engine.cc

@@ -199,8 +199,8 @@ void NativeInputMethodEngine::ImeObserver::OnSurroundingTextChanged(
     int cursor_pos,
     int anchor_pos,
     int offset_pos) {
-  assistive_suggester_->RecordAssistiveCoverageMetrics(text, cursor_pos,
-                                                       anchor_pos);
+  assistive_suggester_->RecordAssistiveMatchMetrics(text, cursor_pos,
+                                                    anchor_pos);
   if (assistive_suggester_->IsAssistiveFeatureEnabled()) {
     // If |assistive_suggester_| changes the surrounding text, no longer need
     // to call the following function, as the information is out-dated.

chrome/browser/chromeos/input_method/native_input_method_engine_browsertest.cc

@@ -6,6 +6,7 @@
 
 #include "base/guid.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
@@ -301,6 +302,8 @@ IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest, NoActiveController) {
 }
 
 IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest, SuggestUserEmail) {
+  base::HistogramTester histogram_tester;
+
   signin::IdentityManager* identity_manager =
       IdentityManagerFactory::GetForProfileIfExists(profile_);
   signin::SetPrimaryAccount(identity_manager, "johnwayne@me.xyz");
@@ -316,16 +319,27 @@ IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest, SuggestUserEmail) {
 
   helper.GetTextInputClient()->InsertText(prefix_text);
   helper.WaitForSurroundingTextChanged(prefix_text);
+  histogram_tester.ExpectUniqueSample("InputMethod.Assistive.Match",
+                                      chromeos::AssistiveType::kPersonalEmail,
+                                      1);
+  histogram_tester.ExpectUniqueSample("InputMethod.Assistive.Coverage",
+                                      chromeos::AssistiveType::kPersonalEmail,
+                                      1);
 
   DispatchKeyPress(ui::VKEY_TAB, false);
   helper.WaitForSurroundingTextChanged(expected_result_text);
 
   EXPECT_EQ(expected_result_text, helper.GetSurroundingText());
+  histogram_tester.ExpectUniqueSample("InputMethod.Assistive.Success",
+                                      chromeos::AssistiveType::kPersonalEmail,
+                                      1);
 
   SetFocus(nullptr);
 }
 
 IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest, DismissSuggestion) {
+  base::HistogramTester histogram_tester;
+
   signin::IdentityManager* identity_manager =
       IdentityManagerFactory::GetForProfileIfExists(profile_);
   signin::SetPrimaryAccount(identity_manager, "johnwayne@me.xyz");
@@ -349,11 +363,16 @@ IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest, DismissSuggestion) {
   helper.WaitForSurroundingTextChanged(expected_result_text);
 
   EXPECT_EQ(expected_result_text, helper.GetSurroundingText());
+  histogram_tester.ExpectUniqueSample("InputMethod.Assistive.Success",
+                                      chromeos::AssistiveType::kPersonalEmail,
+                                      0);
 
   SetFocus(nullptr);
 }
 
 IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest, SuggestUserName) {
+  base::HistogramTester histogram_tester;
+
   TestPersonalDataManagerObserver personal_data_observer(profile_);
   autofill::AutofillProfile autofill_profile(base::GenerateGUID(),
                                              autofill::test::kEmptyOrigin);
@@ -374,6 +393,11 @@ IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest, SuggestUserName) {
 
   helper.GetTextInputClient()->InsertText(prefix_text);
   helper.WaitForSurroundingTextChanged(prefix_text);
+  histogram_tester.ExpectUniqueSample(
+      "InputMethod.Assistive.Match", chromeos::AssistiveType::kPersonalName, 1);
+  histogram_tester.ExpectUniqueSample("InputMethod.Assistive.Coverage",
+                                      chromeos::AssistiveType::kPersonalName,
+                                      1);
 
   // Keep typing
   helper.GetTextInputClient()->InsertText(base::UTF8ToUTF16("jo"));
@@ -384,5 +408,13 @@ IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest, SuggestUserName) {
 
   EXPECT_EQ(expected_result_text, helper.GetSurroundingText());
 
+  // Make sure we do not emit multiple Coverage metrics when users keep typing.
+  histogram_tester.ExpectUniqueSample("InputMethod.Assistive.Coverage",
+                                      chromeos::AssistiveType::kPersonalName,
+                                      1);
+  histogram_tester.ExpectUniqueSample("InputMethod.Assistive.Success",
+                                      chromeos::AssistiveType::kPersonalName,
+                                      1);
+
   SetFocus(nullptr);
 }

chrome/browser/chromeos/input_method/personal_info_suggester.cc

@@ -220,7 +220,10 @@ void PersonalInfoSuggester::ShowSuggestion(const base::string16& text,
     LOG(ERROR) << "Fail to show suggestion. " << error;
   }
 
-  if (!suggestion_shown_) {
+  if (suggestion_shown_) {
+    first_shown_ = false;
+  } else {
+    first_shown_ = true;
     tts_handler_->Announce(
         // TODO(jiwan): Add translation to other languages when we support more
         // than English.

chrome/browser/chromeos/input_method/personal_info_suggester.h

@@ -65,6 +65,8 @@ class PersonalInfoSuggester : public Suggester {
       std::unique_ptr<TtsHandler> tts_handler = nullptr);
   ~PersonalInfoSuggester() override;
 
+  bool IsFirstShown() { return first_shown_; }
+
   // Suggester overrides:
   void OnFocus(int context_id) override;
   void OnBlur() override;
@@ -106,6 +108,9 @@ class PersonalInfoSuggester : public Suggester {
   // If we are showing a suggestion right now.
   bool suggestion_shown_ = false;
 
+  // If we are showing the suggestion for the first time.
+  bool first_shown_ = false;
+
   // The current suggestion text shown.
   base::string16 suggestion_;
 };

tools/metrics/histograms/histograms.xml

@@ -67300,6 +67300,31 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="InputMethod.Assistive.Disabled" enum="IMEAssistiveAction"
+    expires_after="2021-01-01">
+  <owner>jiwan@google.com</owner>
+  <owner>essential-inputs-team@google.com</owner>
+  <summary>
+    The number of times each assistive action could be triggered according to
+    the surrounding text but was not triggered because the user turned off the
+    feature. Recorded when the surrounding text could trigger assistive actions
+    but the corresponding feature was disabled.
+  </summary>
+</histogram>
+
+<histogram name="InputMethod.Assistive.Match" enum="IMEAssistiveAction"
+    expires_after="2021-01-01">
+  <owner>jiwan@google.com</owner>
+  <owner>essential-inputs-team@google.com</owner>
+  <summary>
+    The number of times each assistive action could be triggered according to
+    the surrounding text. This includes cases in InputMethod.Assistive.Coverage,
+    and also includes cases when the feature is turned off or there is
+    insufficient data. Recorded when the surrounding text could trigger
+    assistive actions.
+  </summary>
+</histogram>
+
 <histogram name="InputMethod.Assistive.Success" enum="IMEAssistiveAction"
     expires_after="2021-01-01">
   <owner>jiwan@google.com</owner>