[Mac] Makes ctrl-return follow links when finding in page....

[Mac] Makes ctrl-return follow links when finding in page.

Enables the FindInPageControllerTest browser test on Mac.

BUG=38365, 37808
TEST=Do a find in page for text in a link.  Pressing ctrl-return while the findbar has focus should follow the link.

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

[rohitrao (ping after 24h), 2010-03-19 17:38:43]

I gave up on the FindBarTesting::DisableFindBarAnimations() idea because SetUp()
for a browsertest is called before the Browser object is ever created, so I
would've had to call DisableFindBarAnimations() at the start of every test,
which seemed awful.  Next best idea was to refactor find_bar_bridge.h to have no
objc, so we can include it directly.

[Evan Stade, 2010-03-19 21:03:07]

can you get someone else for the mac refactor? sorry, I'm not familiar with the
objc binding stuff.

http://codereview.chromium.org/1061003/diff/14001/15001
File chrome/browser/find_bar_host_browsertest.cc (right):

http://codereview.chromium.org/1061003/diff/14001/15001#newcode566
chrome/browser/find_bar_host_browsertest.cc:566: // TODO(rohitrao): Enable on
mac.  http://crbug.com/22036
use the MAYBE_ thing if the test doesn't pass; only ifdef it out if the test
wouldn't compile.

[rohitrao (ping after 24h), 2010-03-22 15:22:37]

+shess for the cocoa changes, since this code looks suspiciously like the
autocomplete edit view's doCommandBySelector.

http://codereview.chromium.org/1061003/diff/14001/15001
File chrome/browser/find_bar_host_browsertest.cc (right):

http://codereview.chromium.org/1061003/diff/14001/15001#newcode566
chrome/browser/find_bar_host_browsertest.cc:566: // TODO(rohitrao): Enable on
mac.  http://crbug.com/22036
On 2010/03/19 21:03:07, Evan Stade wrote:
> use the MAYBE_ thing if the test doesn't pass; only ifdef it out if the test
> wouldn't compile.

Done.

[Scott Hess - ex-Googler, 2010-03-22 16:07:59]

LGTM, w/minor style nits.

On Fri, Mar 19, 2010 at 10:38 AM,  <rohitrao@chromium.org> wrote:
> I gave up on the FindBarTesting::DisableFindBarAnimations() idea because
> SetUp()
> for a browsertest is called before the Browser object is ever created, so I
> would've had to call DisableFindBarAnimations() at the start of every test,
> which seemed awful.

Could you override CreateBrowser()?

http://codereview.chromium.org/1061003/diff/26001/27003
File chrome/browser/cocoa/find_bar_bridge.mm (right):

http://codereview.chromium.org/1061003/diff/26001/27003#newcode25
chrome/browser/cocoa/find_bar_bridge.mm:25: [cocoa_controller_
showFindBar:really_animate];
Keep the explicit bool->BOOL conversion in both spots.

http://codereview.chromium.org/1061003/diff/26001/27003#newcode79
chrome/browser/cocoa/find_bar_bridge.mm:79: bool window_visible = window &&
[window isVisible] ? true : false;
Given nil handling, [window isVisible] suffices.

[rohitrao (ping after 24h), 2010-03-22 16:43:27]

I thought about overriding CreateBrowser().  It just felt like a misuse of what
that function is meant for, although I guess a couple other tests are using it
that way.

http://codereview.chromium.org/1061003/diff/26001/27003
File chrome/browser/cocoa/find_bar_bridge.mm (right):

http://codereview.chromium.org/1061003/diff/26001/27003#newcode25
chrome/browser/cocoa/find_bar_bridge.mm:25: [cocoa_controller_
showFindBar:really_animate];
On 2010/03/22 16:07:59, shess wrote:
> Keep the explicit bool->BOOL conversion in both spots.

Done.

http://codereview.chromium.org/1061003/diff/26001/27003#newcode79
chrome/browser/cocoa/find_bar_bridge.mm:79: bool window_visible = window &&
[window isVisible] ? true : false;
On 2010/03/22 16:07:59, shess wrote:
> Given nil handling, [window isVisible] suffices.

Done.

[Scott Hess - ex-Googler, 2010-03-22 16:46:38]

still lgtm and all.

On Mon, Mar 22, 2010 at 9:43 AM,  <rohitrao@chromium.org> wrote:
> I thought about overriding CreateBrowser().  It just felt like a misuse of
> what
> that function is meant for, although I guess a couple other tests are using
> it
> that way.
>
>
> http://codereview.chromium.org/1061003/diff/26001/27003
> File chrome/browser/cocoa/find_bar_bridge.mm (right):
>
> http://codereview.chromium.org/1061003/diff/26001/27003#newcode25
> chrome/browser/cocoa/find_bar_bridge.mm:25: [cocoa_controller_
> showFindBar:really_animate];
> On 2010/03/22 16:07:59, shess wrote:
>>
>> Keep the explicit bool->BOOL conversion in both spots.
>
> Done.
>
> http://codereview.chromium.org/1061003/diff/26001/27003#newcode79
> chrome/browser/cocoa/find_bar_bridge.mm:79: bool window_visible = window
> && [window isVisible] ? true : false;
> On 2010/03/22 16:07:59, shess wrote:
>>
>> Given nil handling, [window isVisible] suffices.
>
> Done.
>
> http://codereview.chromium.org/1061003
>