Commit 7c9cf9a9 authored by Sam Maier's avatar Sam Maier Committed by Commit Bot

Revert "[omnibox metrics] Copy new keyword information into log"

This reverts commit a404b85a.

Reason for revert: Suspected cause of crbug.com/906242

Original change's description:
> [omnibox metrics] Copy new keyword information into log
> 
> We added new fields to the Omnibox event protobuf regarding keyword
> mode. This CL copies the new relevant information from a set of
> suggestions into that protobuf. Also adds new boolean field to a
> suggestion.
> 
> Bug: 837395
> Change-Id: I496d5235790a3c3cd1744155fe766b134a366fd5
> Reviewed-on: https://chromium-review.googlesource.com/c/1312930
> Reviewed-by: Mark Pearson <mpearson@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Commit-Queue: Kevin Bailey <krb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#608528}

TBR=mpearson@chromium.org,krb@chromium.org,tedchoc@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 837395
Change-Id: I9c4a39a1d3104a65796d43b31233cc36647f586c
Reviewed-on: https://chromium-review.googlesource.com/c/1347031Reviewed-by: default avatarSam Maier <smaier@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610214}
parent 55d657cc
......@@ -268,8 +268,6 @@ void AutocompleteControllerAndroid::OnSuggestionSelected(
input_.from_omnibox_focus() ? base::string16() : input_.text(),
false, /* don't know */
input_.type(),
false, /* not keyword mode */
OmniboxEventProto::INVALID,
true,
selected_index,
WindowOpenDisposition::CURRENT_TAB,
......
......@@ -484,9 +484,6 @@ struct AutocompleteMatch {
// it!
base::string16 keyword;
// Set in matches originating from keyword results.
bool from_keyword;
// Set to a matching pedal if appropriate. The pedal is not owned, and the
// owning OmniboxPedalProvider must outlive this.
OmniboxPedal* pedal = nullptr;
......
......@@ -151,7 +151,7 @@ bool BaseSearchProvider::ShouldPrefetch(const AutocompleteMatch& match) {
AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
const base::string16& suggestion,
AutocompleteMatchType::Type type,
bool from_keyword,
bool from_keyword_provider,
const TemplateURL* template_url,
const SearchTermsData& search_terms_data) {
// These calls use a number of default values. For instance, they assume
......@@ -159,12 +159,12 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
// mode. They also assume the caller knows what it's doing and we set
// this match to look as if it was received/created synchronously.
SearchSuggestionParser::SuggestResult suggest_result(
suggestion, type, /*subtype_identifier=*/0, from_keyword,
suggestion, type, /*subtype_identifier=*/0, from_keyword_provider,
/*relevance=*/0, /*relevance_from_server=*/false,
/*input_text=*/base::string16());
suggest_result.set_received_after_last_keystroke(false);
return CreateSearchSuggestion(nullptr, AutocompleteInput(),
from_keyword, suggest_result,
from_keyword_provider, suggest_result,
template_url, search_terms_data, 0, false);
}
......@@ -291,19 +291,17 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
const base::string16 input_lower = base::i18n::ToLower(input.text());
// suggestion.match_contents() should have already been collapsed.
match.allowed_to_be_default_match =
(!in_keyword_mode || suggestion.from_keyword()) &&
(!in_keyword_mode || suggestion.from_keyword_provider()) &&
(base::CollapseWhitespace(input_lower, false) ==
base::i18n::ToLower(suggestion.match_contents()));
if (suggestion.from_keyword()) {
match.from_keyword = true;
if (suggestion.from_keyword_provider())
match.fill_into_edit.append(match.keyword + base::char16(' '));
}
// We only allow inlinable navsuggestions that were received before the
// last keystroke because we don't want asynchronous inline autocompletions.
if (!input.prevent_inline_autocomplete() &&
!suggestion.received_after_last_keystroke() &&
(!in_keyword_mode || suggestion.from_keyword()) &&
(!in_keyword_mode || suggestion.from_keyword_provider()) &&
base::StartsWith(
base::i18n::ToLower(suggestion.suggestion()), input_lower,
base::CompareCase::SENSITIVE)) {
......@@ -337,7 +335,7 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
*match.search_terms_args, search_terms_data));
// Search results don't look like URLs.
match.transition = suggestion.from_keyword() ?
match.transition = suggestion.from_keyword_provider() ?
ui::PAGE_TRANSITION_KEYWORD : ui::PAGE_TRANSITION_GENERATED;
return match;
......@@ -420,8 +418,8 @@ void BaseSearchProvider::AddMatchToMap(
bool in_keyword_mode,
MatchMap* map) {
AutocompleteMatch match = CreateSearchSuggestion(
this, GetInput(result.from_keyword()), in_keyword_mode, result,
GetTemplateURL(result.from_keyword()),
this, GetInput(result.from_keyword_provider()), in_keyword_mode, result,
GetTemplateURL(result.from_keyword_provider()),
client_->GetTemplateURLService()->search_terms_data(),
accepted_suggestion, ShouldAppendExtraParams(result));
if (!match.destination_url.is_valid())
......
......@@ -482,7 +482,6 @@ AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
FillInURLAndContents(remaining_input, template_url, &match);
match.keyword = keyword;
match.from_keyword = true;
match.transition = ui::PAGE_TRANSITION_KEYWORD;
return match;
......
......@@ -719,9 +719,8 @@ void OmniboxEditModel::OpenMatch(AutocompleteMatch match,
fake_single_entry_result.AppendMatches(input_, fake_single_entry_matches);
OmniboxLog log(
input_.from_omnibox_focus() ? base::string16() : input_text,
just_deleted_text_, input_.type(), is_keyword_selected(),
keyword_mode_entry_method_, popup_open, dropdown_ignored ? 0 : index,
disposition, !pasted_text.empty(),
just_deleted_text_, input_.type(), popup_open,
dropdown_ignored ? 0 : index, disposition, !pasted_text.empty(),
SessionID::InvalidValue(), // don't know tab ID; set later if appropriate
ClassifyPage(), elapsed_time_since_user_first_modified_omnibox,
match.allowed_to_be_default_match ? match.inline_autocompletion.length()
......
......@@ -8,8 +8,6 @@ OmniboxLog::OmniboxLog(
const base::string16& text,
bool just_deleted_text,
metrics::OmniboxInputType input_type,
bool in_keyword_mode,
metrics::OmniboxEventProto::KeywordModeEntryMethod entry_method,
bool is_popup_open,
size_t selected_index,
WindowOpenDisposition disposition,
......@@ -23,8 +21,6 @@ OmniboxLog::OmniboxLog(
: text(text),
just_deleted_text(just_deleted_text),
input_type(input_type),
in_keyword_mode(in_keyword_mode),
keyword_mode_entry_method(entry_method),
is_popup_open(is_popup_open),
selected_index(selected_index),
disposition(disposition),
......
......@@ -23,8 +23,6 @@ struct OmniboxLog {
OmniboxLog(const base::string16& text,
bool just_deleted_text,
metrics::OmniboxInputType input_type,
bool in_keyword_mode,
metrics::OmniboxEventProto::KeywordModeEntryMethod entry_method,
bool is_popup_open,
size_t selected_index,
WindowOpenDisposition disposition,
......@@ -48,14 +46,6 @@ struct OmniboxLog {
// The detected type of the user's input.
metrics::OmniboxInputType input_type;
// Whether the Omnibox was in keyword mode when the user selected a
// suggestion.
bool in_keyword_mode;
// Preserves the method that the user used to enter keyword mode. If
// |in_keyword_mode| is false, this should be INVALID.
metrics::OmniboxEventProto::KeywordModeEntryMethod keyword_mode_entry_method;
// True if the popup is open.
bool is_popup_open;
......
......@@ -169,14 +169,10 @@ void OmniboxMetricsProvider::RecordOmniboxOpenedURL(const OmniboxLog& log) {
if (i->subtype_identifier > 0)
suggestion->set_result_subtype_identifier(i->subtype_identifier);
suggestion->set_has_tab_match(i->has_tab_match);
suggestion->set_is_keyword_suggestion(i->from_keyword);
}
for (auto i(log.providers_info.begin()); i != log.providers_info.end(); ++i) {
OmniboxEventProto::ProviderInfo* provider_info =
omnibox_event->add_provider_info();
provider_info->CopyFrom(*i);
}
omnibox_event->set_in_keyword_mode(log.in_keyword_mode);
if (log.in_keyword_mode)
omnibox_event->set_keyword_mode_entry_method(log.keyword_mode_entry_method);
}
......@@ -343,7 +343,7 @@ const AutocompleteInput SearchProvider::GetInput(bool is_keyword) const {
bool SearchProvider::ShouldAppendExtraParams(
const SearchSuggestionParser::SuggestResult& result) const {
return !result.from_keyword() ||
return !result.from_keyword_provider() ||
providers_.default_provider().empty();
}
......@@ -1013,7 +1013,7 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
SearchSuggestionParser::SuggestResult verbatim(
/*suggestion=*/trimmed_verbatim,
AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
/*subtype_identifier=*/0, /*from_keyword=*/false,
/*subtype_identifier=*/0, /*from_keyword_provider=*/false,
verbatim_relevance, relevance_from_server,
/*input_text=*/trimmed_verbatim);
if (has_answer)
......@@ -1039,7 +1039,7 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
SearchSuggestionParser::SuggestResult verbatim(
/*suggestion=*/trimmed_verbatim,
AutocompleteMatchType::SEARCH_OTHER_ENGINE,
/*subtype_identifier=*/0, /*from_keyword=*/true,
/*subtype_identifier=*/0, /*from_keyword_provider=*/true,
keyword_verbatim_relevance, keyword_relevance_from_server,
/*input_text=*/trimmed_verbatim);
AddMatchToMap(verbatim, std::string(),
......@@ -1451,7 +1451,7 @@ AutocompleteMatch SearchProvider::NavigationToMatch(
const SearchSuggestionParser::NavigationResult& navigation) {
base::string16 input;
const bool trimmed_whitespace = base::TrimWhitespace(
navigation.from_keyword() ?
navigation.from_keyword_provider() ?
keyword_input_.text() : input_.text(),
base::TRIM_TRAILING, &input) != base::TRIM_NONE;
AutocompleteMatch match(this, navigation.relevance(), false,
......@@ -1515,8 +1515,6 @@ AutocompleteMatch SearchProvider::NavigationToMatch(
navigation.relevance_from_server() ? kTrue : kFalse);
match.RecordAdditionalInfo(kShouldPrefetchKey, kFalse);
match.from_keyword = navigation.from_keyword();
return match;
}
......
......@@ -59,13 +59,13 @@ AutocompleteMatchType::Type GetAutocompleteMatchType(const std::string& type) {
// SearchSuggestionParser::Result ----------------------------------------------
SearchSuggestionParser::Result::Result(bool from_keyword,
SearchSuggestionParser::Result::Result(bool from_keyword_provider,
int relevance,
bool relevance_from_server,
AutocompleteMatchType::Type type,
int subtype_identifier,
const std::string& deletion_url)
: from_keyword_(from_keyword),
: from_keyword_provider_(from_keyword_provider),
type_(type),
subtype_identifier_(subtype_identifier),
relevance_(relevance),
......@@ -83,7 +83,7 @@ SearchSuggestionParser::SuggestResult::SuggestResult(
const base::string16& suggestion,
AutocompleteMatchType::Type type,
int subtype_identifier,
bool from_keyword,
bool from_keyword_provider,
int relevance,
bool relevance_from_server,
const base::string16& input_text)
......@@ -97,7 +97,7 @@ SearchSuggestionParser::SuggestResult::SuggestResult(
/*deletion_url=*/"",
/*image_dominant_color=*/"",
/*image_url=*/"",
from_keyword,
from_keyword_provider,
relevance,
relevance_from_server,
/*should_prefetch=*/false,
......@@ -114,12 +114,12 @@ SearchSuggestionParser::SuggestResult::SuggestResult(
const std::string& deletion_url,
const std::string& image_dominant_color,
const std::string& image_url,
bool from_keyword,
bool from_keyword_provider,
int relevance,
bool relevance_from_server,
bool should_prefetch,
const base::string16& input_text)
: Result(from_keyword,
: Result(from_keyword_provider,
relevance,
relevance_from_server,
type,
......@@ -198,7 +198,7 @@ void SearchSuggestionParser::SuggestResult::SetAnswer(
int SearchSuggestionParser::SuggestResult::CalculateRelevance(
const AutocompleteInput& input,
bool keyword_provider_requested) const {
if (!from_keyword_ && keyword_provider_requested)
if (!from_keyword_provider_ && keyword_provider_requested)
return 100;
return ((input.type() == metrics::OmniboxInputType::URL) ? 300 : 600);
}
......@@ -212,11 +212,11 @@ SearchSuggestionParser::NavigationResult::NavigationResult(
int subtype_identifier,
const base::string16& description,
const std::string& deletion_url,
bool from_keyword,
bool from_keyword_provider,
int relevance,
bool relevance_from_server,
const base::string16& input_text)
: Result(from_keyword,
: Result(from_keyword_provider,
relevance,
relevance_from_server,
type,
......@@ -289,7 +289,7 @@ SearchSuggestionParser::NavigationResult::CalculateAndClassifyMatchContents(
int SearchSuggestionParser::NavigationResult::CalculateRelevance(
const AutocompleteInput& input,
bool keyword_provider_requested) const {
return (from_keyword_ || !keyword_provider_requested) ? 800 : 150;
return (from_keyword_provider_ || !keyword_provider_requested) ? 800 : 150;
}
// SearchSuggestionParser::Results ---------------------------------------------
......
......@@ -42,7 +42,7 @@ class SearchSuggestionParser {
// highly fragmented SearchProvider logic for each Result type.
class Result {
public:
Result(bool from_keyword,
Result(bool from_keyword_provider,
int relevance,
bool relevance_from_server,
AutocompleteMatchType::Type type,
......@@ -51,7 +51,7 @@ class SearchSuggestionParser {
Result(const Result& other);
virtual ~Result();
bool from_keyword() const { return from_keyword_; }
bool from_keyword_provider() const { return from_keyword_provider_; }
const base::string16& match_contents() const { return match_contents_; }
const ACMatchClassifications& match_contents_class() const {
......@@ -89,8 +89,8 @@ class SearchSuggestionParser {
base::string16 match_contents_;
ACMatchClassifications match_contents_class_;
// True if the result came from a keyword suggestion.
bool from_keyword_;
// True if the result came from the keyword provider.
bool from_keyword_provider_;
AutocompleteMatchType::Type type_;
......@@ -127,7 +127,7 @@ class SearchSuggestionParser {
SuggestResult(const base::string16& suggestion,
AutocompleteMatchType::Type type,
int subtype_identifier,
bool from_keyword,
bool from_keyword_provider,
int relevance,
bool relevance_from_server,
const base::string16& input_text);
......@@ -141,7 +141,7 @@ class SearchSuggestionParser {
const std::string& deletion_url,
const std::string& image_dominant_color,
const std::string& image_url,
bool from_keyword,
bool from_keyword_provider,
int relevance,
bool relevance_from_server,
bool should_prefetch,
......@@ -232,7 +232,7 @@ class SearchSuggestionParser {
int subtype_identifier,
const base::string16& description,
const std::string& deletion_url,
bool from_keyword,
bool from_keyword_provider,
int relevance,
bool relevance_from_server,
const base::string16& input_text);
......
......@@ -33,7 +33,6 @@
#include "components/omnibox/browser/omnibox_field_trial.h"
#include "components/omnibox/browser/url_prefix.h"
#include "components/prefs/pref_service.h"
#include "components/search_engines/template_url_service.h"
#include "components/url_formatter/url_fixer.h"
#include "third_party/metrics_proto/omnibox_input_type.pb.h"
#include "url/third_party/mozilla/url_parse.h"
......@@ -291,27 +290,16 @@ AutocompleteMatch ShortcutsProvider::ShortcutToACMatch(
DCHECK(is_search_type != match.keyword.empty());
const bool keyword_matches =
base::StartsWith(base::UTF16ToUTF8(input.text()),
base::StrCat({base::UTF16ToUTF8(match.keyword), " "}),
base::CompareCase::INSENSITIVE_ASCII);
if (is_search_type) {
match.from_keyword =
// Either the match is not from the default search provider:
match.from_keyword =
match.keyword != client_->GetTemplateURLService()
->GetDefaultSearchProvider()
->keyword() ||
// Or it is, but keyword mode was invoked explicitly and the keyword
// in the input is also of the default search provider.
(input.prefer_keyword() && keyword_matches);
}
// True if input is in keyword mode and the match is a URL suggestion or the
// match has a different keyword.
bool would_cause_leaving_keyword_mode =
input.prefer_keyword() && !(is_search_type && keyword_matches);
if (!would_cause_leaving_keyword_mode) {
bool would_cause_leaving_keyboard_mode =
input.prefer_keyword() &&
(!is_search_type ||
!base::StartsWith(base::UTF16ToUTF8(input.text()),
base::StrCat({base::UTF16ToUTF8(match.keyword), " "}),
base::CompareCase::INSENSITIVE_ASCII));
if (!would_cause_leaving_keyboard_mode) {
if (is_search_type) {
if (match.fill_into_edit.size() >= input.text().size() &&
std::equal(match.fill_into_edit.begin(),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment