Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(628)

Unified Diff: components/omnibox/browser/physical_web_provider.cc

Issue 2591053002: Show PhysicalWebProvider suggestions with omnibox input (Closed)
Patch Set: enable experiment in unit tests Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/omnibox/browser/physical_web_provider.cc
diff --git a/components/omnibox/browser/physical_web_provider.cc b/components/omnibox/browser/physical_web_provider.cc
index fc65e2e4464f0b0669f89351ffc27dfa25b90593..0f8ac1efe5860d70395ec6a542d1e190eeb1ecb1 100644
--- a/components/omnibox/browser/physical_web_provider.cc
+++ b/components/omnibox/browser/physical_web_provider.cc
@@ -2,14 +2,22 @@
// 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/history_url_provider.h"
-#include "components/omnibox/browser/physical_web_provider.h"
+#include "components/omnibox/browser/omnibox_field_trial.h"
+#include "components/omnibox/browser/physical_web_node.h"
+#include "components/omnibox/browser/titled_url_match_utils.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"
@@ -20,17 +28,14 @@
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;
}
// static
@@ -52,11 +57,9 @@ void PhysicalWebProvider::Start(const AutocompleteInput& input,
done_ = false;
matches_.clear();
- // Stop providing suggestions when the user enters text into the omnibox.
- if (!input.from_omnibox_focus()) {
- done_ = true;
- return;
- }
+ had_physical_web_suggestions_ = false;
+ if (input.from_omnibox_focus())
+ had_physical_web_suggestions_at_focus_or_later_ = false;
// Don't provide suggestions in incognito mode.
if (client_->IsOffTheRecord()) {
@@ -73,16 +76,38 @@ void PhysicalWebProvider::Start(const AutocompleteInput& input,
return;
}
- ConstructMatches(data_source->GetMetadata().get());
+ if (input.from_omnibox_focus()) {
+ ConstructZeroSuggestMatches(data_source->GetMetadata());
+
+ if (!matches_.empty()) {
+ had_physical_web_suggestions_ = true;
+ had_physical_web_suggestions_at_focus_or_later_ = true;
+ }
+
+ if (!zero_suggest_enabled_) {
+ matches_.clear();
+ }
+
+ // 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);
+
+ if (!matches_.empty()) {
+ had_physical_web_suggestions_ = true;
+ had_physical_web_suggestions_at_focus_or_later_ = true;
+ }
- // 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));
+ if (!after_typing_enabled_) {
+ matches_.clear();
+ }
}
done_ = true;
@@ -94,11 +119,23 @@ void PhysicalWebProvider::Stop(bool clear_cached_results,
}
void PhysicalWebProvider::AddProviderInfo(ProvidersInfo* provider_info) const {
- // AddProviderInfo is called for each autocomplete provider to allow
- // provider-specific diagnostic info to be added to the omnibox log entry.
- // In this case we do not append any diagnostic info and are taking advantage
- // of the fact that this method is only called when the user has accepted an
- // autocomplete suggestion.
+ // Record whether the provider could have provided a Physical Web suggestion,
+ // even if the suggestion could not be displayed due to the current field
+ // trial.
+ provider_info->push_back(metrics::OmniboxEventProto_ProviderInfo());
+ metrics::OmniboxEventProto_ProviderInfo& new_entry = provider_info->back();
+ new_entry.set_provider(AsOmniboxEventProviderType());
+ new_entry.set_provider_done(done_);
+ std::vector<uint32_t> field_trial_hashes;
+ OmniboxFieldTrial::GetActiveSuggestFieldTrialHashes(&field_trial_hashes);
+ for (size_t i = 0; i < field_trial_hashes.size(); ++i) {
+ if (had_physical_web_suggestions_)
+ new_entry.mutable_field_trial_triggered()->Add(field_trial_hashes[i]);
+ if (had_physical_web_suggestions_at_focus_or_later_) {
+ new_entry.mutable_field_trial_triggered_in_session()->Add(
+ field_trial_hashes[i]);
+ }
+ }
// When the user accepts an autocomplete suggestion, record the number of
// nearby Physical Web URLs at the time the provider last constructed matches.
@@ -111,12 +148,21 @@ PhysicalWebProvider::PhysicalWebProvider(
HistoryURLProvider* history_url_provider)
: AutocompleteProvider(AutocompleteProvider::TYPE_PHYSICAL_WEB),
client_(client),
- history_url_provider_(history_url_provider) {}
+ history_url_provider_(history_url_provider),
+ zero_suggest_enabled_(
+ OmniboxFieldTrial::InPhysicalWebZeroSuggestFieldTrial()),
+ after_typing_enabled_(
+ OmniboxFieldTrial::InPhysicalWebAfterTypingFieldTrial()),
+ zero_suggest_base_relevance_(
+ OmniboxFieldTrial::GetPhysicalWebZeroSuggestBaseRelevance()),
+ after_typing_base_relevance_(
+ OmniboxFieldTrial::GetPhysicalWebAfterTypingBaseRelevance()) {}
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;
@@ -137,7 +183,7 @@ void PhysicalWebProvider::ConstructMatches(base::ListValue* metadata_list) {
// Add match items with decreasing relevance to preserve the ordering in
// the metadata list.
- int relevance = kPhysicalWebUrlBaseRelevance - used_slots;
+ int relevance = zero_suggest_base_relevance_ - used_slots;
// Append an overflow item if creating a match for each metadata item would
// exceed the match limit.
@@ -178,6 +224,41 @@ void PhysicalWebProvider::ConstructMatches(base::ListValue* metadata_list) {
"Omnibox.PhysicalWebProviderMatches", matches_.size(), kMaxMatches);
}
+void PhysicalWebProvider::ConstructQuerySuggestMatches(
+ std::unique_ptr<base::ListValue> metadata_list,
+ const AutocompleteInput& input) {
+ // Passing nullptr for the TitledUrlNodeSorter will cause the returned match
+ // list to be unsorted.
+ bookmarks::TitledUrlIndex index(nullptr);
+ std::vector<std::unique_ptr<PhysicalWebNode>> nodes;
+ const size_t metadata_count = metadata_list->GetSize();
+ 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));
+ 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 = after_typing_base_relevance_ - used_slots;
+ matches_.push_back(bookmarks::TitledUrlMatchToAutocompleteMatch(
+ titled_url_match, AutocompleteMatchType::PHYSICAL_WEB, relevance, this,
+ client_->GetSchemeClassifier(), input, fixed_up_input));
+ ++used_slots;
+ if (matches_.size() >= kPhysicalWebMaxMatches) {
+ break;
+ }
+ }
+}
+
void PhysicalWebProvider::AppendOverflowItem(int additional_url_count,
int relevance,
const base::string16& title) {
« no previous file with comments | « components/omnibox/browser/physical_web_provider.h ('k') | components/omnibox/browser/physical_web_provider_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698