Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(212)

Issue 10698027: dbus: move logic from Property<> to PropertySet (Closed)

Created:
8 years, 5 months ago by keybuk
Modified:
8 years, 5 months ago
Reviewers:
satorux1
CC:
chromium-reviews, stevenjb+watch_chromium.org, oshima+watch_chromium.org, bryeung, kevers
Visibility:
Public.

Description

dbus: move logic from Property<> to PropertySet Rather than implement Get() and Set() in dbus::Property<> move the code into dbus::PropertySet and pass a pointer to the property to operate on from the wrapper call. The advange of this way of doing things is that it's much easier to make subclasses, since you only need to subclass dbus::PropertySet; and ths makes it possible to mock. BUG=chromium-os:28555 TEST=dbus_unittests Change-Id: I760ca608d1e0a17422c11e0115c053d98be33fe0 R=satorux@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=144756

Patch Set 1 #

Patch Set 2 : add missing overrides for clang #

Unified diffs Side-by-side diffs Delta from patch set Stats (+214 lines, -229 lines) Patch
M chromeos/dbus/bluetooth_adapter_client.h View 1 chunk +11 lines, -11 lines 0 comments Download
M chromeos/dbus/bluetooth_device_client.h View 1 chunk +17 lines, -17 lines 0 comments Download
M chromeos/dbus/bluetooth_input_client.h View 1 chunk +1 line, -1 line 0 comments Download
M chromeos/dbus/bluetooth_manager_client.h View 1 chunk +1 line, -1 line 0 comments Download
M chromeos/dbus/bluetooth_node_client.h View 1 chunk +2 lines, -2 lines 0 comments Download
M chromeos/dbus/bluetooth_property.h View 1 2 chunks +12 lines, -48 lines 0 comments Download
M chromeos/dbus/bluetooth_property.cc View 2 chunks +23 lines, -0 lines 0 comments Download
M dbus/property.h View 5 chunks +58 lines, -90 lines 0 comments Download
M dbus/property.cc View 24 chunks +89 lines, -59 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
keybuk
8 years, 5 months ago (2012-06-28 01:36:12 UTC) #1
satorux1
LGTM. looks much cleaner!
8 years, 5 months ago (2012-06-28 17:01:27 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keybuk@chromium.org/10698027/7001
8 years, 5 months ago (2012-06-28 17:34:10 UTC) #3
commit-bot: I haz the power
8 years, 5 months ago (2012-06-28 18:43:32 UTC) #4
Change committed as 144756

Powered by Google App Engine
This is Rietveld 408576698