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
Review URL: https://chromiumcodereview.appspot.com/10698027
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@144756 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 091e0b6f3daf32cf818c0f6bbe8cb0b57c1cf8fb
diff --git a/dbus/property.cc b/dbus/property.cc
index 092135e..19683c7 100644
--- a/dbus/property.cc
+++ b/dbus/property.cc
@@ -91,6 +91,36 @@
}
+void PropertySet::Get(PropertyBase* property, GetCallback callback) {
+ MethodCall method_call(kPropertiesInterface, kPropertiesGet);
+ MessageWriter writer(&method_call);
+ writer.AppendString(interface());
+ writer.AppendString(property->name());
+
+ DCHECK(object_proxy_);
+ object_proxy_->CallMethod(&method_call,
+ ObjectProxy::TIMEOUT_USE_DEFAULT,
+ base::Bind(&PropertySet::OnGet,
+ GetWeakPtr(),
+ property,
+ callback));
+}
+
+void PropertySet::OnGet(PropertyBase* property, GetCallback callback,
+ Response* response) {
+ if (!response) {
+ LOG(WARNING) << property->name() << ": Get: failed.";
+ return;
+ }
+
+ MessageReader reader(response);
+ if (property->PopValueFromReader(&reader))
+ NotifyPropertyChanged(property->name());
+
+ if (!callback.is_null())
+ callback.Run(response);
+}
+
void PropertySet::GetAll() {
MethodCall method_call(kPropertiesInterface, kPropertiesGetAll);
MessageWriter writer(&method_call);
@@ -116,6 +146,28 @@
}
}
+void PropertySet::Set(PropertyBase* property, SetCallback callback) {
+ MethodCall method_call(kPropertiesInterface, kPropertiesSet);
+ MessageWriter writer(&method_call);
+ writer.AppendString(interface());
+ writer.AppendString(property->name());
+ property->AppendSetValueToWriter(&writer);
+
+ DCHECK(object_proxy_);
+ object_proxy_->CallMethod(&method_call,
+ ObjectProxy::TIMEOUT_USE_DEFAULT,
+ base::Bind(&PropertySet::OnSet,
+ GetWeakPtr(),
+ property,
+ callback));
+}
+
+void PropertySet::OnSet(PropertyBase* property, SetCallback callback,
+ Response* response) {
+ LOG_IF(WARNING, !response) << property->name() << ": Set: failed.";
+ if (!callback.is_null())
+ callback.Run(response);
+}
bool PropertySet::UpdatePropertiesFromReader(MessageReader* reader) {
DCHECK(reader);
@@ -163,8 +215,7 @@
//
template <>
-Property<uint8>::Property() : value_(0),
- weak_ptr_factory_(this) {
+Property<uint8>::Property() : value_(0) {
}
template <>
@@ -173,9 +224,8 @@
}
template <>
-void Property<uint8>::AppendToWriter(MessageWriter* writer,
- const uint8& value) {
- writer->AppendVariantOfByte(value);
+void Property<uint8>::AppendSetValueToWriter(MessageWriter* writer) {
+ writer->AppendVariantOfByte(set_value_);
}
//
@@ -183,8 +233,7 @@
//
template <>
-Property<bool>::Property() : value_(false),
- weak_ptr_factory_(this) {
+Property<bool>::Property() : value_(false) {
}
template <>
@@ -193,9 +242,8 @@
}
template <>
-void Property<bool>::AppendToWriter(MessageWriter* writer,
- const bool& value) {
- writer->AppendVariantOfBool(value);
+void Property<bool>::AppendSetValueToWriter(MessageWriter* writer) {
+ writer->AppendVariantOfBool(set_value_);
}
//
@@ -203,8 +251,7 @@
//
template <>
-Property<int16>::Property() : value_(0),
- weak_ptr_factory_(this) {
+Property<int16>::Property() : value_(0) {
}
template <>
@@ -213,9 +260,8 @@
}
template <>
-void Property<int16>::AppendToWriter(MessageWriter* writer,
- const int16& value) {
- writer->AppendVariantOfInt16(value);
+void Property<int16>::AppendSetValueToWriter(MessageWriter* writer) {
+ writer->AppendVariantOfInt16(set_value_);
}
//
@@ -223,8 +269,7 @@
//
template <>
-Property<uint16>::Property() : value_(0),
- weak_ptr_factory_(this) {
+Property<uint16>::Property() : value_(0) {
}
template <>
@@ -233,9 +278,8 @@
}
template <>
-void Property<uint16>::AppendToWriter(MessageWriter* writer,
- const uint16& value) {
- writer->AppendVariantOfUint16(value);
+void Property<uint16>::AppendSetValueToWriter(MessageWriter* writer) {
+ writer->AppendVariantOfUint16(set_value_);
}
//
@@ -243,8 +287,7 @@
//
template <>
-Property<int32>::Property() : value_(0),
- weak_ptr_factory_(this) {
+Property<int32>::Property() : value_(0) {
}
template <>
@@ -253,9 +296,8 @@
}
template <>
-void Property<int32>::AppendToWriter(MessageWriter* writer,
- const int32& value) {
- writer->AppendVariantOfInt32(value);
+void Property<int32>::AppendSetValueToWriter(MessageWriter* writer) {
+ writer->AppendVariantOfInt32(set_value_);
}
//
@@ -263,8 +305,7 @@
//
template <>
-Property<uint32>::Property() : value_(0),
- weak_ptr_factory_(this) {
+Property<uint32>::Property() : value_(0) {
}
template <>
@@ -273,9 +314,8 @@
}
template <>
-void Property<uint32>::AppendToWriter(MessageWriter* writer,
- const uint32& value) {
- writer->AppendVariantOfUint32(value);
+void Property<uint32>::AppendSetValueToWriter(MessageWriter* writer) {
+ writer->AppendVariantOfUint32(set_value_);
}
//
@@ -283,8 +323,7 @@
//
template <>
-Property<int64>::Property() : value_(0),
- weak_ptr_factory_(this) {
+Property<int64>::Property() : value_(0) {
}
template <>
@@ -293,9 +332,8 @@
}
template <>
-void Property<int64>::AppendToWriter(MessageWriter* writer,
- const int64& value) {
- writer->AppendVariantOfInt64(value);
+void Property<int64>::AppendSetValueToWriter(MessageWriter* writer) {
+ writer->AppendVariantOfInt64(set_value_);
}
//
@@ -303,8 +341,7 @@
//
template <>
-Property<uint64>::Property() : value_(0),
- weak_ptr_factory_(this) {
+Property<uint64>::Property() : value_(0) {
}
template <>
@@ -313,9 +350,8 @@
}
template <>
-void Property<uint64>::AppendToWriter(MessageWriter* writer,
- const uint64& value) {
- writer->AppendVariantOfUint64(value);
+void Property<uint64>::AppendSetValueToWriter(MessageWriter* writer) {
+ writer->AppendVariantOfUint64(set_value_);
}
//
@@ -323,8 +359,7 @@
//
template <>
-Property<double>::Property() : value_(0.0),
- weak_ptr_factory_(this) {
+Property<double>::Property() : value_(0.0) {
}
template <>
@@ -333,9 +368,8 @@
}
template <>
-void Property<double>::AppendToWriter(MessageWriter* writer,
- const double& value) {
- writer->AppendVariantOfDouble(value);
+void Property<double>::AppendSetValueToWriter(MessageWriter* writer) {
+ writer->AppendVariantOfDouble(set_value_);
}
//
@@ -348,9 +382,8 @@
}
template <>
-void Property<std::string>::AppendToWriter(MessageWriter* writer,
- const std::string& value) {
- writer->AppendVariantOfString(value);
+void Property<std::string>::AppendSetValueToWriter(MessageWriter* writer) {
+ writer->AppendVariantOfString(set_value_);
}
//
@@ -363,9 +396,8 @@
}
template <>
-void Property<ObjectPath>::AppendToWriter(MessageWriter* writer,
- const ObjectPath& value) {
- writer->AppendVariantOfObjectPath(value);
+void Property<ObjectPath>::AppendSetValueToWriter(MessageWriter* writer) {
+ writer->AppendVariantOfObjectPath(set_value_);
}
//
@@ -384,12 +416,11 @@
}
template <>
-void Property<std::vector<std::string> >::AppendToWriter(
- MessageWriter* writer,
- const std::vector<std::string>& value) {
+void Property<std::vector<std::string> >::AppendSetValueToWriter(
+ MessageWriter* writer) {
MessageWriter variant_writer(NULL);
writer->OpenVariant("as", &variant_writer);
- variant_writer.AppendArrayOfStrings(value);
+ variant_writer.AppendArrayOfStrings(set_value_);
writer->CloseContainer(&variant_writer);
}
@@ -409,12 +440,11 @@
}
template <>
-void Property<std::vector<ObjectPath> >::AppendToWriter(
- MessageWriter* writer,
- const std::vector<ObjectPath>& value) {
+void Property<std::vector<ObjectPath> >::AppendSetValueToWriter(
+ MessageWriter* writer) {
MessageWriter variant_writer(NULL);
writer->OpenVariant("ao", &variant_writer);
- variant_writer.AppendArrayOfObjectPaths(value);
+ variant_writer.AppendArrayOfObjectPaths(set_value_);
writer->CloseContainer(&variant_writer);
}
diff --git a/dbus/property.h b/dbus/property.h
index b214839..5c716e6 100644
--- a/dbus/property.h
+++ b/dbus/property.h
@@ -157,8 +157,14 @@
// Method used by PropertySet to retrieve the value from a MessageReader,
// no knowledge of the contained type is required, this method returns
// true if its expected type was found, false if not.
+ // Implementation provided by specialization.
virtual bool PopValueFromReader(MessageReader*) = 0;
+ // Method used by PropertySet to append the set value to a MessageWriter,
+ // no knowledge of the contained type is required.
+ // Implementation provided by specialization.
+ virtual void AppendSetValueToWriter(MessageWriter* writer) = 0;
+
protected:
// Retrieves the associated property set.
PropertySet* property_set() { return property_set_; }
@@ -223,6 +229,20 @@
const std::string& signal_name,
bool success);
+ // Callback for Get() method, |success| indicates whether or not the
+ // value could be retrived, if true the new value can be obtained by
+ // calling value() on the property.
+ typedef base::Callback<void(bool success)> GetCallback;
+
+ // Requests an updated value from the remote object for |property|
+ // incurring a round-trip. |callback| will be called when the new
+ // value is available. This may not be implemented by some interfaces,
+ // and may be overriden by sub-classes if interfaces use different
+ // method calls.
+ virtual void Get(PropertyBase* property, GetCallback callback);
+ virtual void OnGet(PropertyBase* property, GetCallback callback,
+ Response* response);
+
// Queries the remote object for values of all properties and updates
// initial values. Sub-classes may override to use a different D-Bus
// method, or if the remote object does not support retrieving all
@@ -230,6 +250,19 @@
virtual void GetAll();
virtual void OnGetAll(Response* response);
+ // Callback for Set() method, |success| indicates whether or not the
+ // new property value was accepted by the remote object.
+ typedef base::Callback<void(bool success)> SetCallback;
+
+ // Requests that the remote object for |property| change the property to
+ // its new value. |callback| will be called to indicate the success or
+ // failure of the request, however the new value may not be available
+ // depending on the remote object. This method may be overridden by
+ // sub-classes if interfaces use different method calls.
+ virtual void Set(PropertyBase* property, SetCallback callback);
+ virtual void OnSet(PropertyBase* property, SetCallback callback,
+ Response* response);
+
// Update properties by reading an array of dictionary entries, each
// containing a string with the name and a variant with the value, from
// |message_reader|. Returns false if message is in incorrect format.
@@ -293,8 +326,8 @@
//
// Properties provide a cached value that has an initial sensible default
// until the reply to PropertySet::GetAll() is retrieved and is updated by
-// all calls to that method, Property<>::Get() and property changed signals
-// handled by PropertySet. It can be obtained by calling value() on the
+// all calls to that method, PropertySet::Get() and property changed signals
+// also handled by PropertySet. It can be obtained by calling value() on the
// property.
//
// It is recommended that this cached value be used where necessary, with
@@ -303,119 +336,54 @@
// access.
//
// Where a round-trip is necessary, the Get() method is provided. And to
-// update the remote object value, the Set() method is also provided.
+// update the remote object value, the Set() method is also provided; these
+// both simply call methods on PropertySet.
//
// Handling of particular D-Bus types is performed via specialization,
-// typically the PopValueFromReader() and AppendToWriter() methods will need
-// to be provided, and in rare cases a constructor to provide a default value.
-// Specializations for basic D-Bus types, strings, object paths and arrays
-// are provided for you.
+// typically the PopValueFromReader() and AppendSetValueToWriter() methods
+// will need to be provided, and in rare cases a constructor to provide a
+// default value. Specializations for basic D-Bus types, strings, object
+// paths and arrays are provided for you.
template <class T>
class Property : public PropertyBase {
public:
- // Callback for Get() method, |success| indicates whether or not the
- // value could be retrived, if true the new value can be obtained by
- // calling value() on the property.
- typedef base::Callback<void(bool success)> GetCallback;
-
- // Callback for Set() method, |success| indicates whether or not the
- // new property value was accepted by the remote object.
- typedef base::Callback<void(bool success)> SetCallback;
-
- Property() : weak_ptr_factory_(this) {}
+ Property() {}
// Retrieves the cached value.
const T& value() const { return value_; }
// Requests an updated value from the remote object incurring a
// round-trip. |callback| will be called when the new value is available.
- // This may not be implemented by some interfaces, and may be overriden
- // by sub-classes if interfaces use different method calls.
- virtual void Get(GetCallback callback) {
- MethodCall method_call(kPropertiesInterface, kPropertiesGet);
- MessageWriter writer(&method_call);
- writer.AppendString(property_set()->interface());
- writer.AppendString(name());
-
- ObjectProxy* object_proxy = property_set()->object_proxy();
- DCHECK(object_proxy);
- object_proxy->CallMethod(&method_call,
- ObjectProxy::TIMEOUT_USE_DEFAULT,
- base::Bind(&Property<T>::OnGet,
- GetWeakPtr(),
- callback));
- }
-
- // Callback for Get(), may be overriden by sub-classes if interfaces
- // use different response arguments.
- virtual void OnGet(SetCallback callback, Response* response) {
- if (!response) {
- LOG(WARNING) << name() << ": Get: failed.";
- return;
- }
-
- MessageReader reader(response);
- if (PopValueFromReader(&reader))
- property_set()->NotifyPropertyChanged(name());
-
- if (!callback.is_null())
- callback.Run(response);
+ // This may not be implemented by some interfaces.
+ virtual void Get(dbus::PropertySet::GetCallback callback) {
+ property_set()->Get(this, callback);
}
// Requests that the remote object change the property value to |value|,
// |callback| will be called to indicate the success or failure of the
// request, however the new value may not be available depending on the
- // remote object. This method may be overridden by sub-classes if
- // interfaces use different method calls.
- virtual void Set(const T& value, SetCallback callback) {
- MethodCall method_call(kPropertiesInterface, kPropertiesSet);
- MessageWriter writer(&method_call);
- writer.AppendString(property_set()->interface());
- writer.AppendString(name());
- AppendToWriter(&writer, value);
-
- ObjectProxy* object_proxy = property_set()->object_proxy();
- DCHECK(object_proxy);
- object_proxy->CallMethod(&method_call,
- ObjectProxy::TIMEOUT_USE_DEFAULT,
- base::Bind(&Property<T>::OnSet,
- GetWeakPtr(),
- callback));
+ // remote object.
+ virtual void Set(const T& value, dbus::PropertySet::SetCallback callback) {
+ set_value_ = value;
+ property_set()->Set(this, callback);
}
- // Callback for Set(), may be overriden by sub-classes if interfaces
- // use different response arguments.
- virtual void OnSet(SetCallback callback, Response* response) {
- LOG_IF(WARNING, !response) << name() << ": Set: failed.";
- if (!callback.is_null())
- callback.Run(response);
- }
+ // Method used by PropertySet to retrieve the value from a MessageReader,
+ // no knowledge of the contained type is required, this method returns
+ // true if its expected type was found, false if not.
+ virtual bool PopValueFromReader(MessageReader*);
- // Updates the cached property value, replacing any previous value
- // entirely, by popping from |reader| which should be positioned at the
- // property value, generally of variant type.
+ // Method used by PropertySet to append the set value to a MessageWriter,
+ // no knowledge of the contained type is required.
// Implementation provided by specialization.
- virtual bool PopValueFromReader(MessageReader* reader);
-
- // Appends the passed |value| to |writer|, generally as a variant type.
- // Implementation provided by specialization.
- virtual void AppendToWriter(MessageWriter* writer, const T& value);
-
- protected:
- // Get a weak pointer to this propertyt, provided so that sub-classes
- // overriding methods that make D-Bus calls may use the existing (or
- // override) callbacks without providing their own weak pointer factory.
- base::WeakPtr<Property<T> > GetWeakPtr() {
- return weak_ptr_factory_.GetWeakPtr();
- }
+ virtual void AppendSetValueToWriter(MessageWriter* writer);
private:
// Current cached value of the property.
T value_;
- // Weak pointer factory as D-Bus callbacks may last longer than these
- // objects.
- base::WeakPtrFactory<Property<T> > weak_ptr_factory_;
+ // Replacement value of the property.
+ T set_value_;
};
} // namespace dbus