Change IME conversion status by focusing input element

Change IME conversion status by focusing input element

This CL affects Windows except 8.
 example: When user input focuses <input inputmode="kana" />, IME changes its mode to Hiragana input mode if the IME has.
inputmode attribute is specified in HTML5 draft:
 http://www.w3.org/html/wg/drafts/html/master/single-page.html#input-modalities:-the-inputmode-attribute

TEST=manual test using MS-IME and GoogleJapaneseInput

BUG=244688
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216494

[yoichio, 2013-08-01 05:19:26]

Could you review?

[yoichio, 2013-08-01 05:30:49]

Could you review?

[jochen (gone - plz use gerrit), 2013-08-01 06:43:48]

can you write a test for that?

[yoichio, 2013-08-01 08:08:28]

On 2013/08/01 06:43:48, jochen wrote:
> can you write a test for that?

Do you mean that call TextInputTypeChanged and confirm that IME state is
changed?
It's difficult because IME is a kind of system service and Windows in buildbot
has no 
such a IME mocks.

or Just Imm32Manager mock test?

[jochen (gone - plz use gerrit), 2013-08-01 20:40:28]

On 2013/08/01 08:08:28, yoichio wrote:
> On 2013/08/01 06:43:48, jochen wrote:
> > can you write a test for that?
> 
> Do you mean that call TextInputTypeChanged and confirm that IME state is
> changed?
> It's difficult because IME is a kind of system service and Windows in buildbot
> has no 
> such a IME mocks.
> 
> or Just Imm32Manager mock test?

yes, if it's possible to mock imm32manager and write a unit test, that would be
great.

[jochen (gone - plz use gerrit), 2013-08-07 02:25:23]

lgtm, thank you

https://codereview.chromium.org/21189003/diff/13001/ui/base/ime/win/imm32_man...
File ui/base/ime/win/imm32_manager.h (right):

https://codereview.chromium.org/21189003/diff/13001/ui/base/ime/win/imm32_man...
ui/base/ime/win/imm32_manager.h:83: virtual bool is_composing() const { return
is_composing_; }
i think it's enough to make the methods virtual which you're actually overriding
in the test.

[Yohei Yukawa, 2013-08-07 02:42:14]

https://codereview.chromium.org/21189003/diff/13001/content/browser/renderer_...
File content/browser/renderer_host/render_widget_host_view_win_browsertest.cc
(right):

https://codereview.chromium.org/21189003/diff/13001/content/browser/renderer_...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:40:
HWND window_handle_;
please initialize these fields in the constructor.

https://codereview.chromium.org/21189003/diff/13001/content/browser/renderer_...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:40:
HWND window_handle_;
please put private:

https://codereview.chromium.org/21189003/diff/13001/content/browser/renderer_...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:43: };
Can we put this?
  DISALLOW_COPY_AND_ASSIGN(MockIMM32Manager);

https://codereview.chromium.org/21189003/diff/13001/content/browser/renderer_...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:57:
shell()->web_contents()->GetRenderViewHost()->GetView()));
Please make sure this indent is correct in the style guide.

[yoichio, 2013-08-07 03:33:55]

https://chromiumcodereview.appspot.com/21189003/diff/13001/content/browser/re...
File content/browser/renderer_host/render_widget_host_view_win_browsertest.cc
(right):

https://chromiumcodereview.appspot.com/21189003/diff/13001/content/browser/re...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:40:
HWND window_handle_;
On 2013/08/07 02:42:14, Yohei Yukawa wrote:
> please initialize these fields in the constructor.

Done.

https://chromiumcodereview.appspot.com/21189003/diff/13001/content/browser/re...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:43: };
On 2013/08/07 02:42:14, Yohei Yukawa wrote:
> Can we put this?
>   DISALLOW_COPY_AND_ASSIGN(MockIMM32Manager);

Done.

https://chromiumcodereview.appspot.com/21189003/diff/13001/content/browser/re...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:57:
shell()->web_contents()->GetRenderViewHost()->GetView()));
On 2013/08/07 02:42:14, Yohei Yukawa wrote:
> Please make sure this indent is correct in the style guide.

Done.

https://chromiumcodereview.appspot.com/21189003/diff/13001/ui/base/ime/win/im...
File ui/base/ime/win/imm32_manager.h (right):

https://chromiumcodereview.appspot.com/21189003/diff/13001/ui/base/ime/win/im...
ui/base/ime/win/imm32_manager.h:83: virtual bool is_composing() const { return
is_composing_; }
On 2013/08/07 02:25:23, jochen wrote:
> i think it's enough to make the methods virtual which you're actually
overriding
> in the test.

Done.

[Yohei Yukawa, 2013-08-07 04:17:18]

https://chromiumcodereview.appspot.com/21189003/diff/22001/content/browser/re...
File content/browser/renderer_host/render_widget_host_view_win.h (right):

https://chromiumcodereview.appspot.com/21189003/diff/22001/content/browser/re...
content/browser/renderer_host/render_widget_host_view_win.h:541:
ui::TextInputMode text_input_mode_;
Please move this field next to |text_input_type_| so that
one can easily find we have two different fields that represent
input field attributes.

https://chromiumcodereview.appspot.com/21189003/diff/22001/content/browser/re...
File content/browser/renderer_host/render_widget_host_view_win_browsertest.cc
(right):

https://chromiumcodereview.appspot.com/21189003/diff/22001/content/browser/re...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:90:
view_->TextInputTypeChanged(ui::TEXT_INPUT_TYPE_NONE, false,
(optional) In future CL, please consider to reorder the parameter as
  (ui::TEXT_INPUT_TYPE_NONE, ui::TEXT_INPUT_MODE_EMAIL, false)

https://chromiumcodereview.appspot.com/21189003/diff/22001/ui/base/ime/win/im...
File ui/base/ime/win/imm32_manager.h (right):

https://chromiumcodereview.appspot.com/21189003/diff/22001/ui/base/ime/win/im...
ui/base/ime/win/imm32_manager.h:80: ~IMM32Manager();
Should be as follows.
  virtual ~IMM32Manager();

[yoichio, 2013-08-07 09:33:39]

+nona@ as original author of test file.
I change render_widget_host_view_win_browsertest.cc build environment from
win/aura to win only.
This doesn't change existing test coverage because the tests have been disabled.

https://chromiumcodereview.appspot.com/21189003/diff/22001/content/browser/re...
File content/browser/renderer_host/render_widget_host_view_win.h (right):

https://chromiumcodereview.appspot.com/21189003/diff/22001/content/browser/re...
content/browser/renderer_host/render_widget_host_view_win.h:541:
ui::TextInputMode text_input_mode_;
On 2013/08/07 04:17:18, Yohei Yukawa wrote:
> Please move this field next to |text_input_type_| so that
> one can easily find we have two different fields that represent
> input field attributes.

Done.

https://chromiumcodereview.appspot.com/21189003/diff/22001/content/browser/re...
File content/browser/renderer_host/render_widget_host_view_win_browsertest.cc
(right):

https://chromiumcodereview.appspot.com/21189003/diff/22001/content/browser/re...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:90:
view_->TextInputTypeChanged(ui::TEXT_INPUT_TYPE_NONE, false,
On 2013/08/07 04:17:18, Yohei Yukawa wrote:
> (optional) In future CL, please consider to reorder the parameter as
>   (ui::TEXT_INPUT_TYPE_NONE, ui::TEXT_INPUT_MODE_EMAIL, false)
I see.

https://chromiumcodereview.appspot.com/21189003/diff/22001/ui/base/ime/win/im...
File ui/base/ime/win/imm32_manager.h (right):

https://chromiumcodereview.appspot.com/21189003/diff/22001/ui/base/ime/win/im...
ui/base/ime/win/imm32_manager.h:80: ~IMM32Manager();
On 2013/08/07 04:17:18, Yohei Yukawa wrote:
> Should be as follows.
>   virtual ~IMM32Manager();

Done.

[Yohei Yukawa, 2013-08-07 14:17:11]

with a nit.

https://codereview.chromium.org/21189003/diff/30001/content/browser/renderer_...
File content/browser/renderer_host/render_widget_host_view_win_browsertest.cc
(right):

https://codereview.chromium.org/21189003/diff/30001/content/browser/renderer_...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:34:
~MockIMM32Manager() {}
  virtual ~MockIMM32Manager() {}

[Seigo Nonaka, 2013-08-07 16:42:06]

lgtm

[yoichio, 2013-08-08 02:22:32]

https://chromiumcodereview.appspot.com/21189003/diff/30001/content/browser/re...
File content/browser/renderer_host/render_widget_host_view_win_browsertest.cc
(right):

https://chromiumcodereview.appspot.com/21189003/diff/30001/content/browser/re...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:34:
~MockIMM32Manager() {}
On 2013/08/07 14:17:12, Yohei Yukawa wrote:
>   virtual ~MockIMM32Manager() {}

Done.

[Yohei Yukawa, 2013-08-08 12:54:33]

lgtm

[commit-bot: I haz the power, 2013-08-08 13:49:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoichio@chromium.org/21189003/44001

[commit-bot: I haz the power, 2013-08-08 23:53:48]

Change committed as 216494