Fix BluetoothAdapterProfileChromeOS lifecycle management

device/bluetooth/bluetooth_adapter_chromeos.cc

 // Copyright 2013 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.
 
 #include "device/bluetooth/bluetooth_adapter_chromeos.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/location.h"
@@ skipping 946 matching lines @@
 
   if (profiles_.find(uuid) != profiles_.end()) {
     // TODO(jamuraa) check that the options are the same and error when they are
     // not.
     SetProfileDelegate(uuid, device_path, delegate, success_callback,
                        error_callback);
     return;
   }
 
   profiles_[uuid] = BluetoothAdapterProfileChromeOS::Register(
-      this, uuid, options,
+      this->weak_ptr_factory_.GetWeakPtr(), uuid, options,
       base::Bind(&BluetoothAdapterChromeOS::OnRegisterProfile, this, uuid,
                  device_path, delegate, success_callback, error_callback),
       base::Bind(&BluetoothAdapterChromeOS::OnRegisterProfileError, this, uuid,
                  error_callback));
 }
 
-void BluetoothAdapterChromeOS::ReleaseProfile(const BluetoothUUID& uuid) {
+void BluetoothAdapterChromeOS::RemoveProfile(const BluetoothUUID& uuid) {
   if (profiles_.find(uuid) != profiles_.end()) {

[Ilya Sherman, 2015/02/19 20:37:58]

nit: I don't think you need the if stmt at all anymore -- erase() should be safe
to call even if the key isn't present.  (Though, I'm not 100% sure on this, so
please check that I'm not telling you lies =)

[Marie Janssen, 2015/02/20 22:53:46]

Done.

-    delete profiles_[uuid];
     profiles_.erase(uuid);

[Ilya Sherman, 2015/02/19 20:37:58]

What code is responsible for freeing the memory for the profile, in this case? 
The ownership semantics for profiles are not clear to me.

[Marie Janssen, 2015/02/20 22:53:46]

Profiles are now owned by BluetoothAdapterChromeOS and released through there.

   }
 }
 
 void BluetoothAdapterChromeOS::OnRegisterProfile(
     const BluetoothUUID& uuid,
     const dbus::ObjectPath& device_path,
     BluetoothProfileServiceProvider::Delegate* delegate,
     const ProfileRegisteredCallback& success_callback,
     const ErrorCompletionCallback& error_callback) {
   SetProfileDelegate(uuid, device_path, delegate, success_callback,
@@ skipping 16 matching lines @@
 }
 
 void BluetoothAdapterChromeOS::OnRegisterProfileError(
     const BluetoothUUID& uuid,
     const ErrorCompletionCallback& error_callback,
     const std::string& error_name,
     const std::string& error_message) {
   LOG(WARNING) << object_path_.value()
                << ": Failed to register profile: " << error_name << ": "
                << error_message;
+  delete profiles_[uuid];

[Ilya Sherman, 2015/02/19 20:37:58]

Are you sure it's appropriate to do this *before* clients have a chance to
handle the error callback?

[Marie Janssen, 2015/02/20 22:53:46]

Sockets don't get this pointer until the RegisterProfile callback completes, so
this is okay here.

   error_callback.Run(error_message);
-  ReleaseProfile(uuid);
 }
 
 void BluetoothAdapterChromeOS::OnSetDiscoverable(
     const base::Closure& callback,
     const ErrorCallback& error_callback,
     bool success) {
   DCHECK(IsPresent());
   // Set the discoverable_timeout property to zero so the adapter remains
   // discoverable forever.
   DBusThreadManager::Get()->GetBluetoothAdapterClient()->
@@ skipping 191 matching lines @@
 
     // If the queued request resulted in a pending call, then let it
     // asynchonously process the remaining queued requests once the pending
     // call returns.
     if (discovery_request_pending_)
       return;
   }
 }
 
 }  // namespace chromeos