[Omnibox][ML] Populate signals when logging or ml scoring is on.

Merge request to merge into M115. (cherry picked from commit 908c974) Bug: 1448616 Change-Id: Ib592c3176cf0ced241d670a3f2c05fd0dc91275d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4562849 Reviewed-by: Moe Ahmadi <mahmadi@chromium.org> Commit-Queue: Jun Zou <junzou@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1148857} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4573727 Reviewed-by: Tommy Li <tommycli@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/branch-heads/5790@{#149} Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
chromium · May 30, 2023 · 92d381e · 92d381e
1 parent aed0635
commit 92d381e
Show file tree

Hide file tree

Showing 11 changed files with 39 additions and 23 deletions.
diff --git a/components/omnibox/browser/autocomplete_controller.cc b/components/omnibox/browser/autocomplete_controller.cc
@@ -391,7 +391,7 @@ AutocompleteController::AutocompleteController(
 #endif
 
   // Create URL scoring signal annotators.
-  if (OmniboxFieldTrial::IsLogUrlScoringSignalsEnabled() &&
+  if (OmniboxFieldTrial::IsPopulatingUrlScoringSignalsEnabled() &&
       OmniboxFieldTrial::AreScoringSignalsAnnotatorsEnabled()) {
     url_scoring_signals_annotators_.push_back(
         std::make_unique<HistoryScoringSignalsAnnotator>(
@@ -997,7 +997,7 @@ void AutocompleteController::UpdateResult(
   // The additional signals in `result_` will be lost when `UpdateResult()` is
   // called again. Currently, `result_` is updated in each `UpdateResult()`
   // call.
-  if (OmniboxFieldTrial::IsLogUrlScoringSignalsEnabled() &&
+  if (OmniboxFieldTrial::IsPopulatingUrlScoringSignalsEnabled() &&
       OmniboxFieldTrial::AreScoringSignalsAnnotatorsEnabled()) {
     for (const auto& annotator : url_scoring_signals_annotators_) {
       annotator->AnnotateResult(input_, &result_);

diff --git a/components/omnibox/browser/autocomplete_match.cc b/components/omnibox/browser/autocomplete_match.cc
@@ -1427,7 +1427,7 @@ void AutocompleteMatch::UpgradeMatchWithPropertiesFrom(
 
   // Merge scoring signals from duplicate match for ML model scoring and
   // training.
-  if (OmniboxFieldTrial::IsLogUrlScoringSignalsEnabled()) {
+  if (OmniboxFieldTrial::IsPopulatingUrlScoringSignalsEnabled()) {
     MergeScoringSignals(duplicate_match);
   }
 }

diff --git a/components/omnibox/browser/bookmark_provider.cc b/components/omnibox/browser/bookmark_provider.cc
@@ -250,11 +250,11 @@ std::pair<int, int> BookmarkProvider::CalculateBookmarkMatchRelevance(
   int relevance = static_cast<int>(normalized_sum * kBookmarkScoreRange) +
                   kBaseBookmarkScore;
 
-  // If scoring signal logging is disabled, skip counting bookmarks if relevance
-  // is above max score. Don't waste any time searching for additional
-  // referenced URLs if we already have a perfect title match. Returns a pair of
-  // the relevance score and -1 as a dummy bookmark count.
-  if (!OmniboxFieldTrial::IsLogUrlScoringSignalsEnabled() &&
+  // If scoring signal logging and ML scoring is disabled, skip counting
+  // bookmarks if relevance is above max score. Don't waste any time searching
+  // for additional referenced URLs if we already have a perfect title match.
+  // Returns a pair of the relevance score and -1 as a dummy bookmark count.
+  if (!OmniboxFieldTrial::IsPopulatingUrlScoringSignalsEnabled() &&
       relevance >= kMaxBookmarkScore) {
     return {relevance, /*bookmark_count=*/-1};
   }

diff --git a/components/omnibox/browser/history_quick_provider.cc b/components/omnibox/browser/history_quick_provider.cc
@@ -361,7 +361,7 @@ AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch(
     match.from_keyword = true;
   }
 
-  if (OmniboxFieldTrial::IsLogUrlScoringSignalsEnabled() &&
+  if (OmniboxFieldTrial::IsPopulatingUrlScoringSignalsEnabled() &&
       AutocompleteScoringSignalsAnnotator::IsEligibleMatch(match)) {
     // Propagate scoring signals to AC Match for ML Model training data.
     // `allowed_to_be_default_match` is set in this function, after the ACMatch

diff --git a/components/omnibox/browser/history_url_provider.cc b/components/omnibox/browser/history_url_provider.cc
@@ -753,7 +753,7 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
 void HistoryURLProvider::PromoteMatchesIfNecessary(
     const HistoryURLProviderParams& params) {
   bool populate_scoring_signals =
-      OmniboxFieldTrial::IsLogUrlScoringSignalsEnabled();
+      OmniboxFieldTrial::IsPopulatingUrlScoringSignalsEnabled();
   if (params.promote_type == HistoryURLProviderParams::NEITHER)
     return;
   if (params.promote_type == HistoryURLProviderParams::FRONT_HISTORY_MATCH) {
@@ -817,7 +817,7 @@ void HistoryURLProvider::QueryComplete(
                                    ? 1
                                    : 0;
     bool populate_scoring_signals =
-        OmniboxFieldTrial::IsLogUrlScoringSignalsEnabled();
+        OmniboxFieldTrial::IsPopulatingUrlScoringSignalsEnabled();
     for (size_t i = first_match; i < params->matches.size(); ++i) {
       // All matches score one less than the previous match.
       --relevance;

diff --git a/components/omnibox/browser/omnibox_field_trial.cc b/components/omnibox/browser/omnibox_field_trial.cc
@@ -1016,11 +1016,17 @@ const base::FeatureParam<bool> kDomainSuggestionsAlternativeScoring(
 
 // If true, enables scoring signal annotators for logging Omnibox scoring
 // signals to OmniboxEventProto.
-const base::FeatureParam<bool> kEnableScoringSignalsAnnotators(
+const base::FeatureParam<bool> kEnableScoringSignalsAnnotatorsForLogging(
     &omnibox::kLogUrlScoringSignals,
     "enable_scoring_signals_annotators",
     false);
 
+// If true, enables scoring signal annotators for ML scoring.
+const base::FeatureParam<bool> kEnableScoringSignalsAnnotatorsForMlScoring(
+    &omnibox::kMlUrlScoring,
+    "enable_scoring_signals_annotators_for_ml_scoring",
+    false);
+
 // If true, runs the ML scoring model but does not assign new relevance scores
 // to the URL suggestions and does not rerank them.
 const base::FeatureParam<bool> kMlUrlScoringCounterfactual(
@@ -1061,7 +1067,9 @@ const base::FeatureParam<bool> kMlBatchUrlScoring(&omnibox::kMlUrlScoring,
 MLConfig::MLConfig() {
   log_url_scoring_signals =
       base::FeatureList::IsEnabled(omnibox::kLogUrlScoringSignals);
-  enable_scoring_signals_annotators = kEnableScoringSignalsAnnotators.Get();
+  enable_scoring_signals_annotators =
+      kEnableScoringSignalsAnnotatorsForLogging.Get() ||
+      kEnableScoringSignalsAnnotatorsForMlScoring.Get();
   ml_url_scoring = base::FeatureList::IsEnabled(omnibox::kMlUrlScoring);
   ml_batch_url_scoring = kMlBatchUrlScoring.Get();
   ml_url_scoring_counterfactual = kMlUrlScoringCounterfactual.Get();
@@ -1088,9 +1096,14 @@ const MLConfig& GetMLConfig() {
   return GetMLConfigInternal();
 }
 
-bool IsLogUrlScoringSignalsEnabled() {
+bool IsReportingUrlScoringSignalsEnabled() {
   return GetMLConfig().log_url_scoring_signals;
 }
+
+bool IsPopulatingUrlScoringSignalsEnabled() {
+  return IsReportingUrlScoringSignalsEnabled() || IsMlUrlScoringEnabled();
+}
+
 bool AreScoringSignalsAnnotatorsEnabled() {
   return GetMLConfig().enable_scoring_signals_annotators;
 }

diff --git a/components/omnibox/browser/omnibox_field_trial.h b/components/omnibox/browser/omnibox_field_trial.h
@@ -602,9 +602,8 @@ struct MLConfig {
   // Equivalent to omnibox::kLogUrlScoringSignals.
   bool log_url_scoring_signals{false};
 
-  // If true, enables scoring signal annotators for logging Omnibox URL scoring
-  // signals to OmniboxEventProto. Equivalent to
-  // OmniboxFieldTrial::kLogUrlScoringSignalsEnableScoringSignalsAnnotators.
+  // If true, enables scoring signal annotators for populating additional
+  // Omnibox URL scoring signals for logging or ML scoring.
   bool enable_scoring_signals_annotators{false};
 
   // If true, runs the ML scoring model to assign new relevance scores to the
@@ -664,7 +663,10 @@ const MLConfig& GetMLConfig();
 
 // If enabled, logs Omnibox scoring signals to OmniboxEventProto for training
 // the ML scoring models.
-bool IsLogUrlScoringSignalsEnabled();
+bool IsReportingUrlScoringSignalsEnabled();
+
+// If enabled, populates scoring signals of URL matches.
+bool IsPopulatingUrlScoringSignalsEnabled();
 
 // Whether the scoring signal annotators are enabled for logging Omnibox scoring
 // signals to OmniboxEventProto.

diff --git a/components/omnibox/browser/omnibox_metrics_provider.cc b/components/omnibox/browser/omnibox_metrics_provider.cc
@@ -207,7 +207,7 @@ void OmniboxMetricsProvider::RecordOmniboxOpenedURL(const OmniboxLog& log) {
 
     // Scoring signals are not logged for search suggestions or in incognito
     // mode.
-    if (OmniboxFieldTrial::IsLogUrlScoringSignalsEnabled() &&
+    if (OmniboxFieldTrial::IsReportingUrlScoringSignalsEnabled() &&
         !AutocompleteMatch::IsSearchType(i->type) && !log.is_incognito &&
         i->scoring_signals) {
       suggestion->mutable_scoring_signals()->CopyFrom(*i->scoring_signals);

diff --git a/components/omnibox/browser/scored_history_match.cc b/components/omnibox/browser/scored_history_match.cc
@@ -162,7 +162,7 @@ ScoredHistoryMatch::ScoredHistoryMatch(
   // has been constructed via the no-args constructor.
   ScoredHistoryMatch::Init();
 
-  if (OmniboxFieldTrial::IsLogUrlScoringSignalsEnabled()) {
+  if (OmniboxFieldTrial::IsPopulatingUrlScoringSignalsEnabled()) {
     // Populate the scoring signals available in the URL Row.
     scoring_signals = absl::make_optional<ScoringSignals>();
     scoring_signals->set_typed_count(row.typed_count());
@@ -715,7 +715,7 @@ float ScoredHistoryMatch::GetTopicalityScore(
   IncrementTitleMatchTermScores(terms_to_word_starts_offsets,
                                 word_starts.title_word_starts_, &term_scores);
 
-  if (OmniboxFieldTrial::IsLogUrlScoringSignalsEnabled()) {
+  if (OmniboxFieldTrial::IsPopulatingUrlScoringSignalsEnabled()) {
     // Url matching signals.
     const auto url_matching_signals = ComputeUrlMatchingSignals(
         terms_to_word_starts_offsets, url, word_starts.url_word_starts_,

diff --git a/components/omnibox/browser/shortcuts_provider.cc b/components/omnibox/browser/shortcuts_provider.cc
@@ -197,7 +197,8 @@ void ShortcutsProvider::Start(const AutocompleteInput& input,
   if (input.focus_type() == metrics::OmniboxFocusType::INTERACTION_DEFAULT &&
       input.type() != metrics::OmniboxInputType::EMPTY &&
       !input.text().empty() && initialized_) {
-    GetMatches(input, OmniboxFieldTrial::IsLogUrlScoringSignalsEnabled());
+    GetMatches(input,
+               OmniboxFieldTrial::IsPopulatingUrlScoringSignalsEnabled());
   }
 }
 

diff --git a/components/omnibox/browser/titled_url_match_utils.cc b/components/omnibox/browser/titled_url_match_utils.cc
@@ -147,7 +147,7 @@ AutocompleteMatch TitledUrlMatchToAutocompleteMatch(
     match.from_keyword = true;
   }
 
-  if (OmniboxFieldTrial::IsLogUrlScoringSignalsEnabled() &&
+  if (OmniboxFieldTrial::IsPopulatingUrlScoringSignalsEnabled() &&
       AutocompleteScoringSignalsAnnotator::IsEligibleMatch(match)) {
     match.scoring_signals = absl::make_optional<ScoringSignals>();
     // Populate ACMatches with signals for ML model scoring and training.