Implement IBusProperty

Implement IBusProperty

IBusProperty and IBusPropertyList are one of representations an obeject used in communication with ibus-daemon. 

According to this CL, ibus_property.cc will be comipled and tested but not in 
used production binary at thi moment. 

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

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

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

[satorux1, 2012-05-21 20:05:02]

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_obj...
File chromeos/dbus/ibus/ibus_object.cc (right):

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_obj...
chromeos/dbus/ibus/ibus_object.cc:13: 
use 'ibus' namespace  in the file too.

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_pro...
File chromeos/dbus/ibus/ibus_property.cc (right):

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.cc:31: uint32 type;
= 0

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.cc:63: bool sensitive;
= false. please fix other places too

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_pro...
File chromeos/dbus/ibus/ibus_property_unittest.cc (right):

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property_unittest.cc:111: base::snprintf(prefix_buffer,
sizeof(prefix_buffer), "Sub%lu_", i);
Please just use base::StringPrintf

[Seigo Nonaka, 2012-05-21 20:55:21]

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_obj...
File chromeos/dbus/ibus/ibus_object.cc (right):

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_obj...
chromeos/dbus/ibus/ibus_object.cc:13: 
On 2012/05/21 20:05:02, satorux1 wrote:
> use 'ibus' namespace  in the file too.

Done in other CL. Rebased.

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_pro...
File chromeos/dbus/ibus/ibus_property.cc (right):

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.cc:31: uint32 type;
On 2012/05/21 20:05:02, satorux1 wrote:
> = 0

Done.

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.cc:63: bool sensitive;
On 2012/05/21 20:05:02, satorux1 wrote:
> = false. please fix other places too

Done.

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_pro...
File chromeos/dbus/ibus/ibus_property_unittest.cc (right):

http://codereview.chromium.org/10383253/diff/1008/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property_unittest.cc:111: base::snprintf(prefix_buffer,
sizeof(prefix_buffer), "Sub%lu_", i);
On 2012/05/21 20:05:02, satorux1 wrote:
> Please just use base::StringPrintf

Done.

[satorux1, 2012-05-21 20:58:58]

http://codereview.chromium.org/10383253/diff/8002/chromeos/dbus/ibus/ibus_pro...
File chromeos/dbus/ibus/ibus_property.h (right):

http://codereview.chromium.org/10383253/diff/8002/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.h:82: std::string key() const {
Please document what the key is, with an example.

http://codereview.chromium.org/10383253/diff/8002/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.h:89: uint32 type() const {
ditto. Please document other members too.

[Seigo Nonaka, 2012-05-21 22:04:43]

https://chromiumcodereview.appspot.com/10383253/diff/8002/chromeos/dbus/ibus/...
File chromeos/dbus/ibus/ibus_property.h (right):

https://chromiumcodereview.appspot.com/10383253/diff/8002/chromeos/dbus/ibus/...
chromeos/dbus/ibus/ibus_property.h:82: std::string key() const {
On 2012/05/21 20:58:58, satorux1 wrote:
> Please document what the key is, with an example.

Done. Wrote in member variable section.

https://chromiumcodereview.appspot.com/10383253/diff/8002/chromeos/dbus/ibus/...
chromeos/dbus/ibus/ibus_property.h:89: uint32 type() const {
On 2012/05/21 20:58:58, satorux1 wrote:
> ditto. Please document other members too.

Done.

[satorux1, 2012-06-05 17:00:42]

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

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_obj...
chromeos/dbus/ibus/ibus_object_unittest.cc:198: }
add a blank line after this function

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
File chromeos/dbus/ibus/ibus_property.cc (right):

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.cc:44: DCHECK_LE(type, 4UL);
I think it's unsafe to DCHECK. You are dealing with data from external programs,
and they may be sending broken data.

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.cc:93: DCHECK_LE(state, 3UL);
ditto.

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
File chromeos/dbus/ibus/ibus_property.h (right):

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.h:24: bool CHROMEOS_EXPORT
PopIBusProperty(dbus::MessageReader* reader,
Please write function comments for these.

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.h:33: dbus::MessageWriter* writer);
Please write function comments for these functions.

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.h:89: std::string key() const {
nit: please make it one line, where you can:

std::string key() const { return key_; }

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.h:145: // The identity for the IBusProperty.
please move the comment to key()

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.h:148: // The type of property:
ditto.

[Seigo Nonaka, 2012-06-18 06:02:02]

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

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_obj...
chromeos/dbus/ibus/ibus_object_unittest.cc:198: }
On 2012/06/05 17:00:42, satorux1 wrote:
> add a blank line after this function

Done.

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
File chromeos/dbus/ibus/ibus_property.cc (right):

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.cc:44: DCHECK_LE(type, 4UL);
On 2012/06/05 17:00:42, satorux1 wrote:
> I think it's unsafe to DCHECK. You are dealing with data from external
programs,
> and they may be sending broken data.

Done.

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.cc:93: DCHECK_LE(state, 3UL);
On 2012/06/05 17:00:42, satorux1 wrote:
> ditto.

Done.

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
File chromeos/dbus/ibus/ibus_property.h (right):

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.h:24: bool CHROMEOS_EXPORT
PopIBusProperty(dbus::MessageReader* reader,
On 2012/06/05 17:00:42, satorux1 wrote:
> Please write function comments for these.

Done.

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.h:33: dbus::MessageWriter* writer);
On 2012/06/05 17:00:42, satorux1 wrote:
> Please write function comments for these functions.

Done.

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.h:89: std::string key() const {
On 2012/06/05 17:00:42, satorux1 wrote:
> nit: please make it one line, where you can:
> 
> std::string key() const { return key_; }

Done.

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.h:145: // The identity for the IBusProperty.
On 2012/06/05 17:00:42, satorux1 wrote:
> please move the comment to key()

Done.

http://codereview.chromium.org/10383253/diff/3004/chromeos/dbus/ibus/ibus_pro...
chromeos/dbus/ibus/ibus_property.h:148: // The type of property:
On 2012/06/05 17:00:42, satorux1 wrote:
> ditto.

Done.

[satorux1, 2012-06-18 15:45:52]

LGTM

[commit-bot: I haz the power, 2012-06-19 07:28:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/10383253/27008

[commit-bot: I haz the power, 2012-06-19 09:21:27]

Try job failure for 10383253-27008 (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...

[commit-bot: I haz the power, 2012-06-19 09:39:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/10383253/27008

[commit-bot: I haz the power, 2012-06-19 12:16:17]

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

[Seigo Nonaka, 2012-06-21 07:57:13]

Sorry, I break tree with this CL.
I changed to using Uint64ToString instead of StringPrintf.

Could you check again?

Thanks.

[satorux1, 2012-06-21 14:59:32]

LGTM

[commit-bot: I haz the power, 2012-06-22 02:48:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/10383253/43001

[commit-bot: I haz the power, 2012-06-22 03:43:30]

Try job failure for 10383253-43001 (retry) on mac_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-06-22 04:49:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/10383253/43001

[commit-bot: I haz the power, 2012-06-22 06:16:41]

Change committed as 143545