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

Issue 348015: Fix and re-enable a test I broke with the find bar changes I made last night.... (Closed)

Created:
11 years, 1 month ago by Ben Goodger (Google)
Modified:
9 years, 6 months ago
CC:
chromium-reviews_googlegroups.com, John Grabowski, pam+watch_chromium.org, ben+cc_chromium.org
Visibility:
Public.

Description

Fix and re-enable a test I broke with the find bar changes I made last night. Remove Browser::find_bar(), make everyone use either Browser::GetFindBarController() which creates the FindBarController on demand, or a new method HasFindBarController(), which can be used to check for its existence (so as to avoid creating it unnecessarily). http://crbug.com/26231 TEST=interactive ui tests, standard find tests Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30507

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -16 lines) Patch
M chrome/browser/browser.h View 1 1 chunk +7 lines, -2 lines 0 comments Download
M chrome/browser/browser.cc View 1 2 2 chunks +5 lines, -1 line 0 comments Download
M chrome/browser/cocoa/tab_strip_controller.mm View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/gtk/browser_window_gtk.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/views/find_bar_host_browsertest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/views/find_bar_host_interactive_uitest.cc View 4 chunks +7 lines, -6 lines 1 comment Download
M chrome/browser/views/frame/browser_view.cc View 1 chunk +4 lines, -3 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Ben Goodger (Google)
11 years, 1 month ago (2009-10-29 20:11:27 UTC) #1
Finnur
LGTM
11 years, 1 month ago (2009-10-29 20:24:06 UTC) #2
Paweł Hajdan Jr.
Drive-by. http://codereview.chromium.org/348015/diff/3009/17 File chrome/browser/views/find_bar_host_interactive_uitest.cc (right): http://codereview.chromium.org/348015/diff/3009/17#newcode113 Line 113: // TODO: http://crbug.com/26231 This comment is probably ...
11 years, 1 month ago (2009-10-29 20:45:32 UTC) #3
Ben Goodger (Google)
11 years, 1 month ago (2009-10-29 21:02:14 UTC) #4
Thanks! Removed.

On Thu, Oct 29, 2009 at 1:45 PM, <phajdan.jr@chromium.org> wrote:

> Drive-by.
>
>
> http://codereview.chromium.org/348015/diff/3009/17
> File chrome/browser/views/find_bar_host_interactive_uitest.cc (right):
>
> http://codereview.chromium.org/348015/diff/3009/17#newcode113
> Line 113: // TODO: http://crbug.com/26231
> This comment is probably outdated.
>
>
> http://codereview.chromium.org/348015
>

Powered by Google App Engine
This is Rietveld 408576698