Quick fix for NativeTextfieldViewsTest on win aura.

Quick fix for NativeTextfieldViewsTest on win aura.
Initialize KeyEvent::character_ for synthesized events.
Mark linux FAILS_/DISABLED_ (crbug.com/99128 & crbug.com/97845).

The key_code/[unmodified_]/character code seems overly complex / platform-dependent.
Hopefully yusukes or I can simplify and consolidate it a bit later.

BUG=99128
TEST=NativeTextfieldViewsTest.* pass on win aura, correct expectations on linux [aura].

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

[msw, 2011-10-05 20:12:53]

PTAL, thanks! (Trybots are re-running on a later revision.)

[oshima, 2011-10-05 20:15:09]

http://codereview.chromium.org/8150001/diff/1/views/controls/textfield/native...
File views/controls/textfield/native_textfield_views_unittest.cc (right):

http://codereview.chromium.org/8150001/diff/1/views/controls/textfield/native...
views/controls/textfield/native_textfield_views_unittest.cc:982: EXPECT_STR_EQ("
one two three ", str);
Did this work? It failed on linux because clipboard implmentation is empty for
aura. I wonder if aura/windows build is using window's clipboard.

http://codereview.chromium.org/8150001/diff/1/views/events/event_aura.cc
File views/events/event_aura.cc (right):

http://codereview.chromium.org/8150001/diff/1/views/events/event_aura.cc#newc...
views/events/event_aura.cc:31: return character_;
windows key event has the following code.

  return (native_event().message == WM_CHAR) ? key_code_ :
      GetCharacterFromKeyCode(key_code_, flags());

this isn't necessary any more (on win)?

[msw, 2011-10-05 20:45:34]

http://codereview.chromium.org/8150001/diff/1/views/controls/textfield/native...
File views/controls/textfield/native_textfield_views_unittest.cc (right):

http://codereview.chromium.org/8150001/diff/1/views/controls/textfield/native...
views/controls/textfield/native_textfield_views_unittest.cc:982: EXPECT_STR_EQ("
one two three ", str);
On 2011/10/05 20:15:09, oshima wrote:
> Did this work? It failed on linux because clipboard implmentation is empty for
> aura. I wonder if aura/windows build is using window's clipboard.

Yes, it does work, via clipboard_win's ui::Clipboard. It seems like it should be
using clipboard_aura and failing. Recommendations?

http://codereview.chromium.org/8150001/diff/1/views/events/event_aura.cc
File views/events/event_aura.cc (right):

http://codereview.chromium.org/8150001/diff/1/views/events/event_aura.cc#newc...
views/events/event_aura.cc:31: return character_;
On 2011/10/05 20:15:09, oshima wrote:
> windows key event has the following code.
> 
>   return (native_event().message == WM_CHAR) ? key_code_ :
>       GetCharacterFromKeyCode(key_code_, flags());
> 
> this isn't necessary any more (on win)?

I'm not sure; I think the key_code/[unmodified_]/character code should be
revisited in a followup change. I'm not familiar with Yusuke's work, but it
appears a bit overly complex and platform-dependent... Should I investigate that
work as part of this CL?

[oshima, 2011-10-05 21:02:19]

LGTM

http://codereview.chromium.org/8150001/diff/1/views/controls/textfield/native...
File views/controls/textfield/native_textfield_views_unittest.cc (left):

http://codereview.chromium.org/8150001/diff/1/views/controls/textfield/native...
views/controls/textfield/native_textfield_views_unittest.cc:37: // Bug 99128.
Can you keep the comment and say still a lot of tests fails on linux? You can
assign it to me once you fixed win side.

http://codereview.chromium.org/8150001/diff/1/views/controls/textfield/native...
File views/controls/textfield/native_textfield_views_unittest.cc (right):

http://codereview.chromium.org/8150001/diff/1/views/controls/textfield/native...
views/controls/textfield/native_textfield_views_unittest.cc:982: EXPECT_STR_EQ("
one two three ", str);
On 2011/10/05 20:45:34, msw wrote:
> On 2011/10/05 20:15:09, oshima wrote:
> > Did this work? It failed on linux because clipboard implmentation is empty
for
> > aura. I wonder if aura/windows build is using window's clipboard.
> 
> Yes, it does work, via clipboard_win's ui::Clipboard. It seems like it should
be
> using clipboard_aura and failing. Recommendations?

I see. I think we added aura because linux impl uses gtk.
I think win build can keep using window's clipboard. I'm okay as it is now.

[oshima, 2011-10-05 21:03:23]

http://codereview.chromium.org/8150001/diff/1/views/events/event_aura.cc
File views/events/event_aura.cc (right):

http://codereview.chromium.org/8150001/diff/1/views/events/event_aura.cc#newc...
views/events/event_aura.cc:31: return character_;
On 2011/10/05 20:45:34, msw wrote:
> On 2011/10/05 20:15:09, oshima wrote:
> > windows key event has the following code.
> > 
> >   return (native_event().message == WM_CHAR) ? key_code_ :
> >       GetCharacterFromKeyCode(key_code_, flags());
> > 
> > this isn't necessary any more (on win)?
> 
> I'm not sure; I think the key_code/[unmodified_]/character code should be
> revisited in a followup change. I'm not familiar with Yusuke's work, but it
> appears a bit overly complex and platform-dependent... Should I investigate
that
> work as part of this CL?

No, if this works on the test, that should be enough for now. can you add him to
the bug?

[sadrul, 2011-10-05 21:44:56]

http://codereview.chromium.org/8150001/diff/1/views/controls/textfield/native...
File views/controls/textfield/native_textfield_views_unittest.cc (left):

http://codereview.chromium.org/8150001/diff/1/views/controls/textfield/native...
views/controls/textfield/native_textfield_views_unittest.cc:37: // Bug 99128.
FYI http://codereview.chromium.org/8143021/

Some of the tests also need to be marked DISABLED_ on linux.

[msw, 2011-10-05 22:52:19]

Please double check my FAILS_ / DISABLED_ logic after merging with changes from
http://codereview.chromium.org/8143021.

http://codereview.chromium.org/8150001/diff/1/views/controls/textfield/native...
File views/controls/textfield/native_textfield_views_unittest.cc (left):

http://codereview.chromium.org/8150001/diff/1/views/controls/textfield/native...
views/controls/textfield/native_textfield_views_unittest.cc:37: // Bug 99128.
On 2011/10/05 21:44:56, sadrul wrote:
> FYI http://codereview.chromium.org/8143021/
> 
> Some of the tests also need to be marked DISABLED_ on linux.

I merged with your changes from http://codereview.chromium.org/8143021.

http://codereview.chromium.org/8150001/diff/1/views/events/event_aura.cc
File views/events/event_aura.cc (right):

http://codereview.chromium.org/8150001/diff/1/views/events/event_aura.cc#newc...
views/events/event_aura.cc:31: return character_;
On 2011/10/05 21:03:24, oshima wrote:
> On 2011/10/05 20:45:34, msw wrote:
> > On 2011/10/05 20:15:09, oshima wrote:
> > > windows key event has the following code.
> > > 
> > >   return (native_event().message == WM_CHAR) ? key_code_ :
> > >       GetCharacterFromKeyCode(key_code_, flags());
> > > 
> > > this isn't necessary any more (on win)?
> > 
> > I'm not sure; I think the key_code/[unmodified_]/character code should be
> > revisited in a followup change. I'm not familiar with Yusuke's work, but it
> > appears a bit overly complex and platform-dependent... Should I investigate
> that
> > work as part of this CL?
> 
> No, if this works on the test, that should be enough for now. can you add him
to
> the bug?

Added Yusuke to the bug; he's also cc'ed on this CR.

[sadrul, 2011-10-05 22:54:02]

http://codereview.chromium.org/8150001/diff/8007/views/controls/textfield/nat...
File views/controls/textfield/native_textfield_views_unittest.cc (right):

http://codereview.chromium.org/8150001/diff/8007/views/controls/textfield/nat...
views/controls/textfield/native_textfield_views_unittest.cc:38: #if
defined(OS_WIN)
I think the simplest would be to make this

#if defined(USE_AURA) && defined(OS_LINUX)

Keep the FAILS_ bit, turn some into DISABLED_, and remove the #else block.

[sadrul, 2011-10-05 22:57:17]

http://codereview.chromium.org/8150001/diff/8007/views/controls/textfield/nat...
File views/controls/textfield/native_textfield_views_unittest.cc (right):

http://codereview.chromium.org/8150001/diff/8007/views/controls/textfield/nat...
views/controls/textfield/native_textfield_views_unittest.cc:38: #if
defined(OS_WIN)
On 2011/10/05 22:54:02, sadrul wrote:
> I think the simplest would be to make this
> 
> #if defined(USE_AURA) && defined(OS_LINUX)
> 
> Keep the FAILS_ bit, turn some into DISABLED_, and remove the #else block.

Clearly the #else block should remain as it is.

[msw, 2011-10-05 23:13:28]

Okay, sorry for my confusion; this should be correct.

[sadrul, 2011-10-05 23:13:55]

Yep. This LGTM

[oshima, 2011-10-06 18:07:03]

On 2011/10/05 23:13:55, sadrul wrote:
> Yep. This LGTM

Mike, when can you check in? I have a fix that depends on this
(http://codereview.chromium.org/8165016).

[sky, 2011-10-06 19:40:06]

LGTM

[msw, 2011-10-06 19:51:47]

On 2011/10/06 19:40:06, sky wrote:
> LGTM

Sorry for the delay, landing now.