Show PhysicalWebProvider suggestions with omnibox input

components/omnibox/browser/physical_web_provider.cc

 // Copyright (c) 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 "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
+#include "components/bookmarks/browser/titled_url_index.h"
+#include "components/bookmarks/browser/titled_url_node_sorter.h"
 #include "components/omnibox/browser/autocomplete_provider_client.h"
 #include "components/omnibox/browser/autocomplete_provider_listener.h"
+#include "components/omnibox/browser/autocomplete_provider_utils.h"
 #include "components/omnibox/browser/history_url_provider.h"
-#include "components/omnibox/browser/physical_web_provider.h"
+#include "components/omnibox/browser/physical_web_node.h"
+#include "components/omnibox/browser/url_prefix.h"
 #include "components/omnibox/browser/verbatim_match.h"
 #include "components/physical_web/data_source/physical_web_data_source.h"
 #include "components/url_formatter/url_formatter.h"
 #include "grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/text_elider.h"
 #include "url/gurl.h"
 
 namespace {
 
 // Relevance score of the first Physical Web URL autocomplete match. This score
 // is intended to be between ClipboardURLProvider and ZeroSuggestProvider.
 // Subsequent matches should decrease in relevance to preserve the ordering
 // in the metadata list.
 static const int kPhysicalWebUrlBaseRelevance = 700;
 
 // The maximum length of the page title's part of the overflow item's
 // description. Longer titles will be truncated to this length. In a normal
 // Physical Web match item (non-overflow item) we allow the omnibox display to
 // truncate the title instead.
 static const size_t kMaxTitleLengthInOverflow = 15;
+
+// The maximum number of Physical Web URLs to retrieve from the index.
+static const size_t kPhysicalWebIndexMaxMatches = 50;
 }

[Mark P, 2016/12/21 18:57:50]

style nit (I think): blank line before closing namespace brace

[mattreynolds, 2016/12/21 22:25:58]

Done.

 
 // static
 const size_t PhysicalWebProvider::kPhysicalWebMaxMatches = 1;
 
 // static
 PhysicalWebProvider* PhysicalWebProvider::Create(
     AutocompleteProviderClient* client,
     HistoryURLProvider* history_url_provider) {
   return new PhysicalWebProvider(client, history_url_provider);
 }
 
 void PhysicalWebProvider::Start(const AutocompleteInput& input,
                                 bool minimal_changes) {
   DCHECK(kPhysicalWebMaxMatches < kMaxMatches);
 
   Stop(false, false);
 
   done_ = false;
   matches_.clear();
 
-  // Stop providing suggestions when the user enters text into the omnibox.
-  if (!input.from_omnibox_focus()) {
-    done_ = true;
-    return;
-  }
-
   // Don't provide suggestions in incognito mode.
   if (client_->IsOffTheRecord()) {
     done_ = true;
     nearby_url_count_ = 0;
     return;
   }
 
   physical_web::PhysicalWebDataSource* data_source =
       client_->GetPhysicalWebDataSource();
   if (!data_source) {
     done_ = true;
     nearby_url_count_ = 0;
     return;
   }
 
-  ConstructMatches(data_source->GetMetadata().get());
+  if (input.from_omnibox_focus()) {
+    ConstructZeroSuggestMatches(data_source->GetMetadata());
 
-  // Physical Web matches should never be default. If the omnibox input is
-  // non-empty and we have at least one Physical Web match, add the current URL
-  // as the default so that hitting enter after focusing the omnibox causes the
-  // current page to reload. If the input field is empty, no default match is
-  // required.
-  if (!matches_.empty() && !input.text().empty()) {
-    matches_.push_back(VerbatimMatchForURL(client_, input, input.current_url(),
-                                           history_url_provider_, -1));
+    // In zero-suggest, Physical Web matches should never be default. If the
+    // omnibox input is non-empty and we have at least one Physical Web match,
+    // add the current URL as the default so that hitting enter after focusing
+    // the omnibox causes the current page to reload. If the input field is
+    // empty, no default match is required.
+    if (!matches_.empty() && !input.text().empty()) {
+      matches_.push_back(
+          VerbatimMatchForURL(client_, input, input.current_url(),
+                              history_url_provider_, -1));
+    }
+  } else {
+    ConstructQuerySuggestMatches(data_source->GetMetadata(), input);
   }
 
   done_ = true;
 }
 
 void PhysicalWebProvider::Stop(bool clear_cached_results,
                                bool due_to_user_inactivity) {
   done_ = true;
 }
 
@@ skipping 13 matching lines @@
 PhysicalWebProvider::PhysicalWebProvider(
     AutocompleteProviderClient* client,
     HistoryURLProvider* history_url_provider)
     : AutocompleteProvider(AutocompleteProvider::TYPE_PHYSICAL_WEB),
       client_(client),
       history_url_provider_(history_url_provider) {}
 
 PhysicalWebProvider::~PhysicalWebProvider() {
 }
 
-void PhysicalWebProvider::ConstructMatches(base::ListValue* metadata_list) {
+void PhysicalWebProvider::ConstructZeroSuggestMatches(
+    std::unique_ptr<base::ListValue> metadata_list) {
   nearby_url_count_ = metadata_list->GetSize();
   size_t used_slots = 0;
 
   for (size_t i = 0; i < nearby_url_count_; ++i) {
     base::DictionaryValue* metadata_item = NULL;
     if (!metadata_list->GetDictionary(i, &metadata_item)) {
       continue;
     }
 
     std::string url_string;
@@ skipping 41 matching lines @@
         ACMatchClassification(0, ACMatchClassification::NONE));
 
     matches_.push_back(match);
     ++used_slots;
   }
 
   UMA_HISTOGRAM_EXACT_LINEAR(
       "Omnibox.PhysicalWebProviderMatches", matches_.size(), kMaxMatches);
 }
 
+void PhysicalWebProvider::ConstructQuerySuggestMatches(
+    std::unique_ptr<base::ListValue> metadata_list,
+    const AutocompleteInput& input) {
+  bookmarks::TitledUrlIndex index(NULL);

[Mark P, 2016/12/21 18:57:50]

Please either add a comment (in this changelist or another) to the constructor
of TitledUrlIndex that NULL is valid.  And generally explain what the passed
parameter means.

If you don't want to do that there (I might not understand why), then do it
here.

Also, nit: NULL -> nullptr

[mattreynolds, 2016/12/21 22:25:58]

I'll add the comment here and comment the TitledUrlIndex constructor in a
separate CL.

[Mark P, 2016/12/21 23:21:40]

Acknowledged.

+  std::vector<std::unique_ptr<PhysicalWebNode> > nodes;
+  size_t metadata_count = metadata_list->GetSize();

[Mark P, 2016/12/21 18:57:50]

nit: const

[mattreynolds, 2016/12/21 22:25:58]

Done.

+  for (size_t i = 0; i < metadata_count; ++i) {
+    base::DictionaryValue* metadata_item = NULL;
+    if (metadata_list->GetDictionary(i, &metadata_item)) {
+      nodes.push_back(base::MakeUnique<PhysicalWebNode>(*metadata_item));

[Mark P, 2016/12/21 18:57:50]

This seems roundabout to me.  Why not call index.Add() directly?  Why do you
need the nodes vector?

[mattreynolds, 2016/12/21 22:25:58]

TitledUrlIndex doesn't take ownership of the PhysicalWebNodes we're creating. By
storing them in a vector of unique_ptrs we can ensure they are properly cleaned
up once the vector falls out of scope.

[Mark P, 2016/12/21 23:21:40]

Acknowledged.

+      index.Add(nodes.back().get());
+    }
+  }
+
+  std::vector<bookmarks::TitledUrlMatch> titled_url_matches;
+  index.GetResultsMatching(input.text(), kPhysicalWebIndexMaxMatches,
+                           query_parser::MatchingAlgorithm::DEFAULT,
+                           &titled_url_matches);
+
+  size_t used_slots = 0;
+  const base::string16 fixed_up_input(FixupUserInput(input).second);
+  for (auto titled_url_match : titled_url_matches) {
+    const int relevance = kPhysicalWebUrlBaseRelevance - used_slots;
+    matches_.push_back(
+        TitledUrlMatchToAutocompleteMatch(
+            this, client_->GetSchemeClassifier(), input, fixed_up_input,
+            titled_url_match, AutocompleteMatchType::PHYSICAL_WEB, relevance));
+    ++used_slots;
+    if (matches_.size() >= kPhysicalWebMaxMatches) {
+      break;
+    }
+  }
+}
+
 void PhysicalWebProvider::AppendOverflowItem(int additional_url_count,
                                              int relevance,
                                              const base::string16& title) {
   std::string url_string = "chrome://physical-web";
   GURL url(url_string);
 
   AutocompleteMatch match(this, relevance, false,
       AutocompleteMatchType::PHYSICAL_WEB_OVERFLOW);
   match.destination_url = url;
 
@@ skipping 14 matching lines @@
       AutocompleteInput::FormattedStringWithEquivalentMeaning(
           url, match.contents, client_->GetSchemeClassifier());
 
   match.description =
       l10n_util::GetStringUTF16(IDS_PHYSICAL_WEB_OVERFLOW_DESCRIPTION);
   match.description_class.push_back(
       ACMatchClassification(0, ACMatchClassification::NONE));
 
   matches_.push_back(match);
 }