diff options
author | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-07-22 21:22:49 +0000 |
---|---|---|
committer | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-07-22 21:22:49 +0000 |
commit | a748e37aaa8f9b11b903df75e698b9a1fc300676 (patch) | |
tree | d5e88cb03f76b248921953e76846424398486190 /chrome/browser/autocomplete/autocomplete.h | |
parent | c1e432a2610e72bdbd19b10b769ad6ca9ebfb2c6 (diff) | |
download | chromium_src-a748e37aaa8f9b11b903df75e698b9a1fc300676.zip chromium_src-a748e37aaa8f9b11b903df75e698b9a1fc300676.tar.gz chromium_src-a748e37aaa8f9b11b903df75e698b9a1fc300676.tar.bz2 |
Try and improve result coalescing/display in the omnibox popup.
This eliminates the delay timer in the popup view, and also eliminates the coalesce timer in the controller. Instead, we simply coalesce until we're done, we have at least as many results as we're already showing, or the "maximum delay timeout" (300 ms) fires, indicating we've gone too long without updating.
Additionally, in order to be more responsive when typing rapidly, the controller updates observers immediately with the available results from a previous query if one is still running when a new query is started. While in theory this seems like it might produce flicker, in practice _not_ having it also results in flicker (just less-predictable flicker) since the 300 ms timeout starts kicking in at random times relative to when new keys are pressed.
I also fixed a few small problems with leaving 1-pixel high white rows at the bottom of the popup during rapid typing (which weren't visible before this change since the popup would never shrink during rapid typing).
After eliminating the timeout in the popup view, I was able to refactor the code to be shorter since a few members and helper functions could all be inlined. Then I added some long comments and made things not much shorter after all :/. I also changed two other (self-contained) unrelated spots in the popup to be shorter.
Please patch this in locally and try how it feels. Things to test with this change vs. the old code vs. the old, old (original omnibox) code:
* Type one letter at a time with long pauses in between; see how flickery the popup is
* Type one letter (e.g. "a") and then type rapidly for a while; see how responsive the popup is
* Type words like "amazon", "compusa" and "comcast" at various different speeds and observe the flicker vs. responsiveness tradeoff
* Type or paste some long series of letters (that default to searching), then rapidly press and release the ctrl key
My hope is that this hits a good balance (it's very difficult to be both flicker-free and responsive, I view the previous two sets of code as being off first one side of the scale and then the other). Possible tweaks include the animation tweening mechanism and timing (I experimented with various different speeds and linear tweening, nothing felt significantly better to me but my machine sucks w.r.t. animation quality) and tweaking the controller "max timeout" value and notification behavior upon starting a new query (I tried notifying only if two keys had been typed since the last notification, it didn't feel better).
BUG=none
TEST=see above
Review URL: http://codereview.chromium.org/149659
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@21322 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/autocomplete/autocomplete.h')
-rw-r--r-- | chrome/browser/autocomplete/autocomplete.h | 16 |
1 files changed, 3 insertions, 13 deletions
diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h index 9d566fc..09ecf8f 100644 --- a/chrome/browser/autocomplete/autocomplete.h +++ b/chrome/browser/autocomplete/autocomplete.h @@ -707,7 +707,6 @@ class AutocompleteController : public ACProviderListener { explicit AutocompleteController(const ACProviders& providers) : providers_(providers), history_contents_provider_(NULL), - update_pending_(false), done_(true) { } #endif @@ -804,23 +803,14 @@ class AutocompleteController : public ACProviderListener { // The latest result available from the autocomplete providers. This may be // different than result_ if we've gotten results from our providers that we - // haven't yet shown the user. If more matches may be coming, we'll wait to - // display these in hopes of minimizing flicker in GUI observers; see - // |coalesce_timer_|. + // 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 when there are newer results in |latest_result_| than in |result_| and - // observers have not been notified about them. - bool update_pending_; - // True if a query is not currently running. bool done_; - // Timer that tracks how long it's been since the last provider update we - // received. Instead of notifying about each update immediately, we batch - // updates into groups. - base::OneShotTimer<AutocompleteController> coalesce_timer_; - // 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. |