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

1 files changed, 58 insertions, 90 deletions

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 @@ class PropertyBase {
   // 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 @@ class PropertySet {
                                 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 @@ class PropertySet {
   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 @@ class PropertySet {
 //
 // 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 @@ class PropertySet {
 // 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));
-  }
-
-  // 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);
+  // remote object.
+  virtual void Set(const T& value, dbus::PropertySet::SetCallback callback) {
+    set_value_ = value;
+    property_set()->Set(this, callback);
   }
 
-  // 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.
-  // Implementation provided by specialization.
-  virtual bool PopValueFromReader(MessageReader* reader);
+  // 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*);
 
-  // Appends the passed |value| to |writer|, generally as a 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 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