Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(231)

Issue 477293002: Revert 290058 "Omnibox: Prevent Asynchronous Suggestions from Ch..." (Closed)

Created:
6 years, 4 months ago by miu
Modified:
6 years, 4 months ago
Reviewers:
Mark P
CC:
chromium-reviews
Visibility:
Public.

Description

Revert 290058 "Omnibox: Prevent Asynchronous Suggestions from Ch..." This change closed the build tree for Linux and ChromiumOS bots due to failing interactive_ui_tests: http://build.chromium.org/p/chromium.linux/buildstatus?builder=Linux%20Tests&number=12979 > Omnibox: Prevent Asynchronous Suggestions from Changing Default Match > > Calls to the suggest server may normally result in a new inline > autocompletion. This can be disruptive because it means pressing enter > may bring the user to different places depending on how long he/she > waits after typing the last key. > > This change prevents new suggestions from becoming the default match. > In other words, the default match is only allowed to change on a > keystroke, not due to a reply coming back from the server. > > The consequence of this change is that if previously we'd show an > inline suggestion on a server reply, now we only show it one keystroke > later. I think this trade-off (one keystroke versus inconsistent > omnibox behavior) is a good one to make. > > We still end up with default matches (inline autocompletions within > the omnibox) from the suggest server after this change. Here's an > example of why: > > User types "facebo" > We send a suggest server request. > Server asynchronously returns "facebook" as a top suggestion, > beating the server-provided verbatim score for "facebo". > We decide not to show it within the omnibox. It's instead shown > somewhere in the dropdown. > User types "o". > We send a suggest server request. > We reuse our cached suggestions and suggestion scores. <<< THE KEY > We show "facebook" as an inline suggestion because it beats > the default verbatim score that gets assigned to "faceboo". > (This is the score that we assign by default without having > yet received the most recent suggest server response.) > We receive the response, which includes "facebook" as a top > suggestion, beating the server-provided verbatim score > for "faceboo". > We show "facebook" as an inline suggestion. i.e., we decide > not to demote it because it was already being shown inline > > TESTED: > unit tests plus interactive tests (facebook.com/l, google.com/a) > > BUG=398135 > R=msw@chromium.org > > Review URL: https://codereview.chromium.org/471673002 TBR=mpearson@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290070

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+252 lines, -528 lines) Patch
M trunk/src/chrome/browser/autocomplete/base_search_provider.cc View 2 chunks +7 lines, -13 lines 0 comments Download
M trunk/src/chrome/browser/autocomplete/search_provider.h View 4 chunks +13 lines, -22 lines 0 comments Download
M trunk/src/chrome/browser/autocomplete/search_provider.cc View 11 chunks +26 lines, -111 lines 0 comments Download
M trunk/src/chrome/browser/autocomplete/search_provider_unittest.cc View 39 chunks +206 lines, -369 lines 0 comments Download
M trunk/src/components/omnibox/search_suggestion_parser.h View 2 chunks +0 lines, -12 lines 0 comments Download
M trunk/src/components/omnibox/search_suggestion_parser.cc View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
miu
6 years, 4 months ago (2014-08-16 00:55:26 UTC) #1
miu
Committed patchset #1 manually as r290070 (tree was closed).
6 years, 4 months ago (2014-08-16 00:55:55 UTC) #2
miu
6 years, 4 months ago (2014-08-16 00:56:50 UTC) #3
Message was sent while issue was closed.
Please use the CQ!

Powered by Google App Engine
This is Rietveld 408576698