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

Side by Side 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 unified diff | Download patch
OLDNEW
1 // Copyright (c) 2016 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2016 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/omnibox/browser/physical_web_provider.h" 5 #include "components/omnibox/browser/physical_web_provider.h"
6 6
7 #include "base/memory/ptr_util.h" 7 #include "base/memory/ptr_util.h"
8 #include "base/metrics/histogram_macros.h" 8 #include "base/metrics/histogram_macros.h"
9 #include "base/strings/string_util.h" 9 #include "base/strings/string_util.h"
10 #include "base/strings/utf_string_conversions.h" 10 #include "base/strings/utf_string_conversions.h"
(...skipping 17 matching lines...) Expand all
28 namespace { 28 namespace {
29 29
30 // The maximum length of the page title's part of the overflow item's 30 // The maximum length of the page title's part of the overflow item's
31 // description. Longer titles will be truncated to this length. In a normal 31 // description. Longer titles will be truncated to this length. In a normal
32 // Physical Web match item (non-overflow item) we allow the omnibox display to 32 // Physical Web match item (non-overflow item) we allow the omnibox display to
33 // truncate the title instead. 33 // truncate the title instead.
34 static const size_t kMaxTitleLengthInOverflow = 15; 34 static const size_t kMaxTitleLengthInOverflow = 15;
35 35
36 // The maximum number of Physical Web URLs to retrieve from the index. 36 // The maximum number of Physical Web URLs to retrieve from the index.
37 static const size_t kPhysicalWebIndexMaxMatches = 50; 37 static const size_t kPhysicalWebIndexMaxMatches = 50;
38
39 // A value used to represent an invalid number of nearby URLs.
40 static const size_t kInvalidUrlCount = 100;
38 } 41 }
39 42
40 // static 43 // static
41 const size_t PhysicalWebProvider::kPhysicalWebMaxMatches = 1; 44 const size_t PhysicalWebProvider::kPhysicalWebMaxMatches = 1;
42 45
43 // static 46 // static
44 PhysicalWebProvider* PhysicalWebProvider::Create( 47 PhysicalWebProvider* PhysicalWebProvider::Create(
45 AutocompleteProviderClient* client, 48 AutocompleteProviderClient* client,
46 HistoryURLProvider* history_url_provider) { 49 HistoryURLProvider* history_url_provider) {
47 return new PhysicalWebProvider(client, history_url_provider); 50 return new PhysicalWebProvider(client, history_url_provider);
48 } 51 }
49 52
50 void PhysicalWebProvider::Start(const AutocompleteInput& input, 53 void PhysicalWebProvider::Start(const AutocompleteInput& input,
51 bool minimal_changes) { 54 bool minimal_changes) {
52 DCHECK(kPhysicalWebMaxMatches < kMaxMatches); 55 DCHECK(kPhysicalWebMaxMatches < kMaxMatches);
53 56
54 Stop(false, false); 57 Stop(false, false);
55 58
56 done_ = false; 59 done_ = false;
57 matches_.clear(); 60 matches_.clear();
58 61
62 const bool input_from_focus = input.from_omnibox_focus();
63
59 had_physical_web_suggestions_ = false; 64 had_physical_web_suggestions_ = false;
60 if (input.from_omnibox_focus()) 65 if (input_from_focus) {
61 had_physical_web_suggestions_at_focus_or_later_ = false; 66 BeginOmniboxSession();
67 }
62 68
63 // Don't provide suggestions in incognito mode. 69 // Don't provide suggestions in incognito mode.
64 if (client_->IsOffTheRecord()) { 70 if (client_->IsOffTheRecord()) {
65 done_ = true; 71 done_ = true;
66 nearby_url_count_ = 0; 72 nearby_url_count_ = 0;
73 nearby_url_count_at_focus_ = 0;
74 nearby_url_count_at_focus_valid_ = true;
67 return; 75 return;
68 } 76 }
69 77
70 physical_web::PhysicalWebDataSource* data_source = 78 physical_web::PhysicalWebDataSource* data_source =
71 client_->GetPhysicalWebDataSource(); 79 client_->GetPhysicalWebDataSource();
72 if (!data_source) { 80 if (!data_source) {
73 done_ = true; 81 done_ = true;
74 nearby_url_count_ = 0; 82 nearby_url_count_ = 0;
83 nearby_url_count_at_focus_ = 0;
84 nearby_url_count_at_focus_valid_ = true;
75 return; 85 return;
76 } 86 }
77 87
78 const bool input_from_focus = input.from_omnibox_focus(); 88 auto metadata_list = data_source->GetMetadataList();
89 nearby_url_count_ = metadata_list->size();
90 if (input_from_focus) {
91 nearby_url_count_at_focus_ = nearby_url_count_;
92 nearby_url_count_at_focus_valid_ = true;
93 }
94
79 const bool empty_input_from_user = !input_from_focus && input.text().empty(); 95 const bool empty_input_from_user = !input_from_focus && input.text().empty();
80 96
81 if (input_from_focus || empty_input_from_user) { 97 if (input_from_focus || empty_input_from_user) {
82 ConstructZeroSuggestMatches(data_source->GetMetadataList()); 98 ConstructZeroSuggestMatches(std::move(metadata_list));
83 99
84 if (!matches_.empty()) { 100 if (!matches_.empty()) {
85 had_physical_web_suggestions_ = true; 101 had_physical_web_suggestions_ = true;
86 had_physical_web_suggestions_at_focus_or_later_ = true; 102 had_physical_web_suggestions_at_focus_or_later_ = true;
87 } 103 }
88 104
89 // Don't show zero-suggest suggestions unless the PhysicalWebZeroSuggest 105 // Don't show zero-suggest suggestions unless the PhysicalWebZeroSuggest
90 // omnibox experiment parameter is enabled. If the omnibox input is empty 106 // omnibox experiment parameter is enabled. If the omnibox input is empty
91 // because the user cleared it, also require that PhysicalWebAfterTyping is 107 // because the user cleared it, also require that PhysicalWebAfterTyping is
92 // enabled. 108 // enabled.
93 if (!zero_suggest_enabled_ || 109 if (!zero_suggest_enabled_ ||
94 (empty_input_from_user && !after_typing_enabled_)) { 110 (empty_input_from_user && !after_typing_enabled_)) {
95 matches_.clear(); 111 matches_.clear();
96 } 112 }
97 113
98 // In zero-suggest, Physical Web matches should never be default. If the 114 // In zero-suggest, Physical Web matches should never be default. If the
99 // omnibox input is non-empty and we have at least one Physical Web match, 115 // omnibox input is non-empty and we have at least one Physical Web match,
100 // add the current URL as the default so that hitting enter after focusing 116 // add the current URL as the default so that hitting enter after focusing
101 // the omnibox causes the current page to reload. If the input field is 117 // the omnibox causes the current page to reload. If the input field is
102 // empty, no default match is required. 118 // empty, no default match is required.
103 if (!matches_.empty() && !input.text().empty()) { 119 if (!matches_.empty() && !input.text().empty()) {
104 matches_.push_back(VerbatimMatchForURL( 120 matches_.push_back(VerbatimMatchForURL(
105 client_, input, input.current_url(), history_url_provider_, -1)); 121 client_, input, input.current_url(), history_url_provider_, -1));
106 } 122 }
107 } else { 123 } else {
108 ConstructQuerySuggestMatches(data_source->GetMetadataList(), input); 124 ConstructQuerySuggestMatches(std::move(metadata_list), input);
109 125
110 if (!matches_.empty()) { 126 if (!matches_.empty()) {
111 had_physical_web_suggestions_ = true; 127 had_physical_web_suggestions_ = true;
112 had_physical_web_suggestions_at_focus_or_later_ = true; 128 had_physical_web_suggestions_at_focus_or_later_ = true;
113 } 129 }
114 130
115 // Don't show Physical Web suggestions after the user starts typing unless 131 // Don't show Physical Web suggestions after the user starts typing unless
116 // the PhysicalWebAfterTyping omnibox experiment parameter is enabled. 132 // the PhysicalWebAfterTyping omnibox experiment parameter is enabled.
117 if (!after_typing_enabled_) { 133 if (!after_typing_enabled_) {
118 matches_.clear(); 134 matches_.clear();
(...skipping 22 matching lines...) Expand all
141 if (had_physical_web_suggestions_) 157 if (had_physical_web_suggestions_)
142 new_entry.mutable_field_trial_triggered()->Add(field_trial_hashes[i]); 158 new_entry.mutable_field_trial_triggered()->Add(field_trial_hashes[i]);
143 if (had_physical_web_suggestions_at_focus_or_later_) { 159 if (had_physical_web_suggestions_at_focus_or_later_) {
144 new_entry.mutable_field_trial_triggered_in_session()->Add( 160 new_entry.mutable_field_trial_triggered_in_session()->Add(
145 field_trial_hashes[i]); 161 field_trial_hashes[i]);
146 } 162 }
147 } 163 }
148 164
149 // When the user accepts an autocomplete suggestion, record the number of 165 // When the user accepts an autocomplete suggestion, record the number of
150 // nearby Physical Web URLs at the time the provider last constructed matches. 166 // nearby Physical Web URLs at the time the provider last constructed matches.
151 UMA_HISTOGRAM_EXACT_LINEAR("Omnibox.SuggestionUsed.NearbyURLCount", 167 UMA_HISTOGRAM_EXACT_LINEAR(
152 nearby_url_count_, 50); 168 "Omnibox.SuggestionUsed.NearbyURLCount.AtMatchCreation",
169 nearby_url_count_, 50);
170
171 // On Android, it is somehow possible to accept an omnibox suggestion without
172 // first focusing the omnibox. In this situation, the nearby URL count at
173 // focus will be invalid because the omnibox was never focused. In an earlier
174 // version, this caused invalid data to be recorded by the histogram. Now we
175 // guard against recording the invalid data and instead record that we hit
176 // this corner case.
177 // TODO(crbug.com/691059): Remove once the focus-less suggestion mystery is
178 // solved.
179 UMA_HISTOGRAM_BOOLEAN(
180 "Omnibox.PhysicalWebProvider.SuggestionUsedWithoutOmniboxFocus",
181 !nearby_url_count_at_focus_valid_);
182
183 if (nearby_url_count_at_focus_valid_) {
184 // When the user accepts an autocomplete suggestion, record the number of
185 // nearby Physical Web URLs at the time the omnibox input was last focused.
186 UMA_HISTOGRAM_EXACT_LINEAR("Omnibox.SuggestionUsed.NearbyURLCount.AtFocus",
187 nearby_url_count_at_focus_, 50);
188 }
153 } 189 }
154 190
155 PhysicalWebProvider::PhysicalWebProvider( 191 PhysicalWebProvider::PhysicalWebProvider(
156 AutocompleteProviderClient* client, 192 AutocompleteProviderClient* client,
157 HistoryURLProvider* history_url_provider) 193 HistoryURLProvider* history_url_provider)
158 : AutocompleteProvider(AutocompleteProvider::TYPE_PHYSICAL_WEB), 194 : AutocompleteProvider(AutocompleteProvider::TYPE_PHYSICAL_WEB),
159 client_(client), 195 client_(client),
160 history_url_provider_(history_url_provider), 196 history_url_provider_(history_url_provider),
197 nearby_url_count_(kInvalidUrlCount),
198 nearby_url_count_at_focus_(kInvalidUrlCount),
199 nearby_url_count_at_focus_valid_(false),
161 zero_suggest_enabled_( 200 zero_suggest_enabled_(
162 OmniboxFieldTrial::InPhysicalWebZeroSuggestFieldTrial()), 201 OmniboxFieldTrial::InPhysicalWebZeroSuggestFieldTrial()),
163 after_typing_enabled_( 202 after_typing_enabled_(
164 OmniboxFieldTrial::InPhysicalWebAfterTypingFieldTrial()), 203 OmniboxFieldTrial::InPhysicalWebAfterTypingFieldTrial()),
165 zero_suggest_base_relevance_( 204 zero_suggest_base_relevance_(
166 OmniboxFieldTrial::GetPhysicalWebZeroSuggestBaseRelevance()), 205 OmniboxFieldTrial::GetPhysicalWebZeroSuggestBaseRelevance()),
167 after_typing_base_relevance_( 206 after_typing_base_relevance_(
168 OmniboxFieldTrial::GetPhysicalWebAfterTypingBaseRelevance()) {} 207 OmniboxFieldTrial::GetPhysicalWebAfterTypingBaseRelevance()),
208 had_physical_web_suggestions_(false),
209 had_physical_web_suggestions_at_focus_or_later_(false) {}
169 210
170 PhysicalWebProvider::~PhysicalWebProvider() { 211 PhysicalWebProvider::~PhysicalWebProvider() {
171 } 212 }
172 213
214 void PhysicalWebProvider::BeginOmniboxSession() {
215 // Some metrics are tracked per-session and must be reset each time a new
216 // session begins. Per-session metrics are recorded in AddProviderInfo.
217 nearby_url_count_at_focus_ = kInvalidUrlCount;
218 nearby_url_count_at_focus_valid_ = false;
219 had_physical_web_suggestions_at_focus_or_later_ = false;
220 }
221
173 void PhysicalWebProvider::ConstructZeroSuggestMatches( 222 void PhysicalWebProvider::ConstructZeroSuggestMatches(
174 std::unique_ptr<physical_web::MetadataList> metadata_list) { 223 std::unique_ptr<physical_web::MetadataList> metadata_list) {
175 nearby_url_count_ = metadata_list->size(); 224 size_t nearby_url_count = metadata_list->size();
176 size_t used_slots = 0; 225 size_t used_slots = 0;
177 226
178 for (size_t i = 0; i < nearby_url_count_; ++i) { 227 for (size_t i = 0; i < nearby_url_count; ++i) {
179 const auto& metadata_item = (*metadata_list)[i]; 228 const auto& metadata_item = (*metadata_list)[i];
180 std::string url_string = metadata_item.resolved_url.spec(); 229 std::string url_string = metadata_item.resolved_url.spec();
181 std::string title_string = metadata_item.title; 230 std::string title_string = metadata_item.title;
182 base::string16 title = 231 base::string16 title =
183 AutocompleteMatch::SanitizeString(base::UTF8ToUTF16(title_string)); 232 AutocompleteMatch::SanitizeString(base::UTF8ToUTF16(title_string));
184 233
185 // Add match items with decreasing relevance to preserve the ordering in 234 // Add match items with decreasing relevance to preserve the ordering in
186 // the metadata list. 235 // the metadata list.
187 int relevance = zero_suggest_base_relevance_ - used_slots; 236 int relevance = zero_suggest_base_relevance_ - used_slots;
188 237
189 // Append an overflow item if creating a match for each metadata item would 238 // Append an overflow item if creating a match for each metadata item would
190 // exceed the match limit. 239 // exceed the match limit.
191 const size_t remaining_slots = kPhysicalWebMaxMatches - used_slots; 240 const size_t remaining_slots = kPhysicalWebMaxMatches - used_slots;
192 const size_t remaining_metadata = nearby_url_count_ - i; 241 const size_t remaining_metadata = nearby_url_count - i;
193 if ((remaining_slots == 1) && (remaining_metadata > remaining_slots)) { 242 if ((remaining_slots == 1) && (remaining_metadata > remaining_slots)) {
194 AppendOverflowItem(remaining_metadata, relevance, title); 243 AppendOverflowItem(remaining_metadata, relevance, title);
195 break; 244 break;
196 } 245 }
197 246
198 GURL url(url_string); 247 GURL url(url_string);
199 248
200 AutocompleteMatch match(this, relevance, false, 249 AutocompleteMatch match(this, relevance, false,
201 AutocompleteMatchType::PHYSICAL_WEB); 250 AutocompleteMatchType::PHYSICAL_WEB);
202 match.destination_url = url; 251 match.destination_url = url;
(...skipping 80 matching lines...) Expand 10 before | Expand all | Expand 10 after
283 AutocompleteInput::FormattedStringWithEquivalentMeaning( 332 AutocompleteInput::FormattedStringWithEquivalentMeaning(
284 url, match.contents, client_->GetSchemeClassifier()); 333 url, match.contents, client_->GetSchemeClassifier());
285 334
286 match.description = 335 match.description =
287 l10n_util::GetStringUTF16(IDS_PHYSICAL_WEB_OVERFLOW_DESCRIPTION); 336 l10n_util::GetStringUTF16(IDS_PHYSICAL_WEB_OVERFLOW_DESCRIPTION);
288 match.description_class.push_back( 337 match.description_class.push_back(
289 ACMatchClassification(0, ACMatchClassification::NONE)); 338 ACMatchClassification(0, ACMatchClassification::NONE));
290 339
291 matches_.push_back(match); 340 matches_.push_back(match);
292 } 341 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698