[iOS][omnibox][M110] Fix pedal metrics

Start logging Omnibox.SuggestionUsed.Pedal. Move Omnibox.PedalShown
logging to ios/. Disable cross-platform logging on ios for these metrics
to prevent double-logging.

(cherry picked from commit 81c9d8d)

Bug: 1403460
Change-Id: I11cca9679662d66b5da3a7a547c7a1be79c634dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4124116
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Auto-Submit: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1089872}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4151787
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Christian Xu <christianxu@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#199}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}

components/omnibox/browser/actions/omnibox_pedal.cc

@@ -294,12 +294,15 @@ bool OmniboxPedal::IsConceptMatch(TokenSequence& match_sequence) const {
 }
 
 void OmniboxPedal::RecordActionShown(size_t /*position*/, bool executed) const {
+  // Action metrics are recorded in the UI layer on iOS.
+#if !BUILDFLAG(IS_IOS)
   base::UmaHistogramEnumeration("Omnibox.PedalShown", GetMetricsId(),
                                 OmniboxPedalId::TOTAL_COUNT);
   if (executed) {
     base::UmaHistogramEnumeration("Omnibox.SuggestionUsed.Pedal",
                                   GetMetricsId(), OmniboxPedalId::TOTAL_COUNT);
   }
+#endif  // BUILDFLAG(IS_IOS)
 }
 
 size_t OmniboxPedal::EstimateMemoryUsage() const {

ios/chrome/browser/ui/location_bar/location_bar_coordinator.mm

@@ -102,8 +102,6 @@ @interface LocationBarCoordinator () <LoadQueryCommands,
 }
 // Whether the coordinator is started.
 @property(nonatomic, assign, getter=isStarted) BOOL started;
-// Coordinator for the omnibox popup.
-@property(nonatomic, strong) OmniboxPopupCoordinator* omniboxPopupCoordinator;
 // Mediator for the badges displayed in the LocationBar.
 @property(nonatomic, strong) BadgeMediator* badgeMediator;
 // ViewController for the badges displayed in the LocationBar.
@@ -179,6 +177,7 @@ - (void)start {
       [[OmniboxCoordinator alloc] initWithBaseViewController:nil
                                                      browser:self.browser];
   self.omniboxCoordinator.editController = _editController.get();
+  self.omniboxCoordinator.presenterDelegate = self.popupPresenterDelegate;
   [self.omniboxCoordinator start];
 
   [self.omniboxCoordinator.managedViewController
@@ -191,10 +190,6 @@ - (void)start {
       didMoveToParentViewController:self.viewController];
   self.viewController.offsetProvider = [self.omniboxCoordinator offsetProvider];
 
-  self.omniboxPopupCoordinator = [self.omniboxCoordinator
-      createPopupCoordinator:self.popupPresenterDelegate];
-  [self.omniboxPopupCoordinator start];
-
   // Create button factory that wil be used by the ViewController to get
   // BadgeButtons for a BadgeType.
   BadgeButtonFactory* buttonFactory = [[BadgeButtonFactory alloc] init];
@@ -250,7 +245,6 @@ - (void)stop {
     return;
   [self.browser->GetCommandDispatcher() stopDispatchingToTarget:self];
   // The popup has to be destroyed before the location bar.
-  [self.omniboxPopupCoordinator stop];
   [self.omniboxCoordinator stop];
   [self.badgeMediator disconnect];
   self.badgeMediator = nil;
@@ -271,11 +265,11 @@ - (void)stop {
 }
 
 - (BOOL)omniboxPopupHasAutocompleteResults {
-  return self.omniboxPopupCoordinator.hasResults;
+  return self.omniboxCoordinator.popupCoordinator.hasResults;
 }
 
 - (BOOL)showingOmniboxPopup {
-  return self.omniboxPopupCoordinator.isOpen;
+  return self.omniboxCoordinator.popupCoordinator.isOpen;
 }
 
 - (BOOL)isOmniboxFirstResponder {

ios/chrome/browser/ui/omnibox/omnibox_coordinator.h

@@ -17,12 +17,19 @@ class WebOmniboxEditController;
 // The coordinator for the omnibox.
 @interface OmniboxCoordinator : ChromeCoordinator
 
+// Returns a popup coordinator created by this coordinator.
+// Created and started at `start` and stopped & destroyed at `stop`.
+@property(nonatomic, strong, readonly)
+    OmniboxPopupCoordinator* popupCoordinator;
 // The edit controller interfacing the `textField` and the omnibox components
 // code. Needs to be set before the coordinator is started.
 @property(nonatomic, assign) WebOmniboxEditController* editController;
 // Returns the animatee for the omnibox focus orchestrator.
 @property(nonatomic, strong, readonly) id<EditViewAnimatee> animatee;
 
+/// Positioner for the popup. Has to be configured before calling `start`.
+@property(nonatomic, weak) id<OmniboxPopupPresenterDelegate> presenterDelegate;
+
 // The view controller managed by this coordinator. The parent of this
 // coordinator is expected to add it to the responder chain.
 - (UIViewController*)managedViewController;
@@ -54,10 +61,6 @@ class WebOmniboxEditController;
 
 // Use this method to resign `textField` as the first responder.
 - (void)endEditing;
-// Creates a child popup coordinator. The popup coordinator is linked to the
-// `textField` through components code.
-- (OmniboxPopupCoordinator*)createPopupCoordinator:
-    (id<OmniboxPopupPresenterDelegate>)presenterDelegate;
 
 @end
 

ios/chrome/browser/ui/omnibox/omnibox_coordinator.mm

@@ -82,6 +82,9 @@ @interface OmniboxCoordinator () <OmniboxViewControllerTextInputDelegate>
 @property(nonatomic, strong)
     OmniboxKeyboardAccessoryView* keyboardAccessoryView;
 
+// Redefined as readwrite.
+@property(nonatomic, strong) OmniboxPopupCoordinator* popupCoordinator;
+
 @end
 
 @implementation OmniboxCoordinator {
@@ -97,6 +100,8 @@ @implementation OmniboxCoordinator {
 #pragma mark - public
 
 - (void)start {
+  DCHECK(!self.popupCoordinator);
+
   BOOL isIncognito = self.browser->GetBrowserState()->IsOffTheRecord();
 
   self.viewController =
@@ -161,9 +166,15 @@ - (void)start {
           initWithWebStateList:self.browser->GetWebStateList()
         autocompleteController:_editView->model()->autocomplete_controller()];
   }
+
+  self.popupCoordinator = [self createPopupCoordinator:self.presenterDelegate];
+  [self.popupCoordinator start];
 }
 
 - (void)stop {
+  [self.popupCoordinator stop];
+  self.popupCoordinator = nil;
+
   self.viewController.textChangeDelegate = nil;
   self.returnDelegate.acceptDelegate = nil;
   _editView.reset();
@@ -235,6 +246,7 @@ - (void)insertTextToOmnibox:(NSString*)text {
 
 - (OmniboxPopupCoordinator*)createPopupCoordinator:
     (id<OmniboxPopupPresenterDelegate>)presenterDelegate {
+  DCHECK(!_popupCoordinator);
   std::unique_ptr<OmniboxPopupViewIOS> popupView =
       std::make_unique<OmniboxPopupViewIOS>(_editView->model(),
                                             _editView.get());
@@ -256,6 +268,7 @@ - (OmniboxPopupCoordinator*)createPopupCoordinator:
   self.viewController.returnKeyDelegate = coordinator.popupReturnDelegate;
   self.viewController.popupKeyboardDelegate = coordinator.KeyboardDelegate;
 
+  _popupCoordinator = coordinator;
   return coordinator;
 }
 

ios/chrome/browser/ui/omnibox/popup/omnibox_popup_mediator.mm

@@ -7,11 +7,13 @@
 #import "base/feature_list.h"
 #import "base/ios/ios_util.h"
 #import "base/metrics/histogram_functions.h"
+#import "base/metrics/histogram_macros.h"
 #import "base/metrics/user_metrics.h"
 #import "base/metrics/user_metrics_action.h"
 #import "base/strings/sys_string_conversions.h"
 #import "base/strings/utf_string_conversions.h"
 #import "components/image_fetcher/core/image_data_fetcher.h"
+#import "components/omnibox/browser/actions/omnibox_action_concepts.h"
 #import "components/omnibox/browser/autocomplete_controller.h"
 #import "components/omnibox/browser/autocomplete_input.h"
 #import "components/omnibox/browser/autocomplete_match.h"
@@ -59,6 +61,8 @@ @interface OmniboxPopupMediator () <PedalSectionExtractorDelegate>
 /// List of suggestions without the pedal group. Used to debouce pedals.
 @property(nonatomic, strong)
     NSArray<id<AutocompleteSuggestionGroup>>* nonPedalSuggestions;
+/// Holds the currently displayed pedals group, if any.
+@property(nonatomic, strong) id<AutocompleteSuggestionGroup> currentPedals;
 /// Index of the group containing AutocompleteSuggestion, first group to be
 /// highlighted on down arrow key.
 @property(nonatomic, assign) NSInteger preselectedGroupIndex;
@@ -105,6 +109,7 @@ - (instancetype)initWithFetcher:
 
 - (void)updateMatches:(const AutocompleteResult&)result {
   self.nonPedalSuggestions = nil;
+  self.currentPedals = nil;
 
   self.hasResults = !self.autocompleteResult.empty();
   if (base::FeatureList::IsEnabled(omnibox::kAdaptiveSuggestionsCount)) {
@@ -179,10 +184,16 @@ - (void)autocompleteResultConsumerDidChangeTraitCollection:
 - (void)autocompleteResultConsumer:(id<AutocompleteResultConsumer>)sender
                didSelectSuggestion:(id<AutocompleteSuggestion>)suggestion
                              inRow:(NSUInteger)row {
+  [self logPedalShownForCurrentResult];
+
   if ([suggestion isKindOfClass:[PedalSuggestionWrapper class]]) {
     PedalSuggestionWrapper* pedalSuggestionWrapper =
         (PedalSuggestionWrapper*)suggestion;
     if (pedalSuggestionWrapper.innerPedal.action) {
+      base::UmaHistogramEnumeration(
+          "Omnibox.SuggestionUsed.Pedal",
+          (OmniboxPedalId)pedalSuggestionWrapper.innerPedal.type,
+          OmniboxPedalId::TOTAL_COUNT);
       pedalSuggestionWrapper.innerPedal.action();
     }
   } else if ([suggestion isKindOfClass:[AutocompleteMatchFormatter class]]) {
@@ -346,13 +357,22 @@ - (void)fetchFavicon:(GURL)pageURL completion:(void (^)(UIImage*))completion {
 /// pedal group is removed.
 - (void)invalidatePedals {
   if (self.nonPedalSuggestions) {
+    self.currentPedals = nil;
     [self.consumer updateMatches:self.nonPedalSuggestions
         preselectedMatchGroupIndex:0];
   }
 }
 
 #pragma mark - Private methods
 
+- (void)logPedalShownForCurrentResult {
+  for (PedalSuggestionWrapper* pedalMatch in self.currentPedals.suggestions) {
+    base::UmaHistogramEnumeration("Omnibox.PedalShown",
+                                  (OmniboxPedalId)pedalMatch.innerPedal.type,
+                                  OmniboxPedalId::TOTAL_COUNT);
+  }
+}
+
 /// Wraps `match` with AutocompleteMatchFormatter.
 - (AutocompleteMatchFormatter*)wrapMatch:(const AutocompleteMatch&)match
                               fromResult:(const AutocompleteResult&)result {
@@ -489,15 +509,14 @@ - (AutocompleteMatchFormatter*)wrapMatch:(const AutocompleteMatch&)match
   self.nonPedalSuggestions = groups;
 
   // Get pedals, if any. They go at the very top of the list.
-  id<AutocompleteSuggestionGroup> pedalGroup =
-      [self.pedalSectionExtractor extractPedals:allMatches];
-  if (pedalGroup) {
-    [groups insertObject:pedalGroup atIndex:0];
+  self.currentPedals = [self.pedalSectionExtractor extractPedals:allMatches];
+  if (self.currentPedals) {
+    [groups insertObject:self.currentPedals atIndex:0];
   }
 
   // Preselect the verbatim match. It's the top match, unless we inserted pedals
   // and pushed it one section down.
-  self.preselectedGroupIndex = pedalGroup ? MIN(1, groups.count) : 0;
+  self.preselectedGroupIndex = self.currentPedals ? MIN(1, groups.count) : 0;
 
   return groups;
 }