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

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

Issue 2689803002: Ensure nearby URL count metric is properly initialized (Closed)
Patch Set: add new histogram to detect corner case Created 3 years, 10 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 15ba262ab3d8c5e8acbdcc15a9a85de3dc69ab47..6319cc19aff3fd91c1db85b835748936b2f4bd00 100644
--- a/components/omnibox/browser/physical_web_provider.cc
+++ b/components/omnibox/browser/physical_web_provider.cc
@@ -35,6 +35,9 @@ static const size_t kMaxTitleLengthInOverflow = 15;
// The maximum number of Physical Web URLs to retrieve from the index.
static const size_t kPhysicalWebIndexMaxMatches = 50;
+
+// A value used to represent an invalid number of nearby URLs.
+static const size_t kInvalidUrlCount = 100;
}
// static
@@ -56,14 +59,19 @@ void PhysicalWebProvider::Start(const AutocompleteInput& input,
done_ = false;
matches_.clear();
+ const bool input_from_focus = input.from_omnibox_focus();
+
had_physical_web_suggestions_ = false;
- if (input.from_omnibox_focus())
- had_physical_web_suggestions_at_focus_or_later_ = false;
+ if (input_from_focus) {
+ BeginOmniboxSession();
+ }
// Don't provide suggestions in incognito mode.
if (client_->IsOffTheRecord()) {
done_ = true;
nearby_url_count_ = 0;
+ nearby_url_count_at_focus_ = 0;
+ nearby_url_count_at_focus_valid_ = true;
return;
}
@@ -72,14 +80,22 @@ void PhysicalWebProvider::Start(const AutocompleteInput& input,
if (!data_source) {
done_ = true;
nearby_url_count_ = 0;
+ nearby_url_count_at_focus_ = 0;
+ nearby_url_count_at_focus_valid_ = true;
return;
}
- const bool input_from_focus = input.from_omnibox_focus();
+ auto metadata_list = data_source->GetMetadataList();
+ nearby_url_count_ = metadata_list->size();
+ if (input_from_focus) {
+ nearby_url_count_at_focus_ = nearby_url_count_;
+ nearby_url_count_at_focus_valid_ = true;
+ }
+
const bool empty_input_from_user = !input_from_focus && input.text().empty();
if (input_from_focus || empty_input_from_user) {
- ConstructZeroSuggestMatches(data_source->GetMetadataList());
+ ConstructZeroSuggestMatches(std::move(metadata_list));
if (!matches_.empty()) {
had_physical_web_suggestions_ = true;
@@ -105,7 +121,7 @@ void PhysicalWebProvider::Start(const AutocompleteInput& input,
client_, input, input.current_url(), history_url_provider_, -1));
}
} else {
- ConstructQuerySuggestMatches(data_source->GetMetadataList(), input);
+ ConstructQuerySuggestMatches(std::move(metadata_list), input);
if (!matches_.empty()) {
had_physical_web_suggestions_ = true;
@@ -148,8 +164,28 @@ void PhysicalWebProvider::AddProviderInfo(ProvidersInfo* provider_info) const {
// When the user accepts an autocomplete suggestion, record the number of
// nearby Physical Web URLs at the time the provider last constructed matches.
- UMA_HISTOGRAM_EXACT_LINEAR("Omnibox.SuggestionUsed.NearbyURLCount",
- nearby_url_count_, 50);
+ UMA_HISTOGRAM_EXACT_LINEAR(
+ "Omnibox.SuggestionUsed.NearbyURLCount.AtMatchCreation",
+ nearby_url_count_, 50);
+
+ // On Android, it is somehow possible to accept an omnibox suggestion without
+ // first focusing the omnibox. In this situation, the nearby URL count at
+ // focus will be invalid because the omnibox was never focused. In an earlier
+ // version, this caused invalid data to be recorded by the histogram. Now we
+ // guard against recording the invalid data and instead record that we hit
+ // this corner case.
+ // TODO(crbug.com/691059): Remove once the focus-less suggestion mystery is
+ // solved.
+ UMA_HISTOGRAM_BOOLEAN(
+ "Omnibox.PhysicalWebProvider.SuggestionUsedWithoutOmniboxFocus",
+ !nearby_url_count_at_focus_valid_);
+
+ if (nearby_url_count_at_focus_valid_) {
+ // When the user accepts an autocomplete suggestion, record the number of
+ // nearby Physical Web URLs at the time the omnibox input was last focused.
+ UMA_HISTOGRAM_EXACT_LINEAR("Omnibox.SuggestionUsed.NearbyURLCount.AtFocus",
+ nearby_url_count_at_focus_, 50);
+ }
}
PhysicalWebProvider::PhysicalWebProvider(
@@ -158,6 +194,9 @@ PhysicalWebProvider::PhysicalWebProvider(
: AutocompleteProvider(AutocompleteProvider::TYPE_PHYSICAL_WEB),
client_(client),
history_url_provider_(history_url_provider),
+ nearby_url_count_(kInvalidUrlCount),
+ nearby_url_count_at_focus_(kInvalidUrlCount),
+ nearby_url_count_at_focus_valid_(false),
zero_suggest_enabled_(
OmniboxFieldTrial::InPhysicalWebZeroSuggestFieldTrial()),
after_typing_enabled_(
@@ -165,17 +204,27 @@ PhysicalWebProvider::PhysicalWebProvider(
zero_suggest_base_relevance_(
OmniboxFieldTrial::GetPhysicalWebZeroSuggestBaseRelevance()),
after_typing_base_relevance_(
- OmniboxFieldTrial::GetPhysicalWebAfterTypingBaseRelevance()) {}
+ OmniboxFieldTrial::GetPhysicalWebAfterTypingBaseRelevance()),
+ had_physical_web_suggestions_(false),
+ had_physical_web_suggestions_at_focus_or_later_(false) {}
PhysicalWebProvider::~PhysicalWebProvider() {
}
+void PhysicalWebProvider::BeginOmniboxSession() {
+ // Some metrics are tracked per-session and must be reset each time a new
+ // session begins. Per-session metrics are recorded in AddProviderInfo.
+ nearby_url_count_at_focus_ = kInvalidUrlCount;
+ nearby_url_count_at_focus_valid_ = false;
+ had_physical_web_suggestions_at_focus_or_later_ = false;
+}
+
void PhysicalWebProvider::ConstructZeroSuggestMatches(
std::unique_ptr<physical_web::MetadataList> metadata_list) {
- nearby_url_count_ = metadata_list->size();
+ size_t nearby_url_count = metadata_list->size();
size_t used_slots = 0;
- for (size_t i = 0; i < nearby_url_count_; ++i) {
+ for (size_t i = 0; i < nearby_url_count; ++i) {
const auto& metadata_item = (*metadata_list)[i];
std::string url_string = metadata_item.resolved_url.spec();
std::string title_string = metadata_item.title;
@@ -189,7 +238,7 @@ void PhysicalWebProvider::ConstructZeroSuggestMatches(
// Append an overflow item if creating a match for each metadata item would
// exceed the match limit.
const size_t remaining_slots = kPhysicalWebMaxMatches - used_slots;
- const size_t remaining_metadata = nearby_url_count_ - i;
+ const size_t remaining_metadata = nearby_url_count - i;
if ((remaining_slots == 1) && (remaining_metadata > remaining_slots)) {
AppendOverflowItem(remaining_metadata, relevance, title);
break;

Powered by Google App Engine
This is Rietveld 408576698