diff options
| author | mpearson@chromium.org <mpearson@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-20 23:49:03 +0000 |
|---|---|---|
| committer | mpearson@chromium.org <mpearson@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-20 23:49:03 +0000 |
| commit | 35d5560157d379015cd937816154368af1a0c602 (patch) | |
| tree | f22422d1c76611c94d32715eb82c43252906ef23 /chrome/browser/autocomplete | |
| parent | e6088be7a989ec3dd9ad29b8db12aaebf749a9ba (diff) | |
| download | chromium_src-35d5560157d379015cd937816154368af1a0c602.zip chromium_src-35d5560157d379015cd937816154368af1a0c602.tar.gz chromium_src-35d5560157d379015cd937816154368af1a0c602.tar.bz2 | |
Omnibox: Log Elapsed Time Since Last Time The Default Match Changed
For users opted-in in UMA, in the logs that record interactions with the omnibox. keep track of the length of time the default match has been displayed.
BUG=
TEST=by hand, using LOG(INFO)
Review URL: https://chromiumcodereview.appspot.com/11316057
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@168913 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/autocomplete')
4 files changed, 105 insertions, 34 deletions
diff --git a/chrome/browser/autocomplete/autocomplete_controller.cc b/chrome/browser/autocomplete/autocomplete_controller.cc index 9c2a406..e745381 100644 --- a/chrome/browser/autocomplete/autocomplete_controller.cc +++ b/chrome/browser/autocomplete/autocomplete_controller.cc @@ -213,7 +213,20 @@ void AutocompleteController::Start( } in_start_ = false; CheckIfDone(); - UpdateResult(true); + // The second true forces saying the default match has changed. + // This triggers the edit model to update things such as the inline + // autocomplete state. In particular, if the user has typed a key + // since the last notification, and we're now re-running + // autocomplete, then we need to update the inline autocompletion + // even if the current match is for the same URL as the last run's + // default match. Likewise, the controller doesn't know what's + // happened in the edit since the last time it ran autocomplete. + // The user might have selected all the text and hit delete, then + // typed a new character. The selection and delete won't send any + // signals to the controller so it doesn't realize that anything was + // cleared or changed. Even if the default match hasn't changed, we + // need the edit model to update the display. + UpdateResult(false, true); if (!done_) StartExpireTimer(); @@ -263,13 +276,15 @@ void AutocompleteController::DeleteMatch(const AutocompleteMatch& match) { } void AutocompleteController::ExpireCopiedEntries() { - // Clear out the results. This ensures no results from the previous result set - // are copied over. - result_.Reset(); - // We allow matches from the previous result set to starve out matches from - // the new result set. This means in order to expire matches we have to query - // the providers again. - UpdateResult(false); + // The first true makes UpdateResult() clear out the results and + // regenerate them, thus ensuring that no results from the previous + // result set remain. + // The second true says to pretend the default match changed (even + // if it did not). This may not be necessary but the code has been + // like this for a long time and there's probably no great benefit in + // changing it. Classically we have erred on the side of notifying + // too often because it hasn't had any major side effects. + UpdateResult(true, true); } void AutocompleteController::OnProviderUpdate(bool updated_matches) { @@ -285,7 +300,7 @@ void AutocompleteController::OnProviderUpdate(bool updated_matches) { // Multiple providers may provide synchronous results, so we only update the // results if we're not in Start(). if (!in_start_ && (updated_matches || done_)) - UpdateResult(false); + UpdateResult(false, false); } } @@ -302,7 +317,23 @@ void AutocompleteController::AddProvidersInfo( } } -void AutocompleteController::UpdateResult(bool is_synchronous_pass) { +void AutocompleteController::UpdateResult( + bool regenerate_result, + bool force_notify_default_match_changed) { + const bool last_default_was_valid = result_.default_match() != result_.end(); + // The following two variables are only set and used if + // |last_default_was_valid|. + string16 last_default_fill_into_edit, last_default_associated_keyword; + if (last_default_was_valid) { + last_default_fill_into_edit = result_.default_match()->fill_into_edit; + if (result_.default_match()->associated_keyword != NULL) + last_default_associated_keyword = + result_.default_match()->associated_keyword->keyword; + } + + if (regenerate_result) + result_.Reset(); + AutocompleteResult last_result; last_result.Swap(&result_); @@ -329,27 +360,29 @@ void AutocompleteController::UpdateResult(bool is_synchronous_pass) { UpdateAssociatedKeywords(&result_); UpdateAssistedQueryStats(&result_); - bool notify_default_match = is_synchronous_pass; - if (!is_synchronous_pass) { - const bool last_default_was_valid = - last_result.default_match() != last_result.end(); - const bool default_is_valid = result_.default_match() != result_.end(); - // We've gotten async results. Send notification that the default match - // updated if fill_into_edit differs or associated_keyword differ. (The - // latter can change if we've just started Chrome and the keyword database - // finishes loading while processing this request.) We don't check the URL - // as that may change for the default match even though the fill into edit - // hasn't changed (see SearchProvider for one case of this). - notify_default_match = - (last_default_was_valid != default_is_valid) || - (default_is_valid && - ((result_.default_match()->fill_into_edit != - last_result.default_match()->fill_into_edit) || - (result_.default_match()->associated_keyword.get() != - last_result.default_match()->associated_keyword.get()))); + const bool default_is_valid = result_.default_match() != result_.end(); + string16 default_associated_keyword; + if (default_is_valid && + (result_.default_match()->associated_keyword != NULL)) { + default_associated_keyword = + result_.default_match()->associated_keyword->keyword; } - - NotifyChanged(notify_default_match); + // We've gotten async results. Send notification that the default match + // updated if fill_into_edit differs or associated_keyword differ. (The + // latter can change if we've just started Chrome and the keyword database + // finishes loading while processing this request.) We don't check the URL + // as that may change for the default match even though the fill into edit + // hasn't changed (see SearchProvider for one case of this). + const bool notify_default_match = + (last_default_was_valid != default_is_valid) || + (last_default_was_valid && + ((result_.default_match()->fill_into_edit != + last_default_fill_into_edit) || + (default_associated_keyword != last_default_associated_keyword))); + if (notify_default_match) + last_time_default_match_changed_ = base::TimeTicks::Now(); + + NotifyChanged(force_notify_default_match_changed || notify_default_match); } void AutocompleteController::UpdateAssociatedKeywords( diff --git a/chrome/browser/autocomplete/autocomplete_controller.h b/chrome/browser/autocomplete/autocomplete_controller.h index d0466c7..2339c70 100644 --- a/chrome/browser/autocomplete/autocomplete_controller.h +++ b/chrome/browser/autocomplete/autocomplete_controller.h @@ -9,6 +9,7 @@ #include "base/compiler_specific.h" #include "base/gtest_prod_util.h" #include "base/string16.h" +#include "base/time.h" #include "base/timer.h" #include "chrome/browser/autocomplete/autocomplete_input.h" #include "chrome/browser/autocomplete/autocomplete_provider.h" @@ -125,6 +126,10 @@ class AutocompleteController : public AutocompleteProviderListener { bool done() const { return done_; } const ACProviders* providers() const { return &providers_; } + const base::TimeTicks& last_time_default_match_changed() const { + return last_time_default_match_changed_; + } + // AutocompleteProviderListener: virtual void OnProviderUpdate(bool updated_matches) OVERRIDE; @@ -142,10 +147,22 @@ class AutocompleteController : public AutocompleteProviderListener { RedundantKeywordsIgnoredInResult); FRIEND_TEST_ALL_PREFIXES(AutocompleteProviderTest, UpdateAssistedQueryStats); - // Updates |result_| to reflect the current provider state. Resets timers and - // fires notifications as necessary. |is_synchronous_pass| is true only when - // Start() is calling this to get the synchronous result. - void UpdateResult(bool is_synchronous_pass); + // Updates |result_| to reflect the current provider state and fires + // notifications. If |regenerate_result| then we clear the result + // so when we incorporate the current provider state we end up + // implicitly removing all expired matches. (Normally we allow + // matches from the previous result set carry over. These stale + // results may outrank legitimate matches from the current result + // set. Sometimes we just want the current matches; the easier way + // to do this is to throw everything out and reconstruct the result + // set from the providers' current data.) + // If |force_notify_default_match_changed|, we tell NotifyChanged + // the default match has changed even if it hasn't. This is + // necessary in some cases; for instance, if the user typed a new + // character, the edit model needs to repaint (highlighting changed) + // even if the default match didn't change. + void UpdateResult(bool regenerate_result, + bool force_notify_default_match_changed); // Updates |result| to populate each match's |associated_keyword| if that // match can show a keyword hint. |result| should be sorted by @@ -188,6 +205,18 @@ class AutocompleteController : public AutocompleteProviderListener { // Data from the autocomplete query. AutocompleteResult result_; + // The most recent time the default match (inline match) changed. This may + // be earlier than the most recent keystroke if the recent keystrokes didn't + // change the suggested match in the omnibox. (For instance, if + // a user typed "mail.goog" and the match https://mail.google.com/ was + // the destination match ever since the user typed "ma" then this is + // the time that URL first appeared as the default match.) This may + // also be more recent than the last keystroke if there was an + // asynchronous provider that returned and changed the default + // match. See UpdateResult() for details on when we consider a + // match to have changed. + base::TimeTicks last_time_default_match_changed_; + // Timer used to remove any matches copied from the last result. When run // invokes |ExpireCopiedEntries|. base::OneShotTimer<AutocompleteController> expire_timer_; diff --git a/chrome/browser/autocomplete/autocomplete_log.cc b/chrome/browser/autocomplete/autocomplete_log.cc index 77023c0..7d6f1e6 100644 --- a/chrome/browser/autocomplete/autocomplete_log.cc +++ b/chrome/browser/autocomplete/autocomplete_log.cc @@ -13,6 +13,7 @@ AutocompleteLog::AutocompleteLog( metrics::OmniboxEventProto::PageClassification current_page_classification, base::TimeDelta elapsed_time_since_user_first_modified_omnibox, size_t inline_autocompleted_length, + base::TimeDelta elapsed_time_since_last_change_to_default_match, const AutocompleteResult& result) : text(text), just_deleted_text(just_deleted_text), @@ -23,6 +24,8 @@ AutocompleteLog::AutocompleteLog( elapsed_time_since_user_first_modified_omnibox( elapsed_time_since_user_first_modified_omnibox), inline_autocompleted_length(inline_autocompleted_length), + elapsed_time_since_last_change_to_default_match( + elapsed_time_since_last_change_to_default_match), result(result), providers_info() { } diff --git a/chrome/browser/autocomplete/autocomplete_log.h b/chrome/browser/autocomplete/autocomplete_log.h index 245481b..2782eef 100644 --- a/chrome/browser/autocomplete/autocomplete_log.h +++ b/chrome/browser/autocomplete/autocomplete_log.h @@ -29,6 +29,7 @@ struct AutocompleteLog { current_page_classification, base::TimeDelta elapsed_time_since_user_first_modified_omnibox, size_t inline_autocompleted_length, + base::TimeDelta elapsed_time_since_last_change_to_default_match, const AutocompleteResult& result); ~AutocompleteLog(); @@ -67,6 +68,11 @@ struct AutocompleteLog { // if not available. size_t inline_autocompleted_length; + // The amount of time since the last time the default (i.e., inline) + // match changed. This will certainly be less than + // elapsed_time_since_user_first_modified_omnibox. + base::TimeDelta elapsed_time_since_last_change_to_default_match; + // Result set. const AutocompleteResult& result; |