Revert 120456 - chromeos: bluetooth: tie Proxy lifetime to object, not observer

Change the lifetime of Object Proxies to the remote object, not any
Observer; one of the problems with the previous method was adding
a second observer would treat every new attempt to add an observer
as a new proxy so reconnect signals, etc.

The other main problem is that you couldn't call methods unless there
was an observer that had created the object proxy.

BUG=chromium-os:22086
TEST=verified methods still work

Change-Id: Ic58b88bbad84c7728abd16ddc48d11174f59f59f


Review URL: http://codereview.chromium.org/9314013

TBR=keybuk@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9316113

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@120458 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 49 insertions, 62 deletions

diff --git a/chrome/browser/chromeos/bluetooth/bluetooth_adapter.cc b/chrome/browser/chromeos/bluetooth/bluetooth_adapter.cc
index 2679274..ca9402f 100644
--- a/chrome/browser/chromeos/bluetooth/bluetooth_adapter.cc
+++ b/chrome/browser/chromeos/bluetooth/bluetooth_adapter.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -22,12 +22,12 @@ class BluetoothAdapterImpl : public BluetoothAdapter,
     bluetooth_adapter_client_ =
         dbus_thread_manager->GetBluetoothAdapterClient();
     DCHECK(bluetooth_adapter_client_);
-    bluetooth_adapter_client_->AddObserver(this);
+    bluetooth_adapter_client_->AddObserver(this, id_);
   }
 
   virtual ~BluetoothAdapterImpl() {
     DCHECK(bluetooth_adapter_client_);
-    bluetooth_adapter_client_->RemoveObserver(this);
+    bluetooth_adapter_client_->RemoveObserver(this, id_);
   }
 
   virtual void AddObserver(BluetoothAdapter::Observer* observer) {
diff --git a/chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc b/chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc
index 7e4a3a0..31ef1dc 100644
--- a/chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc
+++ b/chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -9,7 +9,6 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
-#include "chrome/browser/chromeos/dbus/bluetooth_manager_client.h"
 #include "chrome/browser/chromeos/system/runtime_environment.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
@@ -177,34 +176,32 @@ bool PopArrayOfDictEntries(dbus::MessageReader* reader,
 namespace chromeos {
 
 // The BluetoothAdapterClient implementation used in production.
-class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
-                                  private BluetoothManagerClient::Observer {
+class BluetoothAdapterClientImpl: public BluetoothAdapterClient {
  public:
-  explicit BluetoothAdapterClientImpl(dbus::Bus* bus,
-                                      BluetoothManagerClient* manager_client)
+  explicit BluetoothAdapterClientImpl(dbus::Bus* bus)
       : weak_ptr_factory_(this),
         bus_(bus) {
     VLOG(1) << "Creating BluetoothAdapterClientImpl";
-
-    DCHECK(manager_client);
-    manager_client->AddObserver(this);
   }
 
   virtual ~BluetoothAdapterClientImpl() {
   }
 
   // BluetoothAdapterClient override.
-  virtual void AddObserver(BluetoothAdapterClient::Observer* observer) {
-    VLOG(1) << "AddObserver";
+  virtual void AddObserver(Observer* observer, const std::string& object_path) {
+    VLOG(1) << "AddObserver: " << object_path;
     DCHECK(observer);
     observers_.AddObserver(observer);
+    AddObjectProxyForPath(object_path);
   }
 
   // BluetoothAdapterClient override.
-  virtual void RemoveObserver(BluetoothAdapterClient::Observer* observer) {
-    VLOG(1) << "RemoveObserver";
+  virtual void RemoveObserver(Observer* observer,
+                              const std::string& object_path) {
+    VLOG(1) << "RemoveObserver: " << object_path;
     DCHECK(observer);
     observers_.RemoveObserver(observer);
+    RemoveObjectProxyForPath(object_path);
   }
 
   // BluetoothAdapterClient override.
@@ -215,7 +212,12 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
         bluetooth_adapter::kBluetoothAdapterInterface,
         bluetooth_adapter::kStartDiscovery);
 
-    dbus::ObjectProxy* adapter_proxy = GetObjectProxyForPath(object_path);
+    ProxyMap::iterator it = proxy_map_.find(object_path);
+    if (it == proxy_map_.end()) {
+      LOG(ERROR) << "Couldn't find proxy for object path " << object_path;
+      return;
+    }
+    dbus::ObjectProxy* adapter_proxy = it->second;
 
     adapter_proxy->CallMethod(
         &method_call,
@@ -232,7 +234,12 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
         bluetooth_adapter::kBluetoothAdapterInterface,
         bluetooth_adapter::kStopDiscovery);
 
-    dbus::ObjectProxy* adapter_proxy = GetObjectProxyForPath(object_path);
+    ProxyMap::iterator it = proxy_map_.find(object_path);
+    if (it == proxy_map_.end()) {
+      LOG(ERROR) << "Couldn't find proxy for object path " << object_path;
+      return;
+    }
+    dbus::ObjectProxy* adapter_proxy = it->second;
 
     adapter_proxy->CallMethod(
         &method_call,
@@ -242,26 +249,10 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
   }
 
  private:
-  // BluetoothManagerClient::Observer override.
-  virtual void AdapterAdded(const std::string& object_path) OVERRIDE {
-    VLOG(1) << "AdapterAdded: " << object_path;
-  }
-
-  // BluetoothManagerClient::Observer override.
-  virtual void AdapterRemoved(const std::string& object_path) OVERRIDE {
-    VLOG(1) << "AdapterRemoved: " << object_path;
-    RemoveObjectProxyForPath(object_path);
-  }
-
-  // Ensures that we have a dbus object proxy for an adapter with dbus
-  // object path |object_path|, and if not, creates it and stores it in
-  // our |proxy_map_| map.
-  dbus::ObjectProxy* GetObjectProxyForPath(const std::string& object_path) {
-    VLOG(1) << "GetObjectProxyForPath: " << object_path;
-
-    ProxyMap::iterator it = proxy_map_.find(object_path);
-    if (it != proxy_map_.end())
-      return it->second;
+  // Gets a dbus object proxy for an adapter with dbus object path |object_path|
+  // and stores it in our |proxy_map_| map.
+  void AddObjectProxyForPath(const std::string& object_path) {
+    VLOG(1) << "AddObjectProxyForPath: " << object_path;
 
     DCHECK(bus_);
     dbus::ObjectProxy* adapter_proxy = bus_->GetObjectProxy(
@@ -292,8 +283,6 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
                    weak_ptr_factory_.GetWeakPtr(), object_path),
         base::Bind(&BluetoothAdapterClientImpl::DeviceDisappearedConnected,
                    weak_ptr_factory_.GetWeakPtr(), object_path));
-
-    return adapter_proxy;
   }
 
   // Removes the dbus object proxy for the adapter with dbus object path
@@ -332,7 +321,7 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
     VLOG(1) << object_path << ": PropertyChanged: Discovering = "
         << discovering;
 
-    FOR_EACH_OBSERVER(BluetoothAdapterClient::Observer, observers_,
+    FOR_EACH_OBSERVER(Observer, observers_,
                       DiscoveringPropertyChanged(object_path, discovering));
   }
 
@@ -367,8 +356,8 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
       return;
     }
 
-    FOR_EACH_OBSERVER(BluetoothAdapterClient::Observer, observers_,
-                      DeviceFound(object_path, address, device_properties));
+    FOR_EACH_OBSERVER(Observer, observers_, DeviceFound(object_path, address,
+                                                        device_properties));
   }
 
   // Called by dbus:: when the DeviceFound signal is initially connected.
@@ -393,8 +382,8 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
       return;
     }
     VLOG(1) << object_path << ": Device disappeared: " << address;
-    FOR_EACH_OBSERVER(BluetoothAdapterClient::Observer, observers_,
-                      DeviceDisappeared(object_path, address));
+    FOR_EACH_OBSERVER(Observer, observers_, DeviceDisappeared(object_path,
+                                                              address));
   }
 
   // Called by dbus:: when the DeviceDisappeared signal is initially connected.
@@ -431,7 +420,7 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
   ProxyMap proxy_map_;
 
   // List of observers interested in event notifications from us.
-  ObserverList<BluetoothAdapterClient::Observer> observers_;
+  ObserverList<Observer> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(BluetoothAdapterClientImpl);
 };
@@ -441,13 +430,14 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
 class BluetoothAdapterClientStubImpl : public BluetoothAdapterClient {
  public:
   // BluetoothAdapterClient override.
-  virtual void AddObserver(Observer* observer) {
-    VLOG(1) << "AddObserver";
+  virtual void AddObserver(Observer* observer, const std::string& object_path) {
+    VLOG(1) << "AddObserver: " << object_path;
   }
 
   // BluetoothAdapterClient override.
-  virtual void RemoveObserver(Observer* observer) {
-    VLOG(1) << "RemoveObserver";
+  virtual void RemoveObserver(Observer* observer,
+                              const std::string& object_path) {
+    VLOG(1) << "RemoveObserver: " << object_path;
   }
 
   // BluetoothAdapterClient override.
@@ -467,11 +457,9 @@ BluetoothAdapterClient::BluetoothAdapterClient() {
 BluetoothAdapterClient::~BluetoothAdapterClient() {
 }
 
-BluetoothAdapterClient* BluetoothAdapterClient::Create(
-    dbus::Bus* bus,
-    BluetoothManagerClient* manager_client) {
+BluetoothAdapterClient* BluetoothAdapterClient::Create(dbus::Bus* bus) {
   if (system::runtime_environment::IsRunningOnChromeOS()) {
-    return new BluetoothAdapterClientImpl(bus, manager_client);
+    return new BluetoothAdapterClientImpl(bus);
   } else {
     return new BluetoothAdapterClientStubImpl();
   }
diff --git a/chrome/browser/chromeos/dbus/bluetooth_adapter_client.h b/chrome/browser/chromeos/dbus/bluetooth_adapter_client.h
index 56079f4..73aaee2 100644
--- a/chrome/browser/chromeos/dbus/bluetooth_adapter_client.h
+++ b/chrome/browser/chromeos/dbus/bluetooth_adapter_client.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -18,8 +18,6 @@ class Bus;
 
 namespace chromeos {
 
-class BluetoothManagerClient;
-
 // BluetoothAdapterClient is used to communicate with a bluetooth Adapter
 // interface.
 class BluetoothAdapterClient {
@@ -48,8 +46,10 @@ class BluetoothAdapterClient {
 
   // Adds and removes observers for events on the adapter with object path
   // |object_path|.
-  virtual void AddObserver(Observer* observer) = 0;
-  virtual void RemoveObserver(Observer* observer) = 0;
+  virtual void AddObserver(Observer* observer,
+                           const std::string& object_path) = 0;
+  virtual void RemoveObserver(Observer* observer,
+                              const std::string& object_path) = 0;
 
   // Starts a device discovery on the adapter with object path |object_path|.
   virtual void StartDiscovery(const std::string& object_path) = 0;
@@ -59,8 +59,7 @@ class BluetoothAdapterClient {
   virtual void StopDiscovery(const std::string& object_path) = 0;
 
   // Creates the instance.
-  static BluetoothAdapterClient* Create(dbus::Bus* bus,
-                                        BluetoothManagerClient* manager_client);
+  static BluetoothAdapterClient* Create(dbus::Bus* bus);
 
  protected:
   BluetoothAdapterClient();
diff --git a/chrome/browser/chromeos/dbus/dbus_thread_manager.cc b/chrome/browser/chromeos/dbus/dbus_thread_manager.cc
index 25e3da5..8de57ba 100644
--- a/chrome/browser/chromeos/dbus/dbus_thread_manager.cc
+++ b/chrome/browser/chromeos/dbus/dbus_thread_manager.cc
@@ -54,7 +54,7 @@ class DBusThreadManagerImpl : public DBusThreadManager {
     bluetooth_manager_client_.reset(BluetoothManagerClient::Create(
         system_bus_.get()));
     bluetooth_adapter_client_.reset(BluetoothAdapterClient::Create(
-        system_bus_.get(), bluetooth_manager_client_.get()));
+        system_bus_.get()));
     // Create the cros-disks client.
     cros_disks_client_.reset(
         CrosDisksClient::Create(system_bus_.get()));