Implement IBusInputContextClient

Implement IBusInputContextClient

IBusInputContextClient performs performs dbus communication with following spec.
Target Server: ibus-daemon
Service: org.freedesktop.IBus
ObjectPath: /org/freedesktop/IBus/InputContext_XXXX
(The object path is not determined until call CreateInputContext method, please
read commend for detail)

At this moment, all signal handler and method call implementation is not used in
production. According to this CL, IBusInputContextClient is
compiled and tested by chromeos_unittests.

BUG=chromium-os:26334
TEST=unit_tests,chromeos_unittests,dbus_unittests

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

[satorux1, 2012-05-10 21:40:52]

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
File chromeos/dbus/ibus/ibus_input_context_client.cc (right):

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:73: const CommitTextHandler&
commit_text_handler) OVERRIDE {
if this function can be called only once, add this:

DCHECK(commit_text_handler_.is_null());

If the given callback should be non null, add this:

DCHECK(!commit_text_handler.is_null());

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:204: LOG(ERROR) << "Failed to
call method:" << method_name;
"Failed to call method:" ->
"Failed to call method: "

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:213: LOG(ERROR) << "Cannot get
input context:" << response->ToString();
ditto. add a space after :

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:219: // The IBus message
structure may changed.
may be changed.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:247: uint32 state;
Initialize them with 0

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:265: bool visible;
Please initialize variables of primitive types.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:370: // IBusInputContextClient
override.
// IBusInputContextClient overrides.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:373: // IBusInputContextClient
override.
Then please remove all // IBusInputContextClient override. 
from here.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
File chromeos/dbus/ibus/ibus_input_context_client.h (right):

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.h:47: // Resets object proxy.
??? The function comment should match the function name..

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.h:54: // Sets CommitText signal
handler.
Can this function be called multiple times? or only once?

Can this function takes a null-callback?

Please document.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.h:89: virtual void
SetCursorLocation(int32 x, int32 y, int32 w, int32 h) = 0;
w, h -> width, height?

[Seigo Nonaka, 2012-05-10 22:26:08]

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
File chromeos/dbus/ibus/ibus_input_context_client.cc (right):

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:73: const CommitTextHandler&
commit_text_handler) OVERRIDE {
Support multiple calling.
On 2012/05/10 21:40:53, satorux1 wrote:
> if this function can be called only once, add this:
> 
> DCHECK(commit_text_handler_.is_null());
> 
> If the given callback should be non null, add this:
> 
> DCHECK(!commit_text_handler.is_null());

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:204: LOG(ERROR) << "Failed to
call method:" << method_name;
On 2012/05/10 21:40:53, satorux1 wrote:
> "Failed to call method:" ->
> "Failed to call method: "

Done.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:213: LOG(ERROR) << "Cannot get
input context:" << response->ToString();
On 2012/05/10 21:40:53, satorux1 wrote:
> ditto. add a space after :

Done.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:219: // The IBus message
structure may changed.
On 2012/05/10 21:40:53, satorux1 wrote:
> may be changed.

Done.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:247: uint32 state;
On 2012/05/10 21:40:53, satorux1 wrote:
> Initialize them with 0

Done.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:265: bool visible;
On 2012/05/10 21:40:53, satorux1 wrote:
> Please initialize variables of primitive types.

Done.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:370: // IBusInputContextClient
override.
On 2012/05/10 21:40:53, satorux1 wrote:
> // IBusInputContextClient overrides.

Done.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:373: // IBusInputContextClient
override.
On 2012/05/10 21:40:53, satorux1 wrote:
> Then please remove all // IBusInputContextClient override. 
> from here.

Done.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
File chromeos/dbus/ibus/ibus_input_context_client.h (right):

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.h:47: // Resets object proxy.
On 2012/05/10 21:40:53, satorux1 wrote:
> ??? The function comment should match the function name..

Done.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.h:54: // Sets CommitText signal
handler.
On 2012/05/10 21:40:53, satorux1 wrote:
> Can this function be called multiple times? or only once?
> 
> Can this function takes a null-callback?
> 
> Please document.

Done.

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.h:54: // Sets CommitText signal
handler.
On 2012/05/10 21:40:53, satorux1 wrote:
> Can this function be called multiple times? or only once?
> 
> Can this function takes a null-callback?
> 
> Please document.

Done.

[satorux1, 2012-05-11 22:45:14]

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
File chromeos/dbus/ibus/ibus_input_context_client.h (right):

http://codereview.chromium.org/10310090/diff/12002/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.h:89: virtual void
SetCursorLocation(int32 x, int32 y, int32 w, int32 h) = 0;
On 2012/05/10 21:40:53, satorux1 wrote:
> w, h -> width, height?

missed this comment?

http://codereview.chromium.org/10310090/diff/8010/chromeos/dbus/ibus/ibus_inp...
File chromeos/dbus/ibus/ibus_input_context_client.cc (right):

http://codereview.chromium.org/10310090/diff/8010/chromeos/dbus/ibus/ibus_inp...
chromeos/dbus/ibus/ibus_input_context_client.cc:75:
commit_text_handler_.Reset();
You don't have to Reset.

I thought we wanted to add

DCHECK(!commit_text_handler.is_null());

[Seigo Nonaka, 2012-05-12 00:54:38]

http://codereview.chromium.org/10310090/diff/8010/chromeos/dbus/ibus/ibus_inp...
File chromeos/dbus/ibus/ibus_input_context_client.cc (right):

http://codereview.chromium.org/10310090/diff/8010/chromeos/dbus/ibus/ibus_inp...
chromeos/dbus/ibus/ibus_input_context_client.cc:75:
commit_text_handler_.Reset();
On 2012/05/11 22:45:14, satorux1 wrote:
> You don't have to Reset.
> 
> I thought we wanted to add
> 
> DCHECK(!commit_text_handler.is_null());

Done.

[satorux1, 2012-05-22 21:41:00]

LGTM with nits

http://codereview.chromium.org/10310090/diff/18001/chromeos/dbus/ibus/ibus_in...
File chromeos/dbus/ibus/ibus_input_context_client.cc (right):

http://codereview.chromium.org/10310090/diff/18001/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:174: virtual void
SetCursorLocation(int32 x, int32 y, int32 w, int32 h) OVERRIDE {
w -> width, h -> height...

http://codereview.chromium.org/10310090/diff/18001/chromeos/dbus/ibus/ibus_in...
File chromeos/dbus/ibus/ibus_input_context_client.h (right):

http://codereview.chromium.org/10310090/diff/18001/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.h:97: virtual void
SetCursorLocation(int32 x, int32 y, int32 w, int32 h) = 0;
width, height

[Seigo Nonaka, 2012-05-22 22:10:00]

Thanks! Done

http://codereview.chromium.org/10310090/diff/18001/chromeos/dbus/ibus/ibus_in...
File chromeos/dbus/ibus/ibus_input_context_client.cc (right):

http://codereview.chromium.org/10310090/diff/18001/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.cc:174: virtual void
SetCursorLocation(int32 x, int32 y, int32 w, int32 h) OVERRIDE {
On 2012/05/22 21:41:00, satorux1 wrote:
> w -> width, h -> height...

Done.

http://codereview.chromium.org/10310090/diff/18001/chromeos/dbus/ibus/ibus_in...
File chromeos/dbus/ibus/ibus_input_context_client.h (right):

http://codereview.chromium.org/10310090/diff/18001/chromeos/dbus/ibus/ibus_in...
chromeos/dbus/ibus/ibus_input_context_client.h:97: virtual void
SetCursorLocation(int32 x, int32 y, int32 w, int32 h) = 0;
On 2012/05/22 21:41:00, satorux1 wrote:
> width, height

Done.

[commit-bot: I haz the power, 2012-05-22 23:52:12]

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

[commit-bot: I haz the power, 2012-05-23 12:38:50]

Try job failure for 10310090-19002 (retry) (retry) (retry) on win_rel for step
"sync_unit_tests" (clobber build).
It's a second try, previously, steps "unit_tests, sync_unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...