diff options
Diffstat (limited to 'chrome/browser')
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete.cc | 131 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete.h | 116 |
2 files changed, 151 insertions, 96 deletions
diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc index 0288677..cf40e76 100644 --- a/chrome/browser/autocomplete/autocomplete.cc +++ b/chrome/browser/autocomplete/autocomplete.cc @@ -581,11 +581,11 @@ void AutocompleteResult::SortAndCull(const AutocompleteInput& input) { matches_.erase(matches_.begin() + max_matches(), matches_.end()); } - // HistoryContentsProvider use a negative relevance as a way to avoid - // starving out other provider results, yet we may end up using the result. To - // make sure such results are sorted correctly we search for all + // HistoryContentsProvider uses a negative relevance as a way to avoid + // starving out other provider matches, yet we may end up using this match. To + // make sure such matches are sorted correctly we search for all // relevances < 0 and negate them. If we change our relevance algorithm to - // properly mix different providers results, this can go away. + // properly mix different providers' matches, this can go away. for (ACMatches::iterator i = matches_.begin(); i != matches_.end(); ++i) { if (i->relevance < 0) i->relevance = -i->relevance; @@ -617,13 +617,16 @@ void AutocompleteResult::Validate() const { const int AutocompleteController::kNoItemSelected = -1; namespace { -// The maximum time we'll allow the results to go without updating to the -// latest set. -const int kResultUpdateMaxDelayMs = 300; +// The time we'll wait between sending updates to our observers (balances +// flicker against lag). +const int kUpdateDelayMs = 350; }; AutocompleteController::AutocompleteController(Profile* profile) - : done_(true) { + : updated_latest_result_(false), + delay_interval_has_passed_(false), + have_committed_during_this_query_(false), + done_(true) { providers_.push_back(new SearchProvider(this, profile)); providers_.push_back(new HistoryURLProvider(this, profile)); providers_.push_back(new KeywordProvider(this, profile)); @@ -660,40 +663,56 @@ void AutocompleteController::Start(const std::wstring& text, // changes, and when the user finishes an IME composition, inline autocomplete // may no longer be prevented. In both these cases the text itself hasn't // changed since the last query, and some providers can do much less work (and - // get results back more quickly). Taking advantage of this reduces flicker. + // get matches back more quickly). Taking advantage of this reduces flicker. // // NOTE: This comes after constructing |input_| above since that construction // can change the text string (e.g. by stripping off a leading '?'). const bool minimal_changes = (input_.text() == old_input_text) && (input_.synchronous_only() == old_synchronous_only); - // If we're starting a brand new query, send the previous results to the - // observers. - if (!minimal_changes && !done_) + // If we're interrupting an old query, and committing its result won't shrink + // the visible set (which would probably re-expand soon, thus looking very + // flickery), then go ahead and commit what we've got, in order to feel more + // responsive when the user is typing rapidly. + if (!minimal_changes && !done_ && (latest_result_.size() >= result_.size())) CommitResult(); + // If the timer is already running, it could fire shortly after starting this + // query, when we're likely to only have the synchronous results back, thus + // almost certainly causing flicker. Reset it, except when we haven't + // committed anything for the past query, in which case the user is typing + // quickly and we need to keep running the timer lest we lag too far behind. + if (have_committed_during_this_query_) { + update_delay_timer_.Stop(); + delay_interval_has_passed_ = false; + } + // Start the new query. + have_committed_during_this_query_ = false; for (ACProviders::iterator i(providers_.begin()); i != providers_.end(); ++i) { (*i)->Start(input_, minimal_changes); if (synchronous_only) DCHECK((*i)->done()); } + CheckIfDone(); UpdateLatestResult(true); } void AutocompleteController::Stop(bool clear_result) { - for (ACProviders::const_iterator i(providers_.begin()); - i != providers_.end(); ++i) { + for (ACProviders::const_iterator i(providers_.begin()); i != providers_.end(); + ++i) { if (!(*i)->done()) (*i)->Stop(); } + update_delay_timer_.Stop(); + updated_latest_result_ = false; + delay_interval_has_passed_ = false; done_ = true; if (clear_result) result_.Reset(); latest_result_.CopyFrom(result_); - max_delay_timer_.Stop(); } void AutocompleteController::DeleteMatch(const AutocompleteMatch& match) { @@ -707,38 +726,20 @@ void AutocompleteController::DeleteMatch(const AutocompleteMatch& match) { } void AutocompleteController::OnProviderUpdate(bool updated_matches) { - DCHECK(!input_.synchronous_only()); - - if (updated_matches) { + CheckIfDone(); + if (updated_matches || done_) { UpdateLatestResult(false); return; } - - done_ = true; - for (ACProviders::const_iterator i(providers_.begin()); - i != providers_.end(); ++i) { - if (!(*i)->done()) { - done_ = false; - return; - } - } - // In theory we could call Stop() instead of CommitLatestResults() here if we - // knew we'd already called CommitLatestResults() at least once for this - // query. In practice, our observers don't do enough work responding to the - // updates here for the potentially-extra notification to matter. - CommitResult(); } void AutocompleteController::UpdateLatestResult(bool is_synchronous_pass) { - // Add all providers' results. + // Add all providers' matches. latest_result_.Reset(); - done_ = true; - for (ACProviders::const_iterator i(providers_.begin()); - i != providers_.end(); ++i) { + for (ACProviders::const_iterator i(providers_.begin()); i != providers_.end(); + ++i) latest_result_.AppendMatches((*i)->matches()); - if (!(*i)->done()) - done_ = false; - } + updated_latest_result_ = true; // Sort the matches and trim to a small number of "best" matches. latest_result_.SortAndCull(input_); @@ -751,10 +752,10 @@ void AutocompleteController::UpdateLatestResult(bool is_synchronous_pass) { #endif if (is_synchronous_pass) { - if (!max_delay_timer_.IsRunning()) { - max_delay_timer_.Start( - TimeDelta::FromMilliseconds(kResultUpdateMaxDelayMs), - this, &AutocompleteController::CommitResult); + if (!update_delay_timer_.IsRunning()) { + update_delay_timer_.Start( + TimeDelta::FromMilliseconds(kUpdateDelayMs), + this, &AutocompleteController::DelayTimerFired); } NotificationService::current()->Notify( @@ -763,23 +764,42 @@ void AutocompleteController::UpdateLatestResult(bool is_synchronous_pass) { Details<const AutocompleteResult>(&latest_result_)); } - if (done_ || (latest_result_.size() >= result_.size())) + // If nothing is visible, commit immediately so that the first character the + // user types produces an instant response. If the query has finished and we + // haven't ever committed a result set, commit immediately to minimize lag. + // Otherwise, only commit when it's been at least one delay interval since the + // last commit, to minimize flicker. + if (result_.empty() || (done_ && !have_committed_during_this_query_) || + delay_interval_has_passed_) CommitResult(); } +void AutocompleteController::DelayTimerFired() { + delay_interval_has_passed_ = true; + update_delay_timer_.Reset(); + CommitResult(); +} + void AutocompleteController::CommitResult() { - // The max update interval timer either needs to be reset (if more updates - // are to come) or stopped (when we're done with the query). - if (done_) - max_delay_timer_.Stop(); - else - max_delay_timer_.Reset(); + if (!updated_latest_result_) { + // Don't send update notifications when nothing's actually changed. + if (done_) { + update_delay_timer_.Stop(); + delay_interval_has_passed_ = false; + } + return; + } + updated_latest_result_ = false; + delay_interval_has_passed_ = false; + have_committed_during_this_query_ = true; result_.CopyFrom(latest_result_); NotificationService::current()->Notify( NotificationType::AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED, Source<AutocompleteController>(this), Details<const AutocompleteResult>(&result_)); + if (!done_) + update_delay_timer_.Reset(); } ACMatches AutocompleteController::GetMatchesNotInLatestResult( @@ -879,3 +899,14 @@ void AutocompleteController::AddHistoryContentsShortcut() { match.provider = history_contents_provider_; latest_result_.AddMatch(match); } + +void AutocompleteController::CheckIfDone() { + for (ACProviders::const_iterator i(providers_.begin()); i != providers_.end(); + ++i) { + if (!(*i)->done()) { + done_ = false; + return; + } + } + done_ = true; +} diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h index 09ecf8f..b42dc0c 100644 --- a/chrome/browser/autocomplete/autocomplete.h +++ b/chrome/browser/autocomplete/autocomplete.h @@ -19,10 +19,10 @@ // class creates an instance of the controller, which in turn creates a set of // AutocompleteProviders to serve it. The owning class can ask the controller // to Start() a query; the controller in turn passes this call down to the -// providers, each of which keeps track of its own results and whether it has -// finished processing the query. When a provider gets more results or finishes -// processing, it notifies the controller, which merges the combined results -// together and makes them available to interested observers. +// providers, each of which keeps track of its own matches and whether it has +// finished processing the query. When a provider gets more matches or finishes +// processing, it notifies the controller, which merges the combined matches +// together and makes the result available to interested observers. // // The owner may also cancel the current query by calling Stop(), which the // controller will in turn communicate to all the providers. No callbacks will @@ -31,15 +31,12 @@ // IMPORTANT: There is NO THREAD SAFETY built into this portion of the // autocomplete system. All calls to and from the AutocompleteController should // happen on the same thread. AutocompleteProviders are responsible for doing -// their own thread management when they need to return results asynchronously. +// their own thread management when they need to return matches asynchronously. // -// The AutocompleteProviders each return one kind of results, such as history -// results or search results. These results are given "relevance" scores. -// Historically the relevance for each column added up to 100, then scores -// were from 1-100. Both have proved a bit painful, and will be changed going -// forward. The important part is that higher relevance scores are more -// important than lower relevance scores. The relevance scores and class -// providing the result are as follows: +// The AutocompleteProviders each return different kinds of matches, such as +// history or search matches. These matches are given "relevance" scores. +// Higher scores are better matches than lower scores. The relevance scores and +// classes providing the respective matches are as follows: // // UNKNOWN input type: // --------------------------------------------------------------------|----- @@ -138,7 +135,7 @@ // keyword, then the primary provider corresponds to the default provider. // // The value column gives the ranking returned from the various providers. -// ++: a series of results with relevance from n up to (n + max_matches). +// ++: a series of matches with relevance from n up to (n + max_matches). // --: relevance score falls off over time (discounted 50 points @ 15 minutes, // 450 points @ two weeks) @@ -240,7 +237,7 @@ class AutocompleteInput { // Returns whether providers should avoid scheduling asynchronous work. If // this is true, providers should stop after returning all the - // synchronously-available results. This also means any in-progress + // synchronously-available matches. This also means any in-progress // asynchronous work should be canceled, so no later callbacks are fired. const bool synchronous_only() const { return synchronous_only_; } @@ -272,11 +269,11 @@ class AutocompleteInput { // example, a search result may say "Search for asdf" as the description, but // "asdf" should appear in the box. struct AutocompleteMatch { - // Autocomple results return strings that are classified according to a - // separate vector of styles. This vector must be sorted, and associates - // flags with portions of the strings. It is required that all text be - // inside a classification range. Even if you have no classification, you - // should create an entry at offset 0 with no flags. + // Autocomplete matches contain strings that are classified according to a + // separate vector of styles. This vector associates flags with particular + // string segments, and must be in sorted order. All text must be associated + // with some kind of classification. Even if a match has no distinct + // segments, its vector should contain an entry at offset 0 with no flags. // // Example: The user typed "goog" // http://www.google.com/ Google @@ -378,8 +375,8 @@ struct AutocompleteMatch { // enough non-negative items in all the providers to max out the popup. In // this case, the relevance of the additional items will be inverted so they // can be mixed in with the rest of the relevances. This allows a provider - // to group its results, having the added items appear intermixed with its - // other results. + // to group its matches, having the added items appear intermixed with its + // other matches. // // TODO(pkasting): http://b/1111299 This should be calculated algorithmically, // rather than being a fairly fixed value defined by the table above. @@ -403,11 +400,11 @@ struct AutocompleteMatch { // It may be empty if there is no possible navigation. GURL destination_url; - // The text displayed on the left in the search results + // The main text displayed in the address bar dropdown. std::wstring contents; ACMatchClassifications contents_class; - // Displayed to the right of the result as the title or other helper info + // Additional helper text for each entry, such as a title or description. std::wstring description; ACMatchClassifications description_class; @@ -457,7 +454,7 @@ class AutocompleteProvider // Called by a provider as a notification that something has changed. // |updated_matches| should be true iff the matches have changed in some // way (they may not have changed if, for example, the provider did an - // asynchronous query to get more results, came up with none, and is now + // asynchronous query to get more matches, came up with none, and is now // giving up). // // NOTE: Providers MUST only call this method while processing asynchronous @@ -465,7 +462,7 @@ class AutocompleteProvider // // NOTE: There's no parameter to tell the listener _which_ provider is // calling it. Because the AutocompleteController (the typical listener) - // doesn't cache the providers' individual results locally, it has to get + // doesn't cache the providers' individual matches locally, it has to get // them all again when this is called anyway, so such a parameter wouldn't // actually be useful. virtual void OnProviderUpdate(bool updated_matches) = 0; @@ -486,11 +483,11 @@ class AutocompleteProvider void SetProfile(Profile* profile); // Called to start an autocomplete query. The provider is responsible for - // tracking its results for this query and whether it is done processing the - // query. When new results are available or the provider finishes, it + // tracking its matches for this query and whether it is done processing the + // query. When new matches are available or the provider finishes, it // calls the controller's OnProviderUpdate() method. The controller can then - // get the new results using the provider's accessors. - // Exception: Results available immediately after starting the query (that + // get the new matches using the provider's accessors. + // Exception: Matches available immediately after starting the query (that // is, synchronously) do not cause any notifications to be sent. The // controller is expected to check for these without prompting (since // otherwise, starting each provider running would result in a flurry of @@ -625,11 +622,11 @@ class AutocompleteResult { // SortAndCull() has been invoked, and preserves default_match_. void AddMatch(const AutocompleteMatch& match); - // Adds a new set of matches to the set of results. Does not re-sort. + // Adds a new set of matches to the result set. Does not re-sort. void AppendMatches(const ACMatches& matches); // Removes duplicates, puts the list in sorted order and culls to leave only - // the best kMaxMatches results. Sets the default match to the best match + // the best kMaxMatches matches. Sets the default match to the best match // and updates the alternate nav URL. void SortAndCull(const AutocompleteInput& input); @@ -653,9 +650,7 @@ class AutocompleteResult { GURL alternate_nav_url() const { return alternate_nav_url_; } - // Releases the resources associated with this object. Some callers may - // want to perform several searches without creating new results each time. - // They can call this function to re-use the result for another query. + // Clears the matches for this result set. void Reset() { matches_.clear(); default_match_ = end(); @@ -691,11 +686,10 @@ class AutocompleteResult { // AutocompleteController ----------------------------------------------------- // The coordinator for autocomplete queries, responsible for combining the -// results from a series of providers into one AutocompleteResult. +// matches from a series of providers into one AutocompleteResult. class AutocompleteController : public ACProviderListener { public: - // Used to indicate an index that is not selected in a call to Update() - // and for merging results. + // Used to indicate an index that is not selected in a call to Update(). static const int kNoItemSelected; // Normally, you will call the first constructor. Unit tests can use the @@ -727,17 +721,17 @@ class AutocompleteController : public ACProviderListener { // promote a match requiring inline autocomplete too highly. // // |prefer_keyword| should be true when the keyword UI is onscreen; this will - // bias the autocomplete results toward the keyword provider when the input + // bias the autocomplete result set toward the keyword provider when the input // string is a bare keyword. // // If |synchronous_only| is true, the controller asks the providers to only - // return results which are synchronously available, which should mean that + // return matches which are synchronously available, which should mean that // all providers will be done immediately. // // The controller will fire // AUTOCOMPLETE_CONTROLLER_SYNCHRONOUS_MATCHES_AVAILABLE from inside this // call, and unless the query is stopped, will fire at least one (and perhaps - // more) AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED later as more results come in + // more) AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED later as more matches come in // (even if the query completes synchronously). Listeners should use the // result set provided in the accompanying Details object to update // themselves. @@ -774,9 +768,13 @@ class AutocompleteController : public ACProviderListener { private: // Updates |latest_result_| and |done_| 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 results. + // is true only when Start() is calling this to get the synchronous result. void UpdateLatestResult(bool is_synchronous_pass); + // Callback for when |max_delay_timer_| fires; this notes that the delay + // interval has passed and commits the result, if it's changed. + void DelayTimerFired(); + // Copies |latest_result_| to |result_| and notifies observers of updates. void CommitResult(); @@ -790,6 +788,9 @@ class AutocompleteController : public ACProviderListener { // |latest_result_| to show all history matches. void AddHistoryContentsShortcut(); + // Updates |done_| to be accurate with respect to current providers' statuses. + void CheckIfDone(); + // A list of all providers. ACProviders providers_; @@ -802,19 +803,42 @@ class AutocompleteController : public ACProviderListener { AutocompleteResult result_; // The latest result available from the autocomplete providers. This may be - // different than result_ if we've gotten results from our providers that we + // different than |result_| if we've gotten matches from our providers that we // haven't yet shown the user. If there aren't yet as many matches as in // |result|, we'll wait to display these in hopes of minimizing flicker in GUI // observers. AutocompleteResult latest_result_; + // True if |latest_result_| has been updated since it was last committed to + // |result_|. Used to determine whether we need to commit any changes. + bool updated_latest_result_; + + // True when it's been at least one interval of the delay timer since we + // committed any updates. This is used to allow a new update to be committed + // immediately. + // + // NOTE: This can never be true when |have_committed_during_this_query_| is + // false (except transiently while processing the timer firing). + bool delay_interval_has_passed_; + + // True when we've committed a result set at least once during this query. + // When this is false, we commit immediately when |done_| is set, since there + // are no more updates to come and thus no possible flicker due to committing + // immediately. + // + // NOTE: This can never be false when |delay_interval_has_passed_| is true + // (except transiently while processing the timer firing). + bool have_committed_during_this_query_; + // True if a query is not currently running. bool done_; - // Timer that tracks how long it's been since the last time we updated the - // onscreen results. This is used to ensure that observers update somewhat - // responsively even when the user types continuously. - base::RepeatingTimer<AutocompleteController> max_delay_timer_; + // Timer that tracks how long it's been since the last time we sent our + // observers a new result set. This is used both to enforce a lower bound on + // the delay between most commits (to reduce flicker), and ensure that updates + // eventually get committed no matter what delays occur between them or how + // fast or continuously the user is typing. + base::RepeatingTimer<AutocompleteController> update_delay_timer_; DISALLOW_EVIL_CONSTRUCTORS(AutocompleteController); }; |