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

Issue 15755011: InstantExtended: better handling of switching between tab and overlay. (Closed)

Created:
7 years, 7 months ago by samarth
Modified:
7 years, 6 months ago
Reviewers:
sreeram
CC:
chromium-reviews, skanuj+watch_chromium.org, melevin+watch_chromium.org, dhollowa+watch_chromium.org, dougw+watch_chromium.org, donnd+watch_chromium.org, sreeram, gideonwald, dominich, jfweitz+watch_chromium.org, David Black, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, Jered, rpetterson
Visibility:
Public.

Description

InstantExtended: better handling of switching between tab and overlay. This fixes two issues: 1) Set use_tab_for_suggestions_ at the end of OnAutocompleteStart so that it doesn't get overridden by calls to HideOverlay. 2) If not using tab for suggestions, make sure the overlay is ready to use. BUG=241835 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202966

Patch Set 1 #

Patch Set 2 : Rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -3 lines) Patch
M chrome/browser/ui/search/instant_controller.cc View 1 2 chunks +2 lines, -3 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
samarth
I'll try adding a test but I'm not too hopeful given how flaky UseOverlayIfTabNotReady has ...
7 years, 7 months ago (2013-05-23 15:10:37 UTC) #1
samarth
And I'm definitely open to ideas for making this more robust in general!
7 years, 7 months ago (2013-05-23 15:12:30 UTC) #2
sreeram
+rlp: This may fix the extension bug (http://crbug.com/239371), so you may need to do nothing ...
7 years, 7 months ago (2013-05-24 22:47:50 UTC) #3
sreeram
lgtm
7 years, 6 months ago (2013-05-28 22:31:08 UTC) #4
samarth
Thanks Sreeram. I'm going to send you a follow-up CL today with more test coverage ...
7 years, 6 months ago (2013-05-29 18:49:23 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/samarth@chromium.org/15755011/6001
7 years, 6 months ago (2013-05-29 18:49:37 UTC) #6
commit-bot: I haz the power
7 years, 6 months ago (2013-05-29 21:05:51 UTC) #7
Message was sent while issue was closed.
Change committed as 202966

Powered by Google App Engine
This is Rietveld 408576698