Extends IBusObject.

Extends IBusObject.

- Adds int32 and bool I/O into IBusObjectReader/Writer.
- Adds AppendIBusText and PopIBusText into IBusObjectReader/Writer
- Adds AppendStringAsIBusText and PopStringFromIBusText as utility function, because we frequently uses IBusText as a plain text in Chrome.

BUG=None
TEST=chromeos_unittests

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

[satorux1, 2012-05-18 01:23:36]

https://chromiumcodereview.appspot.com/10384141/diff/3001/chromeos/dbus/ibus/...
File chromeos/dbus/ibus/ibus_object.h (right):

https://chromiumcodereview.appspot.com/10384141/diff/3001/chromeos/dbus/ibus/...
chromeos/dbus/ibus/ibus_object.h:103: // NULL. This reader corresponds to second
depth struct field(see above
please add a space before (

https://chromiumcodereview.appspot.com/10384141/diff/3001/chromeos/dbus/ibus/...
chromeos/dbus/ibus/ibus_object.h:106: dbus::MessageReader* GetContentsReader();
I'm confused. Why do we need to expose this? Cannot clients just use
PopIBusObject()?

https://chromiumcodereview.appspot.com/10384141/diff/3001/chromeos/dbus/ibus/...
File chromeos/dbus/ibus/ibus_object_unittest.cc (right):

https://chromiumcodereview.appspot.com/10384141/diff/3001/chromeos/dbus/ibus/...
chromeos/dbus/ibus/ibus_object_unittest.cc:58: int32 expected_int32;
initialize this with 0. Please fix other uninitialized variables of primitive
types too.

[Seigo Nonaka, 2012-05-18 01:41:52]

http://codereview.chromium.org/10384141/diff/3001/chromeos/dbus/ibus/ibus_obj...
File chromeos/dbus/ibus/ibus_object.h (right):

http://codereview.chromium.org/10384141/diff/3001/chromeos/dbus/ibus/ibus_obj...
chromeos/dbus/ibus/ibus_object.h:103: // NULL. This reader corresponds to second
depth struct field(see above
On 2012/05/18 01:23:36, satorux1 wrote:
> please add a space before (

Done.

http://codereview.chromium.org/10384141/diff/3001/chromeos/dbus/ibus/ibus_obj...
chromeos/dbus/ibus/ibus_object.h:106: dbus::MessageReader* GetContentsReader();
If I want to add IBusText into some IBusObject, AppendIBusText takes a pointer
of dbus::MessageWriter.
One solution of this is introduce AppendIBusText into IBusObject. Is it clearer
than this CL's approach?

On 2012/05/18 01:23:36, satorux1 wrote:
> I'm confused. Why do we need to expose this? Cannot clients just use
> PopIBusObject()?

http://codereview.chromium.org/10384141/diff/3001/chromeos/dbus/ibus/ibus_obj...
File chromeos/dbus/ibus/ibus_object_unittest.cc (right):

http://codereview.chromium.org/10384141/diff/3001/chromeos/dbus/ibus/ibus_obj...
chromeos/dbus/ibus/ibus_object_unittest.cc:58: int32 expected_int32;
On 2012/05/18 01:23:36, satorux1 wrote:
> initialize this with 0. Please fix other uninitialized variables of primitive
> types too.

Done.

[satorux1, 2012-05-18 04:54:46]

http://codereview.chromium.org/10384141/diff/3001/chromeos/dbus/ibus/ibus_obj...
File chromeos/dbus/ibus/ibus_object.h (right):

http://codereview.chromium.org/10384141/diff/3001/chromeos/dbus/ibus/ibus_obj...
chromeos/dbus/ibus/ibus_object.h:106: dbus::MessageReader* GetContentsReader();
On 2012/05/18 01:41:52, Seigo Nonaka wrote:
> If I want to add IBusText into some IBusObject, AppendIBusText takes a pointer
> of dbus::MessageWriter.
> One solution of this is introduce AppendIBusText into IBusObject. Is it
clearer
> than this CL's approach?

Where is the IBusObject class? Is there such a class?

Generally, exposing a member as a mutable pointer is not preferable.

> 
> On 2012/05/18 01:23:36, satorux1 wrote:
> > I'm confused. Why do we need to expose this? Cannot clients just use
> > PopIBusObject()?
>

[Seigo Nonaka, 2012-05-18 17:01:47]

http://codereview.chromium.org/10384141/diff/3001/chromeos/dbus/ibus/ibus_obj...
File chromeos/dbus/ibus/ibus_object.h (right):

http://codereview.chromium.org/10384141/diff/3001/chromeos/dbus/ibus/ibus_obj...
chromeos/dbus/ibus/ibus_object.h:106: dbus::MessageReader* GetContentsReader();
Sorry not IBusObject, IBusObjectWriter is correct.

I upload not exposing solution for adding IBusText into IBusObject.

On 2012/05/18 04:54:46, satorux1 wrote:
> On 2012/05/18 01:41:52, Seigo Nonaka wrote:
> > If I want to add IBusText into some IBusObject, AppendIBusText takes a
pointer
> > of dbus::MessageWriter.
> > One solution of this is introduce AppendIBusText into IBusObject. Is it
> clearer
> > than this CL's approach?
> 
> Where is the IBusObject class? Is there such a class?
> 
> Generally, exposing a member as a mutable pointer is not preferable.
> 
> > 
> > On 2012/05/18 01:23:36, satorux1 wrote:
> > > I'm confused. Why do we need to expose this? Cannot clients just use
> > > PopIBusObject()?
> >

[satorux1, 2012-05-21 19:58:14]

Please update the patch description

http://codereview.chromium.org/10384141/diff/14001/chromeos/dbus/ibus/ibus_ob...
File chromeos/dbus/ibus/ibus_object.cc (right):

http://codereview.chromium.org/10384141/diff/14001/chromeos/dbus/ibus/ibus_ob...
chromeos/dbus/ibus/ibus_object.cc:13: // TODO(nona): Remove after complete
libibus removal.
We should put the code in the file in 'ibus' namespace too

http://codereview.chromium.org/10384141/diff/14001/chromeos/dbus/ibus/ibus_ob...
File chromeos/dbus/ibus/ibus_object_unittest.cc (right):

http://codereview.chromium.org/10384141/diff/14001/chromeos/dbus/ibus/ibus_ob...
chromeos/dbus/ibus/ibus_object_unittest.cc:17: using chromeos::ibus::IBusText;
use ibus namespace here too.

[Seigo Nonaka, 2012-05-21 20:12:00]

http://codereview.chromium.org/10384141/diff/14001/chromeos/dbus/ibus/ibus_ob...
File chromeos/dbus/ibus/ibus_object.cc (right):

http://codereview.chromium.org/10384141/diff/14001/chromeos/dbus/ibus/ibus_ob...
chromeos/dbus/ibus/ibus_object.cc:13: // TODO(nona): Remove after complete
libibus removal.
On 2012/05/21 19:58:14, satorux1 wrote:
> We should put the code in the file in 'ibus' namespace too

Done.

http://codereview.chromium.org/10384141/diff/14001/chromeos/dbus/ibus/ibus_ob...
File chromeos/dbus/ibus/ibus_object_unittest.cc (right):

http://codereview.chromium.org/10384141/diff/14001/chromeos/dbus/ibus/ibus_ob...
chromeos/dbus/ibus/ibus_object_unittest.cc:17: using chromeos::ibus::IBusText;
On 2012/05/21 19:58:14, satorux1 wrote:
> use ibus namespace here too.

Done.

[satorux1, 2012-05-21 20:13:31]

LGTM

[commit-bot: I haz the power, 2012-05-21 22:06:51]

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

[commit-bot: I haz the power, 2012-05-22 02:39:57]

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

[commit-bot: I haz the power, 2012-05-22 02:40:42]

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

[commit-bot: I haz the power, 2012-05-22 06:48:17]

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

[commit-bot: I haz the power, 2012-05-22 06:53:13]

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

[commit-bot: I haz the power, 2012-05-22 13:11:21]

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

[commit-bot: I haz the power, 2012-05-22 16:10:23]

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