[Launcher impressions] Record ignores and don't record empty results

There's an extra kind of result action (impression, launch, abandon)
that we don't currently log: ignores. These happen when results are
shown (impression logged) but not used, in such a way that it's not
reasonable to log an abandon. We don't have much interest in ignores
on their own, but they're useful for validating that the metrics are
correct. Let's log them.

Bundled with the change: we're erroneously triggering impressions and
abandons sometimes when there are zero results of a kind displayed. This
adds an extra check.

Bug: 1097599
Change-Id: Ib02ab5a0adb8e986c29375cd022fabf24d31fc06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2301530Reviewed-by: Rachel Wong <wrong@chromium.org>
Commit-Queue: Tony Yeoman <tby@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789805}

chrome/browser/ui/app_list/app_list_notifier_impl.cc

@@ -159,7 +159,8 @@ void AppListNotifierImpl::DoStateTransition(Location location,
   // Notify of impression on kShown -> {kSeen, kIgnored, kLaunched}.
   if (old_state == State::kShown &&
       (new_state == State::kSeen || new_state == State::kLaunched ||
-       new_state == State::kIgnored)) {
+       new_state == State::kIgnored) &&
+      !results_[location].empty()) {
     for (auto& observer : observers_) {
       observer.OnImpression(location, results_[location], query_);
     }
@@ -173,7 +174,7 @@ void AppListNotifierImpl::DoStateTransition(Location location,
   }
 
   // Notify of ignore on * -> kIgnored.
-  if (new_state == State::kIgnored) {
+  if (new_state == State::kIgnored && !results_[location].empty()) {
     for (auto& observer : observers_) {
       observer.OnIgnore(location, results_[location], query_);
     }
@@ -181,7 +182,8 @@ void AppListNotifierImpl::DoStateTransition(Location location,
 
   // Notify of abandon on kSeen -> {kNone, kShown}.
   if (old_state == State::kSeen &&
-      (new_state == State::kNone || new_state == State::kShown)) {
+      (new_state == State::kNone || new_state == State::kShown) &&
+      !results_[location].empty()) {
     for (auto& observer : observers_) {
       observer.OnAbandon(location, results_[location], query_);
     }

chrome/browser/ui/app_list/search/search_metrics_observer.cc

@@ -32,7 +32,8 @@ enum class Action {
   kImpression = 0,
   kLaunch = 1,
   kAbandon = 2,
-  kMaxValue = kAbandon
+  kIgnore = 3,
+  kMaxValue = kIgnore
 };
 
 void LogError(Error error) {
@@ -127,6 +128,12 @@ void SearchMetricsObserver::OnLaunch(ash::AppListNotifier::Location location,
   LogOverallAction(location, query, Action::kLaunch);
 }
 
+void SearchMetricsObserver::OnIgnore(ash::AppListNotifier::Location location,
+                                     const std::vector<std::string>& results,
+                                     const base::string16& query) {
+  LogOverallAction(location, query, Action::kIgnore);
+}
+
 base::Optional<ash::SearchResultType> SearchMetricsObserver::GetType(
     const std::string& result_id) {
   const auto* result = controller_->FindSearchResult(result_id);

chrome/browser/ui/app_list/search/search_metrics_observer.h

@@ -40,6 +40,9 @@ class SearchMetricsObserver : ash::AppListNotifier::Observer {
                 const std::string& launched,
                 const std::vector<std::string>& shown,
                 const base::string16& query) override;
+  void OnIgnore(ash::AppListNotifier::Location location,
+                const std::vector<std::string>& results,
+                const base::string16& query) override;
 
  private:
   // Looks up the ChromeSearchResult object in SearchController that corresponds

tools/metrics/histograms/enums.xml

@@ -2888,6 +2888,7 @@ Unknown properties are collapsed to zero. -->
   <int value="0" label="Impression"/>
   <int value="1" label="Launch"/>
   <int value="2" label="Abandon"/>
+  <int value="3" label="Ignore"/>
 </enum>
 
 <enum name="AppListUserEventError">