Fix focus restore bug in find-in-bar

The focus would only be restored properly the first time the
find-in-bar box was closed.
Also wrote an interactive ui test and took the opportunity to convert the existing find-in-bar interactive ui test to be a browser test.

BUG=23599
TEST=See bug.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=27875

[Finnur, 2009-10-02 03:55:23]

Make the changes necessary to get Mac and Linux compiling and I'll take a better
look. 

On 2009/10/02 00:22:11, jcampan wrote:
>

[Finnur, 2009-10-02 04:28:40]

Also, a few comments. Mostly just nits.

http://codereview.chromium.org/251064/diff/1/4
File chrome/browser/views/find_bar_host_interactive_uitest.cc (right):

http://codereview.chromium.org/251064/diff/1/4#newcode35
Line 35: void ClickOnView(ViewID vid) {
nit: view_id, not vid

http://codereview.chromium.org/251064/diff/1/4#newcode55
Line 55: #if defined(TOOLKIT_VIEWS) || defined(OS_WIN)
I forget how TOOLKIT_VIEWS is defined... but if you need to OR it with OS_WIN,
then how does |view| get defined on OS_WIN in the function above this one?

http://codereview.chromium.org/251064/diff/1/4#newcode77
Line 77: NULL);
nit: I liked the other way of indenting this... :)

http://codereview.chromium.org/251064/diff/1/4#newcode115
Line 115: NULL);
ditto.

http://codereview.chromium.org/251064/diff/1/4#newcode130
Line 130: // Focus the location bar, perform a find, close the find in page,
focus
nit: Focus the location bar, [find something on the page], close the find [box
and] focus...

http://codereview.chromium.org/251064/diff/1/4#newcode133
Line 133: 
nit: Delete this empty line

http://codereview.chromium.org/251064/diff/1/4#newcode142
Line 142: // return to the location bar. (same as before, just checking that
nit: remove the period.
nit: find-in-page -> find box

http://codereview.chromium.org/251064/diff/1/2
File chrome/test/ui_test_utils.h (right):

http://codereview.chromium.org/251064/diff/1/2#newcode117
Line 117: // |ordinal| is an optional parameter that is set to the index of the
current
nit: move this up to the line above.
nit: that -> which, if specified, is set ...

[jcampan, 2009-10-02 16:39:18]

http://codereview.chromium.org/251064/diff/1/4
File chrome/browser/views/find_bar_host_interactive_uitest.cc (right):

http://codereview.chromium.org/251064/diff/1/4#newcode35
Line 35: void ClickOnView(ViewID vid) {
On 2009/10/02 04:28:40, Finnur wrote:
> nit: view_id, not vid

Done.

http://codereview.chromium.org/251064/diff/1/4#newcode55
Line 55: #if defined(TOOLKIT_VIEWS) || defined(OS_WIN)
On 2009/10/02 04:28:40, Finnur wrote:
> I forget how TOOLKIT_VIEWS is defined... but if you need to OR it with OS_WIN,
> then how does |view| get defined on OS_WIN in the function above this one?

It does not really matter as this file is only built on Windows at this point.
We'll have to worry about this when we port this file to Linux.

http://codereview.chromium.org/251064/diff/1/4#newcode77
Line 77: NULL);
On 2009/10/02 04:28:40, Finnur wrote:
> nit: I liked the other way of indenting this... :)

Done.

http://codereview.chromium.org/251064/diff/1/4#newcode115
Line 115: NULL);
On 2009/10/02 04:28:40, Finnur wrote:
> ditto.

Done.

http://codereview.chromium.org/251064/diff/1/4#newcode130
Line 130: // Focus the location bar, perform a find, close the find in page,
focus
On 2009/10/02 04:28:40, Finnur wrote:
> nit: Focus the location bar, [find something on the page], close the find [box
> and] focus...

Done.

http://codereview.chromium.org/251064/diff/1/4#newcode133
Line 133: 
On 2009/10/02 04:28:40, Finnur wrote:
> nit: Delete this empty line

Done.

http://codereview.chromium.org/251064/diff/1/4#newcode142
Line 142: // return to the location bar. (same as before, just checking that
On 2009/10/02 04:28:40, Finnur wrote:
> nit: remove the period.
> nit: find-in-page -> find box

Done.

[Finnur, 2009-10-02 17:35:16]

LG, one nit.

http://codereview.chromium.org/251064/diff/1/4
File chrome/browser/views/find_bar_host_interactive_uitest.cc (right):

http://codereview.chromium.org/251064/diff/1/4#newcode55
Line 55: #if defined(TOOLKIT_VIEWS) || defined(OS_WIN)
Just remove OS_WIN, I guess. This file is built only on Win anyway.

On 2009/10/02 16:39:18, jcampan wrote:
> On 2009/10/02 04:28:40, Finnur wrote:
> > I forget how TOOLKIT_VIEWS is defined... but if you need to OR it with
OS_WIN,
> > then how does |view| get defined on OS_WIN in the function above this one?
> 
> It does not really matter as this file is only built on Windows at this point.
> We'll have to worry about this when we port this file to Linux.