Port autocomplete_edit_view_browsertest.cc to Mac.

Port autocomplete_edit_view_browsertest.cc to Mac.

BUG=49740
TEST=interactive_ui_tests --gtest_filter=AutocompleteEditViewTest.* should pass on Mac.

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

[James Su, 2011-03-02 05:59:15]

Weird that BackspaceInKeywordMode, BasicTextOperations, TabMoveCursorToEnd
passed consistently on my local Mac workstation.

On 2011/03/02 02:25:33, James Su wrote:

[sky, 2011-03-02 17:05:43]

On 2011/03/02 05:59:15, James Su wrote:
> Weird that BackspaceInKeywordMode, BasicTextOperations, TabMoveCursorToEnd
> passed consistently on my local Mac workstation.
> 
> On 2011/03/02 02:25:33, James Su wrote:

The joys of browser tests. Should we wait to look until its sorted out?

  -Scott

[Scott Hess - ex-Googler, 2011-03-02 19:21:06]

LGTM, I think.

http://codereview.chromium.org/6602070/diff/1/chrome/browser/autocomplete/aut...
File chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc (right):

http://codereview.chromium.org/6602070/diff/1/chrome/browser/autocomplete/aut...
chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc:453: #endif
Is this expect even reasonable?  It seems to be making an assertion about
implementation internals for something which you shouldn't rely on anyhow. 
Likewise for some of the following, which seem to assume that keyboard
operations which aren't allowed should sometimes change the selection on some
platforms.

[James Su, 2011-03-02 22:04:42]

http://codereview.chromium.org/6602070/diff/1/chrome/browser/autocomplete/aut...
File chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc (right):

http://codereview.chromium.org/6602070/diff/1/chrome/browser/autocomplete/aut...
chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc:453: #endif
On 2011/03/02 19:21:06, shess wrote:
> Is this expect even reasonable?  It seems to be making an assertion about
> implementation internals for something which you shouldn't rely on anyhow. 
> Likewise for some of the following, which seem to assume that keyboard
> operations which aren't allowed should sometimes change the selection on some
> platforms.

Done.

[James Su, 2011-03-02 22:05:27]

Turns out that the trybot doesn't support VKEY_HOME and VKEY_END.

On 2011/03/02 17:05:43, sky wrote:
> On 2011/03/02 05:59:15, James Su wrote:
> > Weird that BackspaceInKeywordMode, BasicTextOperations, TabMoveCursorToEnd
> > passed consistently on my local Mac workstation.
> > 
> > On 2011/03/02 02:25:33, James Su wrote:
> 
> The joys of browser tests. Should we wait to look until its sorted out?
> 
>   -Scott

[sky, 2011-03-02 22:45:51]

http://codereview.chromium.org/6602070/diff/13/chrome/browser/autocomplete/au...
File chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc (right):

http://codereview.chromium.org/6602070/diff/13/chrome/browser/autocomplete/au...
chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc:180: void
SendKey(ui::KeyboardCode key,
Holy unreadable boolean parameters! You can probably blame me for a lot of this,
but we shouldn't propagate it. Could you convert this to take an int that is a
bitmask of the parameters in ui/base/events.h. Bonus points if you convert
ui_test_utils as well.

http://codereview.chromium.org/6602070/diff/13/chrome/browser/autocomplete/au...
chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc:355:
ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_1, false, false, false, true));
Isn't there some way to isolate this so we don't have so many ifdefs? Same
comment for all the #ifdefs you have in here.

[James Su, 2011-03-02 22:59:09]

http://codereview.chromium.org/6602070/diff/13/chrome/browser/autocomplete/au...
File chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc (right):

http://codereview.chromium.org/6602070/diff/13/chrome/browser/autocomplete/au...
chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc:180: void
SendKey(ui::KeyboardCode key,
On 2011/03/02 22:45:51, sky wrote:
> Holy unreadable boolean parameters! You can probably blame me for a lot of
this,
> but we shouldn't propagate it. Could you convert this to take an int that is a
> bitmask of the parameters in ui/base/events.h. Bonus points if you convert
> ui_test_utils as well.

Totally agree. But it would be a huge change if we change ui_test_utils. I'd
prefer to commit this CL as it and do this change in a separated CL.

http://codereview.chromium.org/6602070/diff/13/chrome/browser/autocomplete/au...
chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc:355:
ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_1, false, false, false, true));
On 2011/03/02 22:45:51, sky wrote:
> Isn't there some way to isolate this so we don't have so many ifdefs? Same
> comment for all the #ifdefs you have in here.
If we change ui_test_utils following your suggestion, then we could isolate this
easily by defining some platform specific constants.
Some other tests can be optimized in this way as well.

[sky, 2011-03-03 00:51:19]

http://codereview.chromium.org/6602070/diff/13/chrome/browser/autocomplete/au...
File chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc (right):

http://codereview.chromium.org/6602070/diff/13/chrome/browser/autocomplete/au...
chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc:180: void
SendKey(ui::KeyboardCode key,
On 2011/03/02 22:59:09, James Su wrote:
> On 2011/03/02 22:45:51, sky wrote:
> > Holy unreadable boolean parameters! You can probably blame me for a lot of
> this,
> > but we shouldn't propagate it. Could you convert this to take an int that is
a
> > bitmask of the parameters in ui/base/events.h. Bonus points if you convert
> > ui_test_utils as well.
> 
> Totally agree. But it would be a huge change if we change ui_test_utils. I'd
> prefer to commit this CL as it and do this change in a separated CL.

Could you at least make this class take the int for now and invoke
SendKeyPressSync with all the bools?

[James Su, 2011-03-03 02:22:37]

http://codereview.chromium.org/6602070/diff/13/chrome/browser/autocomplete/au...
File chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc (right):

http://codereview.chromium.org/6602070/diff/13/chrome/browser/autocomplete/au...
chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc:180: void
SendKey(ui::KeyboardCode key,
On 2011/03/03 00:51:19, sky wrote:
> On 2011/03/02 22:59:09, James Su wrote:
> > On 2011/03/02 22:45:51, sky wrote:
> > > Holy unreadable boolean parameters! You can probably blame me for a lot of
> > this,
> > > but we shouldn't propagate it. Could you convert this to take an int that
is
> a
> > > bitmask of the parameters in ui/base/events.h. Bonus points if you convert
> > > ui_test_utils as well.
> > 
> > Totally agree. But it would be a huge change if we change ui_test_utils. I'd
> > prefer to commit this CL as it and do this change in a separated CL.
> 
> Could you at least make this class take the int for now and invoke
> SendKeyPressSync with all the bools?

Done.

[sky, 2011-03-03 16:04:45]

LGTM

[sky, 2011-03-03 16:04:57]

And thanks! The new code is much more readable.

  -Scott