diff options
| -rw-r--r-- | chrome/app/generated_resources.grd | 15 | ||||
| -rw-r--r-- | chrome/browser/about_flags.cc | 20 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/history_quick_provider.cc | 47 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/history_quick_provider.h | 18 | ||||
| -rw-r--r-- | chrome/common/chrome_switches.cc | 12 | ||||
| -rw-r--r-- | chrome/common/chrome_switches.h | 3 |
6 files changed, 97 insertions, 18 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index fd68740..0427856 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -5517,6 +5517,21 @@ Keep your key file in a safe place. You will need it to create new versions of y <message name="IDS_FLAGS_OMNIBOX_HISTORY_QUICK_PROVIDER_NEW_SCORING_DISABLED" desc="Option name to use old scoring function in HistoryQuickProvider"> Old Scoring </message> + <message name="IDS_FLAGS_OMNIBOX_HISTORY_QUICK_PROVIDER_REORDER_FOR_INLINING_NAME" desc="Title for the flag for omnibox to reorder suggestions in HistoryQuickProvider to make an inlineable appear first"> + Reorder results for inlining in HistoryQuickProvider + </message> + <message name="IDS_FLAGS_OMNIBOX_HISTORY_QUICK_PROVIDER_REORDER_FOR_INLINING_DESCRIPTION" desc="Description for the flag for omnibox to reorder suggestions in HistoryQuickProvider to make an inlineable appear first"> + In omnibox autocomplete, reorder suggestions in HistoryQuickProvider to make an inlineable one appear first. + </message> + <message name="IDS_FLAGS_OMNIBOX_HISTORY_QUICK_PROVIDER_REORDER_FOR_INLINING_AUTOMATIC" desc="Option name for automatic selection of whether to allow HistoryQuickProvider to reorder suggestions to make an inlineable one appear first"> + Automatic + </message> + <message name="IDS_FLAGS_OMNIBOX_HISTORY_QUICK_PROVIDER_REORDER_FOR_INLINING_ENABLED" desc="Option name to allow HistoryQuickProvider to reorder suggestions to make an inlineable one appear first"> + Reorder + </message> + <message name="IDS_FLAGS_OMNIBOX_HISTORY_QUICK_PROVIDER_REORDER_FOR_INLINING_DISABLED" desc="Option name to disallow HistoryQuickProvider to reorder suggestions to make an inlineable one appear first"> + Don't Reorder + </message> <message name="IDS_FLAGS_OMNIBOX_INLINE_HISTORY_QUICK_PROVIDER_NAME" desc="Title for the flag for omnibox to prohibit the HistoryQuickProvider from inlining suggestions"> Inline HistoryQuickProvider suggestions </message> diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc index 4706536..16ea65e8 100644 --- a/chrome/browser/about_flags.cc +++ b/chrome/browser/about_flags.cc @@ -82,6 +82,19 @@ const Experiment::Choice kOmniboxHistoryQuickProviderNewScoringChoices[] = { switches::kOmniboxHistoryQuickProviderNewScoringDisabled } }; +const Experiment::Choice + kOmniboxHistoryQuickProviderReorderForInliningChoices[] = { + { IDS_FLAGS_OMNIBOX_HISTORY_QUICK_PROVIDER_REORDER_FOR_INLINING_AUTOMATIC, + "", + "" }, + { IDS_FLAGS_OMNIBOX_HISTORY_QUICK_PROVIDER_REORDER_FOR_INLINING_ENABLED, + switches::kOmniboxHistoryQuickProviderReorderForInlining, + switches::kOmniboxHistoryQuickProviderReorderForInliningEnabled }, + { IDS_FLAGS_OMNIBOX_HISTORY_QUICK_PROVIDER_REORDER_FOR_INLINING_DISABLED, + switches::kOmniboxHistoryQuickProviderReorderForInlining, + switches::kOmniboxHistoryQuickProviderReorderForInliningDisabled } +}; + const Experiment::Choice kOmniboxInlineHistoryQuickProviderChoices[] = { { IDS_FLAGS_OMNIBOX_INLINE_HISTORY_QUICK_PROVIDER_AUTOMATIC, "", "" }, { IDS_FLAGS_OMNIBOX_INLINE_HISTORY_QUICK_PROVIDER_ALLOWED, @@ -423,6 +436,13 @@ const Experiment kExperiments[] = { MULTI_VALUE_TYPE(kOmniboxHistoryQuickProviderNewScoringChoices) }, { + "omnibox-history-quick-provider-reorder-for-inlining", + IDS_FLAGS_OMNIBOX_HISTORY_QUICK_PROVIDER_REORDER_FOR_INLINING_NAME, + IDS_FLAGS_OMNIBOX_HISTORY_QUICK_PROVIDER_REORDER_FOR_INLINING_DESCRIPTION, + kOsAll, + MULTI_VALUE_TYPE(kOmniboxHistoryQuickProviderReorderForInliningChoices) + }, + { "omnibox-inline-history-quick-provider", IDS_FLAGS_OMNIBOX_INLINE_HISTORY_QUICK_PROVIDER_NAME, IDS_FLAGS_OMNIBOX_INLINE_HISTORY_QUICK_PROVIDER_DESCRIPTION, diff --git a/chrome/browser/autocomplete/history_quick_provider.cc b/chrome/browser/autocomplete/history_quick_provider.cc index dd7fb1d..f903a10 100644 --- a/chrome/browser/autocomplete/history_quick_provider.cc +++ b/chrome/browser/autocomplete/history_quick_provider.cc @@ -45,7 +45,8 @@ bool HistoryQuickProvider::disabled_ = false; HistoryQuickProvider::HistoryQuickProvider(ACProviderListener* listener, Profile* profile) : HistoryProvider(listener, profile, "HistoryQuickProvider"), - languages_(profile_->GetPrefs()->GetString(prefs::kAcceptLanguages)) { + languages_(profile_->GetPrefs()->GetString(prefs::kAcceptLanguages)), + reorder_for_inlining_(false) { enum InliningOption { INLINING_PROHIBITED = 0, INLINING_ALLOWED = 1, @@ -102,6 +103,11 @@ HistoryQuickProvider::HistoryQuickProvider(ACProviderListener* listener, UMA_HISTOGRAM_ENUMERATION( "Omnibox.InlineHistoryQuickProviderFieldTrialBeacon", inlining_option, NUM_OPTIONS); + + reorder_for_inlining_ = CommandLine::ForCurrentProcess()-> + GetSwitchValueASCII(switches:: + kOmniboxHistoryQuickProviderReorderForInlining) == + switches::kOmniboxHistoryQuickProviderReorderForInliningEnabled; } void HistoryQuickProvider::Start(const AutocompleteInput& input, @@ -151,27 +157,32 @@ void HistoryQuickProvider::DoAutocomplete() { if (matches.empty()) return; + if (reorder_for_inlining_) { + // If we're allowed to reorder results in order to get an + // inlineable result to appear first (and hence have a + // HistoryQuickProvider suggestion possibly appear first), find + // the first inlineable result and then swap it to the front. + for (ScoredHistoryMatches::iterator i(matches.begin()); + (i != matches.end()) && + (i->raw_score >= AutocompleteResult::kLowestDefaultScore); + ++i) { + if (i->can_inline) { // this test is only true once because of the break + if (i != matches.begin()) + std::rotate(matches.begin(), i, i + 1); + break; + } + } + } + // Loop over every result and add it to matches_. In the process, // guarantee that scores are decreasing. |max_match_score| keeps // track of the highest score we can assign to any later results we // see. Also, if we're not allowing inline autocompletions in - // general, artificially reduce the starting |max_match_score| - // (which therefore applies to all results) to something low enough - // that guarantees no result will be offered as an autocomplete - // suggestion. In addition, even if we allow inlining of - // suggestions in general, we also reduce the starting - // |max_match_score| if our top suggestion is not inlineable to make - // sure it never gets attempted to be offered as an inline - // suggestion. Note that this strategy will allow a funky case: - // suppose we're allowing inlining in general. If the second result - // is marked as cannot inline yet has a score that would make it - // inlineable, it will keep its score. This is a bit odd--a - // non-inlineable result with a score high enough to make it - // eligible for inlining will keep its high score--but it's okay - // because there is a higher scoring result that is required to be - // shown before this result. Hence, this result, the second in the - // set, will never be inlined. (The autocomplete UI keeps results - // in relevance score order.) + // general or the current best suggestion isn't inlineable, + // artificially reduce the starting |max_match_score| (which + // therefore applies to all results) to something low enough that + // guarantees no result will be offered as an autocomplete + // suggestion. int max_match_score = (PreventInlineAutocomplete(autocomplete_input_) || !matches.begin()->can_inline) ? (AutocompleteResult::kLowestDefaultScore - 1) : diff --git a/chrome/browser/autocomplete/history_quick_provider.h b/chrome/browser/autocomplete/history_quick_provider.h index cee0e2b..9beba79 100644 --- a/chrome/browser/autocomplete/history_quick_provider.h +++ b/chrome/browser/autocomplete/history_quick_provider.h @@ -76,6 +76,24 @@ class HistoryQuickProvider : public HistoryProvider { AutocompleteInput autocomplete_input_; std::string languages_; + // True if we're allowed to reorder results depending on + // inlineability in order to assign higher relevance scores. + // Consider a case where ScoredHistoryMatch provides results x and + // y, where x is not inlineable and has a score of 3000 and y is + // inlineable and has a score of 2500. If reorder_for_inlining_ is + // false, then x gets demoted to a non-inlineable score (1199) and y + // gets demoted to a lower score (1198) because we try to preserve + // the order. On the other hand, if reorder_for_inlining_ is true, + // then y keeps its score of 2500 and x gets demoted to 2499 in + // order to follow y. There will not be any problems with an + // unexpected inline because the non-inlineable result x scores + // lower than the inlineable one. + // TODO(mpearson): remove this variable after we're done experimenting. + // (This member is meant to only exist for experimentation purposes. + // Once we know which behavior is better, we should rip out this variable + // and make the best behavior the default.) + bool reorder_for_inlining_; + // Only used for testing. scoped_ptr<history::InMemoryURLIndex> index_for_testing_; diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc index 23f22e6..a87c46a 100644 --- a/chrome/common/chrome_switches.cc +++ b/chrome/common/chrome_switches.cc @@ -928,6 +928,18 @@ const char kOmniboxHistoryQuickProviderNewScoringEnabled[] = "1"; // 0 means use old scoring ( == current behavior as of 6/2012). const char kOmniboxHistoryQuickProviderNewScoringDisabled[] = "0"; +// Controls whether HistoryQuickProvider is allowed to reorder results +// according to inlineability in order to more aggressively assign/keep +// high relevance scores. +const char kOmniboxHistoryQuickProviderReorderForInlining[] = + "omnibox-history-quick-provider-reorder-for-inlining"; +// The value the kOmniboxHistoryQuickProviderReorderForInlining switch may +// have, as in "--omnibox-history-quick-provider-reorder-for-inlining=1". +// 1 means allow reordering results. +const char kOmniboxHistoryQuickProviderReorderForInliningEnabled[] = "1"; +// 0 means don't allow reordering results ( == current behavior as of 6/2012). +const char kOmniboxHistoryQuickProviderReorderForInliningDisabled[] = "0"; + // Controls whether the omnibox's HistoryQuickProvider is allowed to // inline suggestions. const char kOmniboxInlineHistoryQuickProvider[] = diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h index 5b68da5..a26b43b 100644 --- a/chrome/common/chrome_switches.h +++ b/chrome/common/chrome_switches.h @@ -249,6 +249,9 @@ extern const char kNumPacThreads[]; extern const char kOmniboxHistoryQuickProviderNewScoring[]; extern const char kOmniboxHistoryQuickProviderNewScoringEnabled[]; extern const char kOmniboxHistoryQuickProviderNewScoringDisabled[]; +extern const char kOmniboxHistoryQuickProviderReorderForInlining[]; +extern const char kOmniboxHistoryQuickProviderReorderForInliningEnabled[]; +extern const char kOmniboxHistoryQuickProviderReorderForInliningDisabled[]; extern const char kOmniboxInlineHistoryQuickProvider[]; extern const char kOmniboxInlineHistoryQuickProviderAllowed[]; extern const char kOmniboxInlineHistoryQuickProviderProhibited[]; |