Add Windows commandline switch --enable-views-textfield.

Add Windows commandline switch --enable-views-textfield.
This switch controls use of NativeTextfield[Win|Views].

Enable and fix NativeTextfieldViewsTest on Windows.
Add switch&strings; hook up NativeThemeWin FocusableBorder colors.
Create InputMethodWin for --enable-views-textfield (or USE_AURA).
Consolidate NativeTextfieldWrapper::CreateWrapper impls.

TODO(followup): Fix 2*RequestFocus in unit test setup.
TODO(followup): Also toggle OmniboxView[Win|Views].

BUG=131660
TEST=NativeTextfieldViewsTest.*, use the flag to flip the find bar, bookmark bubble/edit, etc. (not omnibox yet)

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

[msw, 2012-06-07 20:24:01]

Hey Scott and Oshima, please take a look; thanks!
Note my two planned followup TODOs in the CL description.

[oshima, 2012-06-07 21:11:41]

switch changes LGTM with nits.

http://codereview.chromium.org/10535046/diff/4002/ui/views/controls/textfield...
File ui/views/controls/textfield/native_textfield_win.cc (right):

http://codereview.chromium.org/10535046/diff/4002/ui/views/controls/textfield...
ui/views/controls/textfield/native_textfield_win.cc:9: #include
"base/command_line.h"
do you need this?

http://codereview.chromium.org/10535046/diff/4002/ui/views/controls/textfield...
ui/views/controls/textfield/native_textfield_win.cc:26: #include
"ui/base/ui_base_switches.h"
ditto

[msw, 2012-06-07 21:43:37]

http://codereview.chromium.org/10535046/diff/4002/ui/views/controls/textfield...
File ui/views/controls/textfield/native_textfield_win.cc (right):

http://codereview.chromium.org/10535046/diff/4002/ui/views/controls/textfield...
ui/views/controls/textfield/native_textfield_win.cc:9: #include
"base/command_line.h"
On 2012/06/07 21:11:41, oshima wrote:
> do you need this?

Removed, thanks for catching that!

http://codereview.chromium.org/10535046/diff/4002/ui/views/controls/textfield...
ui/views/controls/textfield/native_textfield_win.cc:26: #include
"ui/base/ui_base_switches.h"
On 2012/06/07 21:11:41, oshima wrote:
> ditto

Done.

[sky, 2012-06-07 22:26:16]

http://codereview.chromium.org/10535046/diff/11004/ui/views/controls/textfiel...
File ui/views/controls/textfield/textfield_controller.h (right):

http://codereview.chromium.org/10535046/diff/11004/ui/views/controls/textfiel...
ui/views/controls/textfield/textfield_controller.h:57: virtual bool
IsCommandIdEnabled(int command_id) const { return false; }
It's my understanding you shouldn't do this. Only virtual methods you can inline
are void.

[msw, 2012-06-07 23:44:06]

http://codereview.chromium.org/10535046/diff/11004/ui/views/controls/textfiel...
File ui/views/controls/textfield/textfield_controller.h (right):

http://codereview.chromium.org/10535046/diff/11004/ui/views/controls/textfiel...
ui/views/controls/textfield/textfield_controller.h:57: virtual bool
IsCommandIdEnabled(int command_id) const { return false; }
On 2012/06/07 22:26:16, sky wrote:
> It's my understanding you shouldn't do this. Only virtual methods you can
inline
> are void.

You're right, it's in our style guide! Reverted this bit.

[tfarina, 2012-06-08 00:15:00]

http://codereview.chromium.org/10535046/diff/5023/ui/views/controls/textfield...
File ui/views/controls/textfield/textfield.cc (right):

http://codereview.chromium.org/10535046/diff/5023/ui/views/controls/textfield...
ui/views/controls/textfield/textfield.cc:485: if (!UseNativeTextfieldViews())
nit: what happens if this is true? Is that even possible?

[msw, 2012-06-08 00:23:48]

http://codereview.chromium.org/10535046/diff/5023/ui/views/controls/textfield...
File ui/views/controls/textfield/textfield.cc (right):

http://codereview.chromium.org/10535046/diff/5023/ui/views/controls/textfield...
ui/views/controls/textfield/textfield.cc:485: if (!UseNativeTextfieldViews())
On 2012/06/08 00:15:00, tfarina wrote:
> nit: what happens if this is true? Is that even possible?

I'm not sure I understand what you're asking. Both cases are possible and work
as I expect. If the user doesn't explicitly supply --enable-views-textfield,
we'll use the classic, well-supported NativeTextfieldWin behavior already in
production. Otherwise, we'll use the more experimental NativeTextfieldViews.

[sky, 2012-06-08 03:01:48]

LGTM

[msw, 2012-06-08 06:11:54]

I made the CL a bit more prudent in creating new InputMethodWin objects.
Now it only constructs them for --enable-views-textfield on non-Aura builds.
As before, InputMethodWin is constructed regardless of flags on Aura builds.

I plan to land with green tryjobs, please let me know if anything is amiss.

[commit-bot: I haz the power, 2012-06-08 08:21:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/10535046/6020

[commit-bot: I haz the power, 2012-06-08 09:31:43]

Try job failure for 10535046-6020 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[msw, 2012-06-08 15:45:04]

My local and "win" views_unittests build errors are confusing.

Without the allocator.gyp change, I get local errors like:
LNK2005: __malloc_dbg already defined in allocator.lib(allocator_shim.obj)
LIBCMTD.lib

Local and "win_rel" build with the allocator.gyp change.
But the "win" bot has errors with the same change like:
msvcprtd.lib(MSVCP90D.dll) :error LNK2005: "public: __thiscall
std::basic_string<char,struct std::char_traits<char>,class std::allocator<char>
>::~basic_string<char,struct std::char_traits<char>,class std::allocator<char>
>(void)"
(??1?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@QAE@XZ) already
defined in allocator.lib(allocator_shim.obj)

I sent "win" & "win_rel" try jobs without the allocator.gyp change. I'll presume
it's a local issue and will leave allocator.gyp out of the change if these try
jobs pass. My local build however is definitely hosed without it; advice is
appreciated!

[commit-bot: I haz the power, 2012-06-08 17:15:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/10535046/6021

[commit-bot: I haz the power, 2012-06-08 18:22:23]

Failed to apply patch for ui/base/ui_base_switches.h:
While running patch -p1 --forward --force;
patching file ui/base/ui_base_switches.h
Hunk #1 FAILED at 17.
1 out of 1 hunk FAILED -- saving rejects to file ui/base/ui_base_switches.h.rej

[commit-bot: I haz the power, 2012-06-08 19:20:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/10535046/25

[commit-bot: I haz the power, 2012-06-08 20:25:47]

Change committed as 141272