Avoid DCHECK failure for chrome:// URLs in autocomplete suggestions

components/omnibox/browser/physical_web_provider_unittest.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/omnibox/browser/physical_web_provider.h"
 
 #include <memory>
 #include <string>
 
 #include "base/memory/ptr_util.h"
@@ skipping 73 matching lines @@
   TestSchemeClassifier scheme_classifier_;
 };
 
 class PhysicalWebProviderTest : public testing::Test {
  protected:
   PhysicalWebProviderTest() : provider_(NULL) {}
   ~PhysicalWebProviderTest() override {}
 
   void SetUp() override {
     client_.reset(new FakeAutocompleteProviderClient());
-    provider_ = PhysicalWebProvider::Create(client_.get());
+    provider_ = PhysicalWebProvider::Create(client_.get(), NULL);
   }
 
   void TearDown() override {
     provider_ = NULL;
   }
 
   // Create a dummy metadata list with |metadata_count| items. Each item is
   // populated with a unique scanned URL and page metadata.
   static std::unique_ptr<base::ListValue> CreateMetadata(
       size_t metadata_count) {
     auto metadata_list = base::MakeUnique<base::ListValue>();
     for (size_t i = 0; i < metadata_count; ++i) {
       std::string item_id = base::SizeTToString(i);
       std::string url = "https://example.com/" + item_id;
       auto metadata_item = base::MakeUnique<base::DictionaryValue>();
       metadata_item->SetString("scannedUrl", url);
       metadata_item->SetString("resolvedUrl", url);
       metadata_item->SetString("icon", url);
       metadata_item->SetString("title", "Example title " + item_id);
       metadata_item->SetString("description", "Example description " + item_id);
       metadata_list->Append(std::move(metadata_item));
     }
     return metadata_list;
   }
 
+  // Construct an AutocompleteInput to represent tapping the omnibox from the
+  // new tab page.
+  static AutocompleteInput CreateInputForNTP() {
+    return AutocompleteInput(base::string16(), base::string16::npos,
+        std::string(), GURL(), metrics::OmniboxEventProto::OTHER,

[Mark P, 2016/08/31 22:39:33]

better: INSTANT_NTP_WITH_OMNIBOX_AS_STARTING_FOCUS

[mattreynolds, 2016/09/01 01:24:11]

Done.

+        true, false, true, true, true, TestSchemeClassifier());

[Mark P, 2016/08/31 22:39:33]

nit: change the first "true" to "false"; that's more typical
ditto below

[mattreynolds, 2016/09/01 01:24:11]

Done.

+  }
+
+  // Construct an AutocompleteInput to represent tapping the omnibox with |url|
+  // as the current web page.
+  static AutocompleteInput CreateInputWithCurrentUrl(const std::string& url) {
+    return AutocompleteInput(base::ASCIIToUTF16(url), base::string16::npos,
+        std::string(), GURL(url), metrics::OmniboxEventProto::OTHER,
+        true, false, true, true, true, TestSchemeClassifier());
+  }
+
   std::unique_ptr<FakeAutocompleteProviderClient> client_;
   scoped_refptr<PhysicalWebProvider> provider_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PhysicalWebProviderTest);
 };
 
 TEST_F(PhysicalWebProviderTest, TestEmptyMetadataListCreatesNoMatches) {
   MockPhysicalWebDataSource* data_source =
       client_->GetMockPhysicalWebDataSource();
   EXPECT_TRUE(data_source);
 
   data_source->SetMetadata(CreateMetadata(0));
 
-  // Construct an AutocompleteInput representing a typical "zero-suggest"
-  // case. The provider will verify that the content of the omnibox input field
-  // was not entered by the user.
-  std::string url("http://www.cnn.com/");
-  const AutocompleteInput input(base::ASCIIToUTF16(url), base::string16::npos,
-      std::string(), GURL(url), metrics::OmniboxEventProto::OTHER,
-      true, false, true, true, true, TestSchemeClassifier());
-  provider_->Start(input, false);
+  // Run the test with no text in the omnibox input to simulate NTP.
+  provider_->Start(CreateInputForNTP(), false);
+  EXPECT_TRUE(provider_->matches().empty());
 
+  // Run the test again with a URL in the omnibox input.
+  provider_->Start(CreateInputWithCurrentUrl("http://www.cnn.com"), false);
   EXPECT_TRUE(provider_->matches().empty());
 }
 
 TEST_F(PhysicalWebProviderTest, TestSingleMetadataItemCreatesOneMatch) {
   MockPhysicalWebDataSource* data_source =
       client_->GetMockPhysicalWebDataSource();
   EXPECT_TRUE(data_source);
 
   // Extract the URL and title before inserting the metadata into the data
   // source.
   std::unique_ptr<base::ListValue> metadata_list = CreateMetadata(1);
   base::DictionaryValue* metadata_item;
   EXPECT_TRUE(metadata_list->GetDictionary(0, &metadata_item));
   std::string resolved_url;
   EXPECT_TRUE(metadata_item->GetString("resolvedUrl", &resolved_url));
   std::string title;
   EXPECT_TRUE(metadata_item->GetString("title", &title));
 
   data_source->SetMetadata(std::move(metadata_list));
 
-  // Construct an AutocompleteInput representing a typical "zero-suggest"
-  // case. The provider will verify that the content of the omnibox input field
-  // was not entered by the user.
-  std::string url("http://www.cnn.com/");
-  const AutocompleteInput input(base::ASCIIToUTF16(url), base::string16::npos,
-      std::string(), GURL(url), metrics::OmniboxEventProto::OTHER,
-      true, false, true, true, true, TestSchemeClassifier());
-  provider_->Start(input, false);
+  // Run the test with no text in the omnibox input to simulate NTP.
+  provider_->Start(CreateInputForNTP(), false);
 
   // Check that there is only one match item and its fields are correct.
   EXPECT_EQ(1U, provider_->matches().size());
   const AutocompleteMatch& metadata_match = provider_->matches().front();
   EXPECT_EQ(AutocompleteMatchType::PHYSICAL_WEB, metadata_match.type);
   EXPECT_EQ(resolved_url, metadata_match.destination_url.spec());
   EXPECT_EQ(resolved_url, base::UTF16ToASCII(metadata_match.contents));
   EXPECT_EQ(title, base::UTF16ToASCII(metadata_match.description));
+  EXPECT_FALSE(metadata_match.allowed_to_be_default_match);
+
+  // Run the test again with a URL in the omnibox input. An additional match
+  // should be added as a default match.
+  provider_->Start(CreateInputWithCurrentUrl("http://www.cnn.com"), false);
+
+  size_t metadata_match_count = 0;
+  size_t default_match_count = 0;
+  for (size_t i = 0; i < provider_->matches().size(); i++) {
+    const AutocompleteMatch& match = provider_->matches()[i];
+    if (match.type == AutocompleteMatchType::PHYSICAL_WEB) {
+      EXPECT_EQ(AutocompleteMatchType::PHYSICAL_WEB, match.type);

[Mark P, 2016/08/31 22:39:33]

would it be easy (and correct) to make a copy of metadata_match around line 186
and simply check that this match is the same as that one?  (Hopefully
AutocompleteMatch equality is defined.)

[mattreynolds, 2016/09/01 01:24:11]

Unfortunately equality is not implemented for AutocompleteMatch. I think it
would be nice to factor out the comparisons for an individual match so it's
clearer that we're checking the same things in both cases. This test will need
to be updated when we change the strings for the match item, so I'll do the
refactoring then.

+      EXPECT_EQ(resolved_url, match.destination_url.spec());
+      EXPECT_EQ(resolved_url, base::UTF16ToASCII(match.contents));
+      EXPECT_EQ(title, base::UTF16ToASCII(match.description));
+      EXPECT_FALSE(match.allowed_to_be_default_match);
+      ++metadata_match_count;
+    } else {
+      EXPECT_TRUE(match.allowed_to_be_default_match);
+      ++default_match_count;
+    }
+  }
+  EXPECT_EQ(2U, provider_->matches().size());
+  EXPECT_EQ(1U, metadata_match_count);
+  EXPECT_EQ(1U, default_match_count);
 }
 
 TEST_F(PhysicalWebProviderTest, TestManyMetadataItemsCreatesOverflowItem) {
   // This test is intended to verify that an overflow item is created when the
   // number of nearby Physical Web URLs exceeds the maximum allowable matches
   // for this provider. The actual limit for the PhysicalWebProvider may be
   // changed in the future, so create enough metadata to exceed the
   // AutocompleteProvider's limit.
   const size_t metadata_count = AutocompleteProvider::kMaxMatches + 1;
 
   MockPhysicalWebDataSource* data_source =
       client_->GetMockPhysicalWebDataSource();
   EXPECT_TRUE(data_source);
 
   data_source->SetMetadata(CreateMetadata(metadata_count));
 
-  // Construct an AutocompleteInput representing a typical "zero-suggest"
-  // case. The provider will verify that the content of the omnibox input field
-  // was not entered by the user.
-  std::string url("http://www.cnn.com/");
-  const AutocompleteInput input(base::ASCIIToUTF16(url), base::string16::npos,
-      std::string(), GURL(url), metrics::OmniboxEventProto::OTHER,
-      true, false, true, true, true, TestSchemeClassifier());
-  provider_->Start(input, false);
+  {
+    // Run the test with no text in the omnibox input to simulate NTP.
+    provider_->Start(CreateInputForNTP(), false);
 
-  const size_t match_count = provider_->matches().size();
-  EXPECT_LT(match_count, metadata_count);
+    const size_t match_count = provider_->matches().size();
+    EXPECT_LT(match_count, metadata_count);
 
-  // Check that the overflow item is at the end of the match list and its fields
-  // are correct.
-  const AutocompleteMatch& overflow_match = provider_->matches().back();
-  EXPECT_EQ(AutocompleteMatchType::PHYSICAL_WEB_OVERFLOW, overflow_match.type);
-  EXPECT_EQ("chrome://physical-web/", overflow_match.destination_url.spec());
-  EXPECT_EQ("chrome://physical-web/",
-      base::UTF16ToASCII(overflow_match.contents));
-  std::string description = l10n_util::GetPluralStringFUTF8(
-      IDS_PHYSICAL_WEB_OVERFLOW, metadata_count - match_count + 1);
-  EXPECT_EQ(description, base::UTF16ToASCII(overflow_match.description));
+    // Check that the overflow item is present and its fields are correct. There
+    // may be additional match items, but none should be marked as default.
+    size_t overflow_match_count = 0;
+    for (size_t i = 0; i < match_count; i++) {
+      const AutocompleteMatch& match = provider_->matches()[i];
+      if (match.type == AutocompleteMatchType::PHYSICAL_WEB_OVERFLOW) {
+        EXPECT_EQ(AutocompleteMatchType::PHYSICAL_WEB_OVERFLOW, match.type);

[Mark P, 2016/08/31 22:39:33]

Analogously, if AutocompletMatch equality is defined, perhaps you can declare an
overflow match object and check it for equality with the current match both here
and in 269.
If this turns out complicated, then skip it.  This is just a nicety in a test.

+        EXPECT_EQ("chrome://physical-web/", match.destination_url.spec());
+        EXPECT_EQ("chrome://physical-web/", base::UTF16ToASCII(match.contents));
+        const size_t metadata_matches = match_count - 1;
+        std::string description = l10n_util::GetPluralStringFUTF8(
+            IDS_PHYSICAL_WEB_OVERFLOW, metadata_count - metadata_matches);
+        EXPECT_EQ(description, base::UTF16ToASCII(match.description));
+        ++overflow_match_count;
+      }
+      EXPECT_FALSE(match.allowed_to_be_default_match);
+    }
+    EXPECT_EQ(1U, overflow_match_count);
+  }
+
+  {
+    // Run the test again with a URL in the omnibox input. An additional match
+    // should be added as a default match.
+    provider_->Start(CreateInputWithCurrentUrl("http://www.cnn.com"), false);
+
+    const size_t match_count = provider_->matches().size();
+    EXPECT_LT(match_count - 1, metadata_count);
+
+    // Check that the overflow item and default match are present and their
+    // fields are correct.
+    size_t overflow_match_count = 0;
+    size_t default_match_count = 0;
+    for (size_t i = 0; i < match_count; i++) {
+      const AutocompleteMatch& match = provider_->matches()[i];
+      if (match.type == AutocompleteMatchType::PHYSICAL_WEB_OVERFLOW) {
+        EXPECT_EQ(AutocompleteMatchType::PHYSICAL_WEB_OVERFLOW, match.type);
+        EXPECT_EQ("chrome://physical-web/", match.destination_url.spec());
+        EXPECT_EQ("chrome://physical-web/", base::UTF16ToASCII(match.contents));
+        const size_t metadata_matches = match_count - 2;
+        std::string description = l10n_util::GetPluralStringFUTF8(
+            IDS_PHYSICAL_WEB_OVERFLOW, metadata_count - metadata_matches);
+        EXPECT_EQ(description, base::UTF16ToASCII(match.description));
+        EXPECT_FALSE(match.allowed_to_be_default_match);
+        ++overflow_match_count;
+      } else if (match.allowed_to_be_default_match) {
+        ++default_match_count;
+      }
+    }
+    EXPECT_EQ(1U, overflow_match_count);
+    EXPECT_EQ(1U, default_match_count);
+  }
 }
 
 TEST_F(PhysicalWebProviderTest, TestNoMatchesWithUserInput) {
   MockPhysicalWebDataSource* data_source =
       client_->GetMockPhysicalWebDataSource();
   EXPECT_TRUE(data_source);
 
   data_source->SetMetadata(CreateMetadata(1));
 
   // Construct an AutocompleteInput to simulate user input in the omnibox input
   // field. The provider should not generate any matches.
   std::string text("user input");
   const AutocompleteInput input(base::ASCIIToUTF16(text), text.length(),
       std::string(), GURL(),
       metrics::OmniboxEventProto::INSTANT_NTP_WITH_OMNIBOX_AS_STARTING_FOCUS,
       true, false, true, true, false, TestSchemeClassifier());
   provider_->Start(input, false);
 
   EXPECT_TRUE(provider_->matches().empty());
 }
 
 }