Commit df6487ff authored by manukh's avatar manukh Committed by Commit Bot

[omnibox] Add feature-slicing to omnibox logs.

Adds a OmniboxTriggeredFeatureService, accessible through
AutocompleteProviderClient, that registers features as they're triggered
which are copied to the metrics logs once an omnibox session ends.

Change-Id: I311fc20b185dd58b1acc6174321e47a38f794c67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2437743
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Reviewed-by: default avatarMoe Ahmadi <mahmadi@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Reviewed-by: default avatarMark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823683}
parent 27eda86b
...@@ -329,7 +329,7 @@ void AutocompleteControllerAndroid::OnSuggestionSelected( ...@@ -329,7 +329,7 @@ void AutocompleteControllerAndroid::OnSuggestionSelected(
now - autocomplete_controller_->last_time_default_match_changed(), now - autocomplete_controller_->last_time_default_match_changed(),
autocomplete_controller_->result()); autocomplete_controller_->result());
log.is_query_started_from_tile = is_query_started_from_tiles_; log.is_query_started_from_tile = is_query_started_from_tiles_;
autocomplete_controller_->AddProvidersInfo(&log.providers_info); autocomplete_controller_->AddProviderAndTriggeringLogs(&log);
OmniboxEventGlobalTracker::GetInstance()->OnURLOpened(&log); OmniboxEventGlobalTracker::GetInstance()->OnURLOpened(&log);
......
...@@ -180,7 +180,9 @@ ChromeAutocompleteProviderClient::ChromeAutocompleteProviderClient( ...@@ -180,7 +180,9 @@ ChromeAutocompleteProviderClient::ChromeAutocompleteProviderClient(
unified_consent::UrlKeyedDataCollectionConsentHelper:: unified_consent::UrlKeyedDataCollectionConsentHelper::
NewPersonalizedDataCollectionConsentHelper( NewPersonalizedDataCollectionConsentHelper(
ProfileSyncServiceFactory::GetForProfile(profile_))), ProfileSyncServiceFactory::GetForProfile(profile_))),
storage_partition_(nullptr) { storage_partition_(nullptr),
omnibox_triggered_feature_service_(
std::make_unique<OmniboxTriggeredFeatureService>()) {
if (OmniboxFieldTrial::IsPedalSuggestionsEnabled()) if (OmniboxFieldTrial::IsPedalSuggestionsEnabled())
pedal_provider_ = std::make_unique<OmniboxPedalProvider>(*this); pedal_provider_ = std::make_unique<OmniboxPedalProvider>(*this);
} }
...@@ -250,12 +252,6 @@ ChromeAutocompleteProviderClient::GetRemoteSuggestionsService( ...@@ -250,12 +252,6 @@ ChromeAutocompleteProviderClient::GetRemoteSuggestionsService(
create_if_necessary); create_if_necessary);
} }
query_tiles::TileService*
ChromeAutocompleteProviderClient::GetQueryTileService() const {
ProfileKey* profile_key = profile_->GetProfileKey();
return query_tiles::TileServiceFactory::GetForKey(profile_key);
}
DocumentSuggestionsService* DocumentSuggestionsService*
ChromeAutocompleteProviderClient::GetDocumentSuggestionsService( ChromeAutocompleteProviderClient::GetDocumentSuggestionsService(
bool create_if_necessary) const { bool create_if_necessary) const {
...@@ -344,6 +340,17 @@ ChromeAutocompleteProviderClient::GetComponentUpdateService() { ...@@ -344,6 +340,17 @@ ChromeAutocompleteProviderClient::GetComponentUpdateService() {
return g_browser_process->component_updater(); return g_browser_process->component_updater();
} }
query_tiles::TileService*
ChromeAutocompleteProviderClient::GetQueryTileService() const {
ProfileKey* profile_key = profile_->GetProfileKey();
return query_tiles::TileServiceFactory::GetForKey(profile_key);
}
OmniboxTriggeredFeatureService*
ChromeAutocompleteProviderClient::GetOmniboxTriggeredFeatureService() const {
return omnibox_triggered_feature_service_.get();
}
bool ChromeAutocompleteProviderClient::IsOffTheRecord() const { bool ChromeAutocompleteProviderClient::IsOffTheRecord() const {
return profile_->IsOffTheRecord(); return profile_->IsOffTheRecord();
} }
......
...@@ -62,6 +62,8 @@ class ChromeAutocompleteProviderClient : public AutocompleteProviderClient { ...@@ -62,6 +62,8 @@ class ChromeAutocompleteProviderClient : public AutocompleteProviderClient {
component_updater::ComponentUpdateService* GetComponentUpdateService() component_updater::ComponentUpdateService* GetComponentUpdateService()
override; override;
query_tiles::TileService* GetQueryTileService() const override; query_tiles::TileService* GetQueryTileService() const override;
OmniboxTriggeredFeatureService* GetOmniboxTriggeredFeatureService()
const override;
bool IsOffTheRecord() const override; bool IsOffTheRecord() const override;
bool SearchSuggestEnabled() const override; bool SearchSuggestEnabled() const override;
...@@ -118,6 +120,9 @@ class ChromeAutocompleteProviderClient : public AutocompleteProviderClient { ...@@ -118,6 +120,9 @@ class ChromeAutocompleteProviderClient : public AutocompleteProviderClient {
// Injectable storage partitiion, used for testing. // Injectable storage partitiion, used for testing.
content::StoragePartition* storage_partition_; content::StoragePartition* storage_partition_;
std::unique_ptr<OmniboxTriggeredFeatureService>
omnibox_triggered_feature_service_;
DISALLOW_COPY_AND_ASSIGN(ChromeAutocompleteProviderClient); DISALLOW_COPY_AND_ASSIGN(ChromeAutocompleteProviderClient);
}; };
......
...@@ -914,7 +914,7 @@ void SearchTabHelper::OpenAutocompleteMatch( ...@@ -914,7 +914,7 @@ void SearchTabHelper::OpenAutocompleteMatch(
/*elapsed_time_since_last_change_to_default_match=*/ /*elapsed_time_since_last_change_to_default_match=*/
elapsed_time_since_last_change_to_default_match, elapsed_time_since_last_change_to_default_match,
/*result=*/autocomplete_controller_->result()); /*result=*/autocomplete_controller_->result());
autocomplete_controller_->AddProvidersInfo(&log.providers_info); autocomplete_controller_->AddProviderAndTriggeringLogs(&log);
OmniboxEventGlobalTracker::GetInstance()->OnURLOpened(&log); OmniboxEventGlobalTracker::GetInstance()->OnURLOpened(&log);
......
...@@ -1127,7 +1127,7 @@ void NewTabPageHandler::OpenAutocompleteMatch( ...@@ -1127,7 +1127,7 @@ void NewTabPageHandler::OpenAutocompleteMatch(
/*elapsed_time_since_last_change_to_default_match=*/ /*elapsed_time_since_last_change_to_default_match=*/
elapsed_time_since_last_change_to_default_match, elapsed_time_since_last_change_to_default_match,
/*result=*/autocomplete_controller_->result()); /*result=*/autocomplete_controller_->result());
autocomplete_controller_->AddProvidersInfo(&log.providers_info); autocomplete_controller_->AddProviderAndTriggeringLogs(&log);
OmniboxEventGlobalTracker::GetInstance()->OnURLOpened(&log); OmniboxEventGlobalTracker::GetInstance()->OnURLOpened(&log);
......
...@@ -167,6 +167,8 @@ static_library("browser") { ...@@ -167,6 +167,8 @@ static_library("browser") {
"omnibox_popup_view.h", "omnibox_popup_view.h",
"omnibox_prefs.cc", "omnibox_prefs.cc",
"omnibox_prefs.h", "omnibox_prefs.h",
"omnibox_triggered_feature_service.cc",
"omnibox_triggered_feature_service.h",
"omnibox_view.cc", "omnibox_view.cc",
"omnibox_view.h", "omnibox_view.h",
"on_device_head_model.cc", "on_device_head_model.cc",
......
...@@ -544,12 +544,12 @@ void AutocompleteController::OnProviderUpdate(bool updated_matches) { ...@@ -544,12 +544,12 @@ void AutocompleteController::OnProviderUpdate(bool updated_matches) {
UpdateResult(false, false); UpdateResult(false, false);
} }
void AutocompleteController::AddProvidersInfo( void AutocompleteController::AddProviderAndTriggeringLogs(
ProvidersInfo* provider_info) const { OmniboxLog* logs) const {
provider_info->clear(); logs->providers_info.clear();
for (auto i(providers_.begin()); i != providers_.end(); ++i) { for (const auto& provider : providers_) {
// Add per-provider info, if any. // Add per-provider info, if any.
(*i)->AddProviderInfo(provider_info); provider->AddProviderInfo(&logs->providers_info);
// This is also a good place to put code to add info that you want to // This is also a good place to put code to add info that you want to
// add for every provider. // add for every provider.
...@@ -559,16 +559,22 @@ void AutocompleteController::AddProvidersInfo( ...@@ -559,16 +559,22 @@ void AutocompleteController::AddProvidersInfo(
// OmniboxPedalProvider is not a "true" AutocompleteProvider and isn't // OmniboxPedalProvider is not a "true" AutocompleteProvider and isn't
// included in the list of providers, though needs to report information for // included in the list of providers, though needs to report information for
// its field trial. Manually call AddProviderInfo for pedals. // its field trial. Manually call AddProviderInfo for pedals.
provider_client_->GetPedalProvider()->AddProviderInfo(provider_info); provider_client_->GetPedalProvider()->AddProviderInfo(
&logs->providers_info);
} }
// Add any features that have been triggered.
// |GetOmniboxTriggeredFeatureService()| can be null in tests.
if (provider_client_->GetOmniboxTriggeredFeatureService())
provider_client_->GetOmniboxTriggeredFeatureService()->RecordToLogs(
&logs->feature_triggered_in_session);
} }
void AutocompleteController::ResetSession() { void AutocompleteController::ResetSession() {
search_service_worker_signal_sent_ = false; search_service_worker_signal_sent_ = false;
for (Providers::const_iterator i(providers_.begin()); i != providers_.end(); for (const auto& provider : providers_) {
++i) { provider->ResetSession();
(*i)->ResetSession();
} }
if (OmniboxFieldTrial::IsPedalSuggestionsEnabled()) { if (OmniboxFieldTrial::IsPedalSuggestionsEnabled()) {
...@@ -576,6 +582,10 @@ void AutocompleteController::ResetSession() { ...@@ -576,6 +582,10 @@ void AutocompleteController::ResetSession() {
// a "true" AutocompleteProvider. Manually call ResetSession() for pedals. // a "true" AutocompleteProvider. Manually call ResetSession() for pedals.
provider_client_->GetPedalProvider()->ResetSession(); provider_client_->GetPedalProvider()->ResetSession();
} }
// |GetOmniboxTriggeredFeatureService()| can be null in tests.
if (provider_client_->GetOmniboxTriggeredFeatureService())
provider_client_->GetOmniboxTriggeredFeatureService()->ResetSession();
} }
void AutocompleteController::UpdateMatchDestinationURLWithQueryFormulationTime( void AutocompleteController::UpdateMatchDestinationURLWithQueryFormulationTime(
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include "components/omnibox/browser/autocomplete_provider_client.h" #include "components/omnibox/browser/autocomplete_provider_client.h"
#include "components/omnibox/browser/autocomplete_provider_listener.h" #include "components/omnibox/browser/autocomplete_provider_listener.h"
#include "components/omnibox/browser/autocomplete_result.h" #include "components/omnibox/browser/autocomplete_result.h"
#include "components/omnibox/browser/omnibox_log.h"
class ClipboardProvider; class ClipboardProvider;
class DocumentProvider; class DocumentProvider;
...@@ -127,12 +128,10 @@ class AutocompleteController : public AutocompleteProviderListener, ...@@ -127,12 +128,10 @@ class AutocompleteController : public AutocompleteProviderListener,
void OnProviderUpdate(bool updated_matches) override; void OnProviderUpdate(bool updated_matches) override;
// Called when an omnibox event log entry is generated. // Called when an omnibox event log entry is generated.
// Populates provider_info with diagnostic information about the status // Populates |log.provider_info| with diagnostic information about the status
// of various providers. In turn, calls // of various providers and |log.feature_triggered_in_session| with triggered
// AutocompleteProvider::AddProviderInfo() so each provider can add // features.
// provider-specific information, information we want to log for a particular void AddProviderAndTriggeringLogs(OmniboxLog* logs) const;
// provider but not others.
void AddProvidersInfo(ProvidersInfo* provider_info) const;
// Called when a new omnibox session starts. // Called when a new omnibox session starts.
// We start a new session when the user first begins modifying the omnibox // We start a new session when the user first begins modifying the omnibox
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "components/history/core/browser/keyword_id.h" #include "components/history/core/browser/keyword_id.h"
#include "components/history/core/browser/top_sites.h" #include "components/history/core/browser/top_sites.h"
#include "components/omnibox/browser/keyword_extensions_delegate.h" #include "components/omnibox/browser/keyword_extensions_delegate.h"
#include "components/omnibox/browser/omnibox_triggered_feature_service.h"
#include "components/omnibox/browser/shortcuts_backend.h" #include "components/omnibox/browser/shortcuts_backend.h"
#include "third_party/metrics_proto/omnibox_event.pb.h" #include "third_party/metrics_proto/omnibox_event.pb.h"
...@@ -79,6 +80,8 @@ class AutocompleteProviderClient { ...@@ -79,6 +80,8 @@ class AutocompleteProviderClient {
virtual std::unique_ptr<KeywordExtensionsDelegate> virtual std::unique_ptr<KeywordExtensionsDelegate>
GetKeywordExtensionsDelegate(KeywordProvider* keyword_provider) = 0; GetKeywordExtensionsDelegate(KeywordProvider* keyword_provider) = 0;
virtual query_tiles::TileService* GetQueryTileService() const = 0; virtual query_tiles::TileService* GetQueryTileService() const = 0;
virtual OmniboxTriggeredFeatureService* GetOmniboxTriggeredFeatureService()
const = 0;
// The value to use for Accept-Languages HTTP header when making an HTTP // The value to use for Accept-Languages HTTP header when making an HTTP
// request. // request.
......
...@@ -83,6 +83,10 @@ class MockAutocompleteProviderClient ...@@ -83,6 +83,10 @@ class MockAutocompleteProviderClient
query_tiles::TileService* GetQueryTileService() const override { query_tiles::TileService* GetQueryTileService() const override {
return nullptr; return nullptr;
} }
OmniboxTriggeredFeatureService* GetOmniboxTriggeredFeatureService()
const override {
return nullptr;
}
component_updater::ComponentUpdateService* GetComponentUpdateService() component_updater::ComponentUpdateService* GetComponentUpdateService()
override { override {
......
...@@ -846,7 +846,7 @@ void OmniboxEditModel::OpenMatch(AutocompleteMatch match, ...@@ -846,7 +846,7 @@ void OmniboxEditModel::OpenMatch(AutocompleteMatch match,
// tab, we don't know the tab ID yet.) // tab, we don't know the tab ID yet.)
log.tab_id = client_->GetSessionID(); log.tab_id = client_->GetSessionID();
} }
autocomplete_controller()->AddProvidersInfo(&log.providers_info); autocomplete_controller()->AddProviderAndTriggeringLogs(&log);
client_->OnURLOpenedFromOmnibox(&log); client_->OnURLOpenedFromOmnibox(&log);
OmniboxEventGlobalTracker::GetInstance()->OnURLOpened(&log); OmniboxEventGlobalTracker::GetInstance()->OnURLOpened(&log);
LOCAL_HISTOGRAM_BOOLEAN("Omnibox.EventCount", true); LOCAL_HISTOGRAM_BOOLEAN("Omnibox.EventCount", true);
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "components/omnibox/browser/autocomplete_provider.h" #include "components/omnibox/browser/autocomplete_provider.h"
#include "components/omnibox/browser/omnibox_triggered_feature_service.h"
#include "components/sessions/core/session_id.h" #include "components/sessions/core/session_id.h"
#include "third_party/metrics_proto/omnibox_event.pb.h" #include "third_party/metrics_proto/omnibox_event.pb.h"
#include "third_party/metrics_proto/omnibox_input_type.pb.h" #include "third_party/metrics_proto/omnibox_input_type.pb.h"
...@@ -107,10 +108,14 @@ struct OmniboxLog { ...@@ -107,10 +108,14 @@ struct OmniboxLog {
const AutocompleteResult& result; const AutocompleteResult& result;
// Diagnostic information from providers. See // Diagnostic information from providers. See
// AutocompleteController::AddProvidersInfo() and // AutocompleteController::AddProviderAndTriggeringLogs() and
// AutocompleteProvider::AddProviderInfo() above. // AutocompleteProvider::AddProviderInfo().
ProvidersInfo providers_info; ProvidersInfo providers_info;
// The features that have been triggered (see
// OmniboxTriggeredFeatureService::Feature).
OmniboxTriggeredFeatureService::Features feature_triggered_in_session;
// Whether the omnibox input is a search query that is started // Whether the omnibox input is a search query that is started
// by clicking on a image tile. Currently only used on Android. // by clicking on a image tile. Currently only used on Android.
bool is_query_started_from_tile = false; bool is_query_started_from_tile = false;
......
...@@ -119,4 +119,8 @@ void OmniboxMetricsProvider::RecordOmniboxOpenedURL(const OmniboxLog& log) { ...@@ -119,4 +119,8 @@ void OmniboxMetricsProvider::RecordOmniboxOpenedURL(const OmniboxLog& log) {
omnibox_event->set_keyword_mode_entry_method(log.keyword_mode_entry_method); omnibox_event->set_keyword_mode_entry_method(log.keyword_mode_entry_method);
if (log.is_query_started_from_tile) if (log.is_query_started_from_tile)
omnibox_event->set_is_query_started_from_tile(true); omnibox_event->set_is_query_started_from_tile(true);
for (auto feature : log.feature_triggered_in_session) {
omnibox_event->add_feature_triggered_in_session(
static_cast<size_t>(feature));
}
} }
// Copyright 2020 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 "omnibox_triggered_feature_service.h"
OmniboxTriggeredFeatureService::OmniboxTriggeredFeatureService() = default;
OmniboxTriggeredFeatureService::~OmniboxTriggeredFeatureService() = default;
void OmniboxTriggeredFeatureService::RecordToLogs(
Features* feature_triggered_in_session) const {
*feature_triggered_in_session = features_;
}
void OmniboxTriggeredFeatureService::TriggerFeature(Feature feature) {
features_.insert(feature);
}
void OmniboxTriggeredFeatureService::ResetSession() {
features_.clear();
}
// Copyright 2020 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.
#ifndef COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_TRIGGERED_FEATURE_SERVICE_H_
#define COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_TRIGGERED_FEATURE_SERVICE_H_
#include <set>
#include <vector>
// Tracks the features that trigger during an omnibox session and records them
// to the logs. This is used for counterfactual slicing metrics by feature.
class OmniboxTriggeredFeatureService {
public:
// The list of features used for counterfactual slicing.
enum class Feature {};
using Features = std::set<Feature>;
OmniboxTriggeredFeatureService();
~OmniboxTriggeredFeatureService();
// Records |features_| for this session to |feature_triggered_in_session|.
void RecordToLogs(Features* feature_triggered_in_session) const;
// Invoked to indicate |feature| was triggered.
void TriggerFeature(Feature feature);
// Invoked when a new omnibox session starts. Clears |features_|.
void ResetSession();
private:
// The set of features triggered in the current omnibox session via
// |TriggerFeature()|.
std::set<Feature> features_;
};
#endif // COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_TRIGGERED_FEATURE_SERVICE_H_
...@@ -48,6 +48,8 @@ class AutocompleteProviderClientImpl : public AutocompleteProviderClient { ...@@ -48,6 +48,8 @@ class AutocompleteProviderClientImpl : public AutocompleteProviderClient {
std::unique_ptr<KeywordExtensionsDelegate> GetKeywordExtensionsDelegate( std::unique_ptr<KeywordExtensionsDelegate> GetKeywordExtensionsDelegate(
KeywordProvider* keyword_provider) override; KeywordProvider* keyword_provider) override;
query_tiles::TileService* GetQueryTileService() const override; query_tiles::TileService* GetQueryTileService() const override;
OmniboxTriggeredFeatureService* GetOmniboxTriggeredFeatureService()
const override;
std::string GetAcceptLanguages() const override; std::string GetAcceptLanguages() const override;
std::string GetEmbedderRepresentationOfAboutScheme() const override; std::string GetEmbedderRepresentationOfAboutScheme() const override;
std::vector<base::string16> GetBuiltinURLs() override; std::vector<base::string16> GetBuiltinURLs() override;
...@@ -78,6 +80,8 @@ class AutocompleteProviderClientImpl : public AutocompleteProviderClient { ...@@ -78,6 +80,8 @@ class AutocompleteProviderClientImpl : public AutocompleteProviderClient {
AutocompleteSchemeClassifierImpl scheme_classifier_; AutocompleteSchemeClassifierImpl scheme_classifier_;
std::unique_ptr<unified_consent::UrlKeyedDataCollectionConsentHelper> std::unique_ptr<unified_consent::UrlKeyedDataCollectionConsentHelper>
url_consent_helper_; url_consent_helper_;
std::unique_ptr<OmniboxTriggeredFeatureService>
omnibox_triggered_feature_service_;
DISALLOW_COPY_AND_ASSIGN(AutocompleteProviderClientImpl); DISALLOW_COPY_AND_ASSIGN(AutocompleteProviderClientImpl);
}; };
......
...@@ -45,7 +45,9 @@ AutocompleteProviderClientImpl::AutocompleteProviderClientImpl( ...@@ -45,7 +45,9 @@ AutocompleteProviderClientImpl::AutocompleteProviderClientImpl(
url_consent_helper_(unified_consent::UrlKeyedDataCollectionConsentHelper:: url_consent_helper_(unified_consent::UrlKeyedDataCollectionConsentHelper::
NewPersonalizedDataCollectionConsentHelper( NewPersonalizedDataCollectionConsentHelper(
ProfileSyncServiceFactory::GetForBrowserState( ProfileSyncServiceFactory::GetForBrowserState(
browser_state_))) {} browser_state_))),
omnibox_triggered_feature_service_(
std::make_unique<OmniboxTriggeredFeatureService>()) {}
AutocompleteProviderClientImpl::~AutocompleteProviderClientImpl() {} AutocompleteProviderClientImpl::~AutocompleteProviderClientImpl() {}
...@@ -141,6 +143,11 @@ query_tiles::TileService* AutocompleteProviderClientImpl::GetQueryTileService() ...@@ -141,6 +143,11 @@ query_tiles::TileService* AutocompleteProviderClientImpl::GetQueryTileService()
return nullptr; return nullptr;
} }
OmniboxTriggeredFeatureService*
AutocompleteProviderClientImpl::GetOmniboxTriggeredFeatureService() const {
return omnibox_triggered_feature_service_.get();
}
std::string AutocompleteProviderClientImpl::GetAcceptLanguages() const { std::string AutocompleteProviderClientImpl::GetAcceptLanguages() const {
return browser_state_->GetPrefs()->GetString( return browser_state_->GetPrefs()->GetString(
language::prefs::kAcceptLanguages); language::prefs::kAcceptLanguages);
......
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