Introduce RenderWidgetHostViewWinTest for Tsf handling

Introduce RenderWidgetHostViewWinTest for Tsf handling

BUG=None
TBR=avi
TEST=try bots

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

[Yohei Yukawa, 2012-09-10 08:40:12]

lgtm

[Seigo Nonaka, 2012-09-10 16:20:06]

yukawa: Thank you for your review.

Add sky@ as a owner of ui and content.

Thanks

[sky, 2012-09-10 16:44:09]

http://codereview.chromium.org/10912171/diff/10003/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_win_browsertest.cc
(right):

http://codereview.chromium.org/10912171/diff/10003/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:23:
command_line->AppendSwitch("--enable-text-service-framework");
Can't you use the constant here?

http://codereview.chromium.org/10912171/diff/10003/ui/base/win/tsf_bridge.cc
File ui/base/win/tsf_bridge.cc (right):

http://codereview.chromium.org/10912171/diff/10003/ui/base/win/tsf_bridge.cc#...
ui/base/win/tsf_bridge.cc:335: TsfBridge* old_bridge = TsfBridge::GetInstance();
Who owns old_bridge?

http://codereview.chromium.org/10912171/diff/10003/ui/ui_unittests.gypi
File ui/ui_unittests.gypi (right):

http://codereview.chromium.org/10912171/diff/10003/ui/ui_unittests.gypi#newco...
ui/ui_unittests.gypi:42: 'base/win/mock_tsf_bridge.cc',
Aren't these automatically excluded since they have win in the path?

[Seigo Nonaka, 2012-09-10 19:04:10]

http://codereview.chromium.org/10912171/diff/10003/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_win_browsertest.cc
(right):

http://codereview.chromium.org/10912171/diff/10003/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_win_browsertest.cc:23:
command_line->AppendSwitch("--enable-text-service-framework");
Sorry, I forget using constant.
Fixed.

On 2012/09/10 16:44:09, sky wrote:
> Can't you use the constant here?

http://codereview.chromium.org/10912171/diff/10003/ui/base/win/tsf_bridge.cc
File ui/base/win/tsf_bridge.cc (right):

http://codereview.chromium.org/10912171/diff/10003/ui/base/win/tsf_bridge.cc#...
ui/base/win/tsf_bridge.cc:335: TsfBridge* old_bridge = TsfBridge::GetInstance();
The caller should release old_bridge.
Revise comment in header file.

On 2012/09/10 16:44:09, sky wrote:
> Who owns old_bridge?

http://codereview.chromium.org/10912171/diff/10003/ui/ui_unittests.gypi
File ui/ui_unittests.gypi (right):

http://codereview.chromium.org/10912171/diff/10003/ui/ui_unittests.gypi#newco...
ui/ui_unittests.gypi:42: 'base/win/mock_tsf_bridge.cc',
Yes! thanks.
Fixed.
On 2012/09/10 16:44:09, sky wrote:
> Aren't these automatically excluded since they have win in the path?

[sky, 2012-09-10 21:15:36]

LGTM

http://codereview.chromium.org/10912171/diff/1010/content/content_tests.gypi
File content/content_tests.gypi (right):

http://codereview.chromium.org/10912171/diff/1010/content/content_tests.gypi#...
content/content_tests.gypi:606:
'browser/renderer_host/render_widget_host_view_win_browsertest.cc',
sort

http://codereview.chromium.org/10912171/diff/1010/ui/base/win/mock_tsf_bridge.cc
File ui/base/win/mock_tsf_bridge.cc (right):

http://codereview.chromium.org/10912171/diff/1010/ui/base/win/mock_tsf_bridge...
ui/base/win/mock_tsf_bridge.cc:6: #include "ui/base/win/mock_tsf_bridge.h"
this should be your first include, newline, then other includes.

[Seigo Nonaka, 2012-09-12 19:02:24]

Thank you for your review.

http://codereview.chromium.org/10912171/diff/1010/content/content_tests.gypi
File content/content_tests.gypi (right):

http://codereview.chromium.org/10912171/diff/1010/content/content_tests.gypi#...
content/content_tests.gypi:606:
'browser/renderer_host/render_widget_host_view_win_browsertest.cc',
On 2012/09/10 21:15:36, sky wrote:
> sort

Done.

http://codereview.chromium.org/10912171/diff/1010/ui/base/win/mock_tsf_bridge.cc
File ui/base/win/mock_tsf_bridge.cc (right):

http://codereview.chromium.org/10912171/diff/1010/ui/base/win/mock_tsf_bridge...
ui/base/win/mock_tsf_bridge.cc:6: #include "ui/base/win/mock_tsf_bridge.h"
On 2012/09/10 21:15:36, sky wrote:
> this should be your first include, newline, then other includes.

Done.

[commit-bot: I haz the power, 2012-09-12 19:03:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nona@chromium.org/10912171/1011

[commit-bot: I haz the power, 2012-09-12 19:03:12]

Presubmit check for 10912171-1011 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    content

Presubmit checks took 1.2s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[Seigo Nonaka, 2012-09-13 02:51:38]

Add avi@ as content/ owner.
Could you check content/content_tests.gypi ?

Thank you.

[Seigo Nonaka, 2012-09-13 02:51:54]

Add avi@ as content/ owner.
Could you check content/content_tests.gypi ?

Thank you.

[commit-bot: I haz the power, 2012-09-22 16:48:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nona@chromium.org/10912171/30001

[commit-bot: I haz the power, 2012-09-22 20:10:53]

Change committed as 158189