[omnibox] Update pedal attachment and execution for button row

This CL updates pedal logic to work with and without the button row
feature enabled. Pedals may attach to matches that also have tab-switch
and keyword buttons. ExecutePedal is used instead of OpenMatch when
the intent is to perform the pedal action.

Bug: 1046523
Change-Id: I5a46383ca716df2f41f21eec3ffcf2969e7ab593
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2032189Reviewed-by: Tommy Li <tommycli@chromium.org>
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737451}

chrome/browser/ui/views/omnibox/omnibox_result_view.cc

@@ -361,9 +361,9 @@ void OmniboxResultView::ButtonPressed(views::Button* button,
     // TODO(orinj): Implement.
   } else if (button == pedal_button_) {
     DCHECK(match_.pedal);
-    // TODO(orinj): Open the match in a way that does not conflict with search
-    // or tab switch. Various dispositions are now possible from a single match.
-    OpenMatch(WindowOpenDisposition::SWITCH_TO_TAB, event.time_stamp());
+    // Pedal action intent means we execute the match instead of opening it.
+    popup_contents_view_->model()->edit_model()->ExecutePedal(
+        match_, event.time_stamp());
   } else {
     NOTREACHED();
   }

components/omnibox/browser/autocomplete_result.cc

@@ -386,11 +386,19 @@ void AutocompleteResult::ConvertInSuggestionPedalMatches(
   // Used to ensure we keep only one Pedal of each kind.
   std::unordered_set<OmniboxPedal*> pedals_found;
   for (auto& match : matches_) {
+    if (OmniboxFieldTrial::IsSuggestionButtonRowEnabled()) {
+      // Skip only matches that have already detected their pedal. With the
+      // button row enabled, a pedal may attach along with tab switch and
+      // associated keyword buttons.
+      if (match.pedal)
+        continue;
+    } else {
       // Skip matches that will not show Pedal because they already
       // have a tab match or associated keyword.  Also skip matches
       // that have already detected their Pedal.
       if (match.has_tab_match || match.associated_keyword || match.pedal)
         continue;
+    }
 
     OmniboxPedal* const pedal = provider->FindPedalMatch(match.contents);
     if (pedal) {

components/omnibox/browser/omnibox_edit_model.cc

@@ -719,6 +719,21 @@ void OmniboxEditModel::EnterKeywordModeForDefaultSearchProvider(
   EmitKeywordHistogram(entry_method);
 }
 
+void OmniboxEditModel::ExecutePedal(const AutocompleteMatch& match,
+                                    base::TimeTicks match_selection_timestamp) {
+  CHECK(match.pedal);
+  {
+    // This block resets omnibox to unedited state and closes popup, which
+    // may not seem necessary in cases of navigation but makes sense for
+    // taking Pedal actions in general.
+    base::AutoReset<bool> tmp(&in_revert_, true);
+    view_->RevertAll();
+  }
+  OmniboxPedal::ExecutionContext context(*client_, *controller_,
+                                         match_selection_timestamp);
+  match.pedal->Execute(context);
+}
+
 void OmniboxEditModel::OpenMatch(AutocompleteMatch match,
                                  WindowOpenDisposition disposition,
                                  const GURL& alternate_nav_url,
@@ -731,17 +746,11 @@ void OmniboxEditModel::OpenMatch(AutocompleteMatch match,
   autocomplete_controller()->UpdateMatchDestinationURLWithQueryFormulationTime(
       elapsed_time_since_user_first_modified_omnibox, &match);
 
-  if (match.pedal) {
-    {
-      // This block resets omnibox to unedited state and closes popup, which
-      // may not seem necessary in cases of navigation but makes sense for
-      // taking Pedal actions in general.
-      base::AutoReset<bool> tmp(&in_revert_, true);
-      view_->RevertAll();
-    }
-    OmniboxPedal::ExecutionContext context(*client_, *controller_,
-                                           match_selection_timestamp);
-    match.pedal->Execute(context);
+  // Matches with |pedal| may be opened normally or executed, but when a match
+  // is a dedicated Pedal suggestion, it should always be executed. This only
+  // happens when the button row feature is disabled.
+  if (match.pedal && !OmniboxFieldTrial::IsSuggestionButtonRowEnabled()) {
+    ExecutePedal(match, match_selection_timestamp);
     return;
   }
 

components/omnibox/browser/omnibox_edit_model.h

@@ -215,6 +215,10 @@ class OmniboxEditModel {
       WindowOpenDisposition disposition,
       base::TimeTicks match_selection_timestamp = base::TimeTicks());
 
+  // Executes the |pedal| associated with given match.
+  void ExecutePedal(const AutocompleteMatch& match,
+                    base::TimeTicks match_selection_timestamp);
+
   // Asks the browser to load |match|. |index| is only used for logging, and
   // can be kNoMatch if the popup was closed, or if none of the suggestions
   // in the popup were used (in the unusual no-default-match case). In that