[omnibox] Filter translate pedal when on chrome:// internal URLs

This CL prevents the Translate page pedal from appearing when the
current_url is any builtin chrome:// URL, including chrome://newtab .
This prevents bad message crashes when in guest mode (see bug).

Bug: 1131136
Change-Id: Id0c05a66d3bd6509c0f5f3b242555607fc6dcec1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2432209
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Reviewed-by: manuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810913}

chrome/browser/ui/omnibox/omnibox_pedals_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "base/environment.h"
 #include "base/strings/utf_string_conversions.h"
+#include "components/omnibox/browser/autocomplete_input.h"
 #include "components/omnibox/browser/mock_autocomplete_provider_client.h"
 #include "components/omnibox/browser/omnibox_pedal_provider.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -548,6 +549,8 @@ TEST(OmniboxPedals, DataLoadsForAllLocales) {
       },
       // clang-format on
   };
+  AutocompleteInput input;
+  input.set_current_url(GURL("https://example.com"));
   for (const TestCase& test_case : test_cases) {
     // Prepare the shared ResourceBundle with data for tested locale.
     env->SetVar("LANG", test_case.locale);
@@ -557,22 +560,25 @@ TEST(OmniboxPedals, DataLoadsForAllLocales) {
     // Instantiating the provider loads concept data from shared ResourceBundle.
     OmniboxPedalProvider provider(client);
 
-    EXPECT_EQ(provider.FindPedalMatch(base::UTF8ToUTF16("")), nullptr);
+    EXPECT_EQ(provider.FindPedalMatch(input, base::UTF8ToUTF16("")), nullptr);
 #if defined(OS_CHROMEOS)
     // TODO(orinj): Get ChromeOS to use the right dataset, but for now make this
     //  a soft failure so as to not block all other platforms. To ensure this
     //  is not going to cause failure in production, still test that English
     //  triggering functions. Data is there; it works; but warn about locale.
-    if (!provider.FindPedalMatch(base::UTF8ToUTF16(test_case.triggers[0]))) {
-      EXPECT_NE(provider.FindPedalMatch(base::UTF8ToUTF16("clear history")),
-                nullptr);
+    if (!provider.FindPedalMatch(input,
+                                 base::UTF8ToUTF16(test_case.triggers[0]))) {
+      EXPECT_NE(
+          provider.FindPedalMatch(input, base::UTF8ToUTF16("clear history")),
+          nullptr);
       LOG(WARNING) << "ChromeOS using English for locale " << test_case.locale;
       continue;
     }
 #endif
 
     for (const std::string& trigger : test_case.triggers) {
-      EXPECT_NE(provider.FindPedalMatch(base::UTF8ToUTF16(trigger)), nullptr)
+      EXPECT_NE(provider.FindPedalMatch(input, base::UTF8ToUTF16(trigger)),
+                nullptr)
           << "locale: " << test_case.locale << std::endl
           << "trigger: " << trigger;
     }

components/omnibox/browser/autocomplete_controller.cc

@@ -690,7 +690,7 @@ void AutocompleteController::UpdateResult(
   result_.SortAndCull(input_, template_url_service_, preserve_default_match);
 
   if (OmniboxFieldTrial::IsPedalSuggestionsEnabled()) {
-    result_.ConvertInSuggestionPedalMatches(provider_client_.get());
+    result_.AttachPedalsToMatches(input_, *provider_client_);
   }
 
   // Need to validate before invoking CopyOldMatches as the old matches are not

components/omnibox/browser/autocomplete_result.cc

@@ -445,9 +445,10 @@ void AutocompleteResult::DemoteOnDeviceSearchSuggestions() {
   }
 }
 
-void AutocompleteResult::ConvertInSuggestionPedalMatches(
-    AutocompleteProviderClient* client) {
-  OmniboxPedalProvider* provider = client->GetPedalProvider();
+void AutocompleteResult::AttachPedalsToMatches(
+    const AutocompleteInput& input,
+    const AutocompleteProviderClient& client) {
+  OmniboxPedalProvider* provider = client.GetPedalProvider();
   // Used to ensure we keep only one Pedal of each kind.
   std::unordered_set<OmniboxPedal*> pedals_found;
 
@@ -460,7 +461,7 @@ void AutocompleteResult::ConvertInSuggestionPedalMatches(
       continue;
     }
 
-    OmniboxPedal* const pedal = provider->FindPedalMatch(match.contents);
+    OmniboxPedal* const pedal = provider->FindPedalMatch(input, match.contents);
     if (pedal) {
       const auto result = pedals_found.insert(pedal);
       if (result.second)

components/omnibox/browser/autocomplete_result.h

@@ -92,7 +92,8 @@ class AutocompleteResult {
   void GroupAndDemoteMatchesWithHeaders();
 
   // Sets |pedal| in matches that have Pedal-triggering text.
-  void ConvertInSuggestionPedalMatches(AutocompleteProviderClient* client);
+  void AttachPedalsToMatches(const AutocompleteInput& input,
+                             const AutocompleteProviderClient& client);
 
   // Sets |has_tab_match| in matches whose URL matches an open tab's URL.
   // Also, fixes up the description if not using another UI element to

components/omnibox/browser/autocomplete_result_unittest.cc

@@ -1715,6 +1715,9 @@ TEST_F(AutocompleteResultTest, AttachesPedals) {
   EXPECT_NE(nullptr, client.GetPedalProvider());
 
   AutocompleteResult result;
+  AutocompleteInput input(base::ASCIIToUTF16("a"),
+                          metrics::OmniboxEventProto::OTHER,
+                          TestSchemeClassifier());
 
   // Populate result with test matches.
   {
@@ -1750,15 +1753,11 @@ TEST_F(AutocompleteResultTest, AttachesPedals) {
     for (size_t i = 0; i < base::size(data); i++) {
       matches[i].contents = base::UTF8ToUTF16(data[i].contents);
     }
-
-    AutocompleteInput input(base::ASCIIToUTF16("a"),
-                            metrics::OmniboxEventProto::OTHER,
-                            TestSchemeClassifier());
     result.AppendMatches(input, matches);
   }
 
   // Attach |pedal| to result matches where appropriate.
-  result.ConvertInSuggestionPedalMatches(&client);
+  result.AttachPedalsToMatches(input, client);
 
   // Ensure the entity suggestion doesn't get a pedal even though its contents
   // form a concept match.

components/omnibox/browser/omnibox_pedal.cc

@@ -121,6 +121,7 @@ void OmniboxPedal::Execute(OmniboxPedal::ExecutionContext& context) const {
 }
 
 bool OmniboxPedal::IsReadyToTrigger(
+    const AutocompleteInput& input,
     const AutocompleteProviderClient& client) const {
   return true;
 }

components/omnibox/browser/omnibox_pedal.h

@@ -22,6 +22,7 @@ struct VectorIcon;
 }
 #endif
 
+class AutocompleteInput;
 class AutocompleteProviderClient;
 class OmniboxEditController;
 class OmniboxClient;
@@ -139,7 +140,8 @@ class OmniboxPedal {
   // Returns true if this Pedal is ready to be used now, or false if
   // it does not apply under current conditions. (Example: the UpdateChrome
   // Pedal may not be ready to trigger if no update is available.)
-  virtual bool IsReadyToTrigger(const AutocompleteProviderClient& client) const;
+  virtual bool IsReadyToTrigger(const AutocompleteInput& input,
+                                const AutocompleteProviderClient& client) const;
 
 #if (!defined(OS_ANDROID) || BUILDFLAG(ENABLE_VR)) && !defined(OS_IOS)
   // Returns the vector icon to represent this Pedal's action in suggestion.

components/omnibox/browser/omnibox_pedal_implementations.cc

@@ -6,6 +6,7 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
+#include "components/omnibox/browser/autocomplete_input.h"
 #include "components/omnibox/browser/autocomplete_provider_client.h"
 #include "components/omnibox/browser/buildflags.h"
 #include "components/omnibox/browser/omnibox_client.h"
@@ -101,6 +102,17 @@ class OmniboxPedalTranslate : public OmniboxPedal {
   void Execute(ExecutionContext& context) const override {
     context.client_.PromptPageTranslation();
   }
+  bool IsReadyToTrigger(
+      const AutocompleteInput& input,
+      const AutocompleteProviderClient& client) const override {
+    // Built-in chrome:// URLs do not generally support translation, and the
+    // translate UI does not yet inform users with a clear helpful error message
+    // when requesting translation for a page that doesn't support translation,
+    // so this is a quick early-out to prevent bad message crashes.
+    // See: https://crbug.com/1131136
+    return !input.current_url().SchemeIs(
+        client.GetEmbedderRepresentationOfAboutScheme());
+  }
 };
 
 // =============================================================================

components/omnibox/browser/omnibox_pedal_implementations_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/task_environment.h"
 #include "base/time/time.h"
+#include "components/omnibox/browser/autocomplete_input.h"
 #include "components/omnibox/browser/mock_autocomplete_provider_client.h"
 #include "components/omnibox/browser/omnibox_pedal_provider.h"
 #include "components/omnibox/browser/test_omnibox_client.h"
@@ -26,9 +27,10 @@ class OmniboxPedalImplementationsTest : public testing::Test {
 
 TEST_F(OmniboxPedalImplementationsTest, PedalClearBrowsingDataExecutes) {
   MockAutocompleteProviderClient client;
+  AutocompleteInput input;
   OmniboxPedalProvider provider(client);
   const OmniboxPedal* pedal =
-      provider.FindPedalMatch(base::ASCIIToUTF16("clear browser data"));
+      provider.FindPedalMatch(input, base::ASCIIToUTF16("clear browser data"));
   base::TimeTicks match_selection_timestamp;
   OmniboxPedal::ExecutionContext context(
       *omnibox_client_, *omnibox_edit_controller_, match_selection_timestamp);

components/omnibox/browser/omnibox_pedal_provider.cc

@@ -11,6 +11,7 @@
 #include "base/strings/string_tokenizer.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "components/omnibox/browser/autocomplete_input.h"
 #include "components/omnibox/browser/autocomplete_provider_client.h"
 #include "components/omnibox/browser/omnibox_field_trial.h"
 #include "components/omnibox/browser/omnibox_pedal.h"
@@ -37,6 +38,12 @@ OmniboxPedalProvider::~OmniboxPedalProvider() {}
 void OmniboxPedalProvider::AddProviderInfo(ProvidersInfo* provider_info) const {
   provider_info->push_back(metrics::OmniboxEventProto_ProviderInfo());
   metrics::OmniboxEventProto_ProviderInfo& new_entry = provider_info->back();
+  // Note: SEARCH is used here because the suggestions that Pedals attach to are
+  // almost exclusively coming from search suggestions (they could in theory
+  // attach to others if the match content were a concept match, but in practice
+  // only search suggestions have the relevant text). PEDAL is not used because
+  // Pedals are not themselves suggestions produced by an autocomplete provider.
+  // This may change. See http://cl/327103601 for context and discussion.
   new_entry.set_provider(metrics::OmniboxEventProto::SEARCH);
   new_entry.set_provider_done(true);
 
@@ -58,6 +65,7 @@ void OmniboxPedalProvider::ResetSession() {
 }
 
 OmniboxPedal* OmniboxPedalProvider::FindPedalMatch(
+    const AutocompleteInput& input,
     const base::string16& match_text) {
   OmniboxPedal::Tokens match_tokens = Tokenize(match_text);
   if (match_tokens.empty()) {
@@ -72,7 +80,7 @@ OmniboxPedal* OmniboxPedalProvider::FindPedalMatch(
 
   for (const auto& pedal : pedals_) {
     if (pedal.second->IsTriggerMatch(match_tokens) &&
-        pedal.second->IsReadyToTrigger(client_)) {
+        pedal.second->IsReadyToTrigger(input, client_)) {
       field_trial_triggered_ = true;
       field_trial_triggered_in_session_ = true;
 

components/omnibox/browser/omnibox_pedal_provider.h

@@ -16,8 +16,12 @@
 #include "components/omnibox/browser/omnibox_pedal_implementations.h"
 
 class OmniboxPedal;
+class AutocompleteInput;
 class AutocompleteProviderClient;
 
+// Note: This is not an autocomplete provider; it doesn't produce suggestions
+// but rather "annotates" suggestions by attaching pedals to matches from other
+// providers (search in particular).
 class OmniboxPedalProvider {
  public:
   explicit OmniboxPedalProvider(AutocompleteProviderClient& client);
@@ -26,8 +30,9 @@ class OmniboxPedalProvider {
   OmniboxPedalProvider& operator=(const OmniboxPedalProvider&) = delete;
 
   // Returns the Pedal triggered by given |match_text| or nullptr if none
-  // trigger.
-  OmniboxPedal* FindPedalMatch(const base::string16& match_text);
+  // trigger. The |input| is used to determine suitability for current context.
+  OmniboxPedal* FindPedalMatch(const AutocompleteInput& input,
+                               const base::string16& match_text);
 
   // "Fake" implementation of AutocompleteProvider AddProviderInfo, though this
   // class is not a true subclass of AutocompleteProvider. This is used

components/omnibox/browser/omnibox_pedal_provider_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "base/environment.h"
 #include "base/strings/utf_string_conversions.h"
+#include "components/omnibox/browser/autocomplete_input.h"
 #include "components/omnibox/browser/mock_autocomplete_provider_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/resource/resource_bundle.h"
@@ -17,10 +18,11 @@ class OmniboxPedalProviderTest : public testing::Test {
 
 TEST_F(OmniboxPedalProviderTest, QueriesTriggerPedals) {
   MockAutocompleteProviderClient client;
+  AutocompleteInput input;
   OmniboxPedalProvider provider(client);
-  EXPECT_EQ(provider.FindPedalMatch(base::ASCIIToUTF16("")), nullptr);
-  EXPECT_EQ(provider.FindPedalMatch(base::ASCIIToUTF16("clear histor")),
+  EXPECT_EQ(provider.FindPedalMatch(input, base::ASCIIToUTF16("")), nullptr);
+  EXPECT_EQ(provider.FindPedalMatch(input, base::ASCIIToUTF16("clear histor")),
             nullptr);
-  EXPECT_NE(provider.FindPedalMatch(base::ASCIIToUTF16("clear history")),
+  EXPECT_NE(provider.FindPedalMatch(input, base::ASCIIToUTF16("clear history")),
             nullptr);
 }