Track active references in ShillClientHelper

chromeos/dbus/shill_client_helper.h

Index: chromeos/dbus/shill_client_helper.h
diff --git a/chromeos/dbus/shill_client_helper.h b/chromeos/dbus/shill_client_helper.h
index 12cd9948cf2f04b2bc86ae3e8293d53bd59a1ef6..90db963c48cdc304ce889d83942d7cb4ee7da080 100644
--- a/chromeos/dbus/shill_client_helper.h
+++ b/chromeos/dbus/shill_client_helper.h
@@ -69,10 +69,23 @@ class ShillClientHelper {
   typedef base::Callback<void(bool result)> BooleanCallback;
 
 
-  ShillClientHelper(dbus::Bus* bus, dbus::ObjectProxy* proxy);
+  // Interface class for owner of this class so that it can be notified.
+  class Owner {
+   public:
+    // Notifies the owner that |helper| can be released (no active references).
+    virtual void NotifyReleased(ShillClientHelper* helper) = 0;
+
+   protected:
+    virtual ~Owner() {}
+  };
+
+  explicit ShillClientHelper(dbus::ObjectProxy* proxy);
 
   virtual ~ShillClientHelper();
 
+  // Sets |owner_|. This is optional and should only be called at most once.
+  void SetOwner(Owner* owner);

[hashimoto, 2013/09/17 03:28:53]

nit: Since the Owner interface has only one method, it can be easily replaced
with a callback base::Bind(&ShillServiceClient::NotifyReleased, ...).
I think callbacks are better than interfaces because the cause and the result
(which method is called as which method's callback) is understandable without
looking at header files.

[stevenjb, 2013/09/17 21:03:17]

Sure, I'll change this to SetReleasedCallback. 

+
   // Adds an |observer| of the PropertyChanged signal.
   void AddPropertyChangedObserver(ShillPropertyChangedObserver* observer);
 
@@ -131,6 +144,13 @@ class ShillClientHelper {
       const ListValueCallback& callback,
       const ErrorCallback& error_callback);
 
+  // Reference / Ownership management. If the number of active refs (observers
+  // + in-progress method calls) becomes 0, |owner_| (if set) will be notified.
+  void AddRef();
+  void Release();

[hashimoto, 2013/09/17 03:28:53]

nit: How about making these methods private by making HelperRelease an inner
class of ShillClientHelper?

[stevenjb, 2013/09/17 21:03:17]

Done.

+
+  const dbus::ObjectProxy* object_proxy() const { return proxy_; }
+
   // Appends the value (basic types and string-to-string dictionary) to the
   // writer as a variant.
   static void AppendValueDataAsVariant(dbus::MessageWriter* writer,
@@ -154,6 +174,8 @@ class ShillClientHelper {
   void OnPropertyChanged(dbus::Signal* signal);
 
   dbus::ObjectProxy* proxy_;
+  Owner* owner_;
+  int active_refs_;
   PropertyChangedHandler property_changed_handler_;
   ObserverList<ShillPropertyChangedObserver, true /* check_empty */>
       observer_list_;