Shill: Delay the ConnectToSignal call until an observer is added.

chromeos/dbus/shill_client_helper.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROMEOS_DBUS_SHILL_CLIENT_HELPER_H_
 #define CHROMEOS_DBUS_SHILL_CLIENT_HELPER_H_
 
 #include <string>
 
 #include "base/basictypes.h"
@@ skipping 54 matching lines @@
   ShillClientHelper(dbus::Bus* bus, dbus::ObjectProxy* proxy);
 
   virtual ~ShillClientHelper();
 
   // Adds an |observer| of the PropertyChanged signal.
   void AddPropertyChangedObserver(ShillPropertyChangedObserver* observer);
 
   // Removes an |observer| of the PropertyChanged signal.
   void RemovePropertyChangedObserver(ShillPropertyChangedObserver* observer);
 
-  // Starts monitoring PropertyChanged signal.
+  // Starts monitoring PropertyChanged signal. If there aren't observers for the
+  // PropertyChanged signal, the actual monitoring will be delayed until the
+  // first observer is added.
   void MonitorPropertyChanged(const std::string& interface_name);
 
   // Calls a method without results.
   void CallVoidMethod(dbus::MethodCall* method_call,
                       const VoidDBusMethodCallback& callback);
 
   // Calls a method with an object path result.
   void CallObjectPathMethod(dbus::MethodCall* method_call,
                             const ObjectPathDBusMethodCallback& callback);
 
@@ skipping 32 matching lines @@
   // This method returns NULL when method call fails.
   base::DictionaryValue* CallDictionaryValueMethodAndBlock(
       dbus::MethodCall* method_call);
 
   // Appends the value (basic types and string-to-string dictionary) to the
   // writer as a variant.
   static void AppendValueDataAsVariant(dbus::MessageWriter* writer,
                                        const base::Value& value);
 
  private:
+  // Starts monitoring PropertyChanged signal.
+  void MonitorPropertyChangedInternal(const std::string& interface_name);
+
   // Handles the result of signal connection setup.
   void OnSignalConnected(const std::string& interface,
                          const std::string& signal,
                          bool success);
 
   // Handles PropertyChanged signal.
   void OnPropertyChanged(dbus::Signal* signal);
 
   // Handles responses for methods without results.
   void OnVoidMethod(const VoidDBusMethodCallback& callback,
@@ skipping 35 matching lines @@
   // Handles errors for method calls.
   void OnError(const ErrorCallback& error_callback,
                dbus::ErrorResponse* response);
 
   // TODO(hashimoto): Remove this when we no longer need to make blocking calls.
   BlockingMethodCaller blocking_method_caller_;
   dbus::ObjectProxy* proxy_;
   PropertyChangedHandler property_changed_handler_;
   ObserverList<ShillPropertyChangedObserver, true /* check_empty */>
       observer_list_;
+  std::vector<std::string> monitored_interfaces_;

[hashimoto, 2013/02/13 04:01:31]

nit: Since all users are using this class for a single class, just having a
string, instead of vector<string>, may suffice. (maybe interface name should be
passed to the ctor, though it can be done in a separate patch if we are going to
merge this to M26)
another-nit: How about renaming this variable to "interfaces_to_be_monitored_"
or something?  At least, interfaces are not observed as long as they are stored
in this vector.

[deymo, 2013/02/13 05:41:07]

1st nit: The current interface doesn't prevent someone to call several times
MonitorPropertyChanged with different interfaces. With this interface, even
handling "one string or nothing" looks simpler with a vector. +1 to pass the
interface name in the constructor (but in another CL). In that other case a
boolean will be better.

2nd nit: Done.

On 2013/02/13 04:01:31, hashimoto wrote:

 
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate its weak pointers before any other members are destroyed.
   base::WeakPtrFactory<ShillClientHelper> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ShillClientHelper);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROMEOS_DBUS_SHILL_CLIENT_HELPER_H_