Bluetooth extension api fixes for WebUI

extensions/browser/api/bluetooth/bluetooth_event_router.cc

Index: extensions/browser/api/bluetooth/bluetooth_event_router.cc
diff --git a/extensions/browser/api/bluetooth/bluetooth_event_router.cc b/extensions/browser/api/bluetooth/bluetooth_event_router.cc
index 9302f7ce854fd71f0eb717fd5c39df87bf088886..09624a3d9772a53a5f2c8fabe538ec1d966b4246 100644
--- a/extensions/browser/api/bluetooth/bluetooth_event_router.cc
+++ b/extensions/browser/api/bluetooth/bluetooth_event_router.cc
@@ -37,7 +37,7 @@ namespace bt_private = api::bluetooth_private;
 
 BluetoothEventRouter::BluetoothEventRouter(content::BrowserContext* context)
     : browser_context_(context),
-      adapter_(NULL),
+      adapter_(nullptr),
       num_event_listeners_(0),
       extension_registry_observer_(this),
       weak_ptr_factory_(this) {
@@ -53,7 +53,7 @@ BluetoothEventRouter::~BluetoothEventRouter() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (adapter_.get()) {
     adapter_->RemoveObserver(this);
-    adapter_ = NULL;
+    adapter_ = nullptr;
   }
   CleanUpAllExtensions();
 }
@@ -78,7 +78,29 @@ void BluetoothEventRouter::StartDiscoverySession(
     const std::string& extension_id,
     const base::Closure& callback,
     const base::Closure& error_callback) {
-  if (adapter != adapter_.get()) {

[armansito, 2015/10/21 23:19:19]

Since you removed this check, then maybe add a DHECK(adapter == adapter_.get())
here?

[stevenjb, 2015/10/21 23:36:40]

Actually, the test is effectively moved to StartDiscoverySessionImpl below.

+  if (!adapter_.get() && IsBluetoothSupported()) {
+    GetAdapter(base::Bind(
+        &BluetoothEventRouter::OnAdapterInitialized,
+        weak_ptr_factory_.GetWeakPtr(),
+        base::Bind(&BluetoothEventRouter::StartDiscoverySessionImpl,
+                   weak_ptr_factory_.GetWeakPtr(), make_scoped_refptr(adapter),
+                   extension_id, callback, error_callback)));
+    return;
+  }
+  StartDiscoverySessionImpl(adapter, extension_id, callback, error_callback);
+}
+
+void BluetoothEventRouter::StartDiscoverySessionImpl(
+    device::BluetoothAdapter* adapter,
+    const std::string& extension_id,
+    const base::Closure& callback,
+    const base::Closure& error_callback) {
+  if (!adapter_.get() || adapter != adapter_.get()) {

[armansito, 2015/10/21 23:19:19]

You removed the "adapter != adapter_.get()" above but kept it here. Let's be
consistent.

[stevenjb, 2015/10/21 23:36:40]

This is called only from StartDiscoverySession() (thus
StartDiscoverySessionImpl()). I added a clarifying comment.

+    if (adapter_.get()) {
+      LOG(ERROR) << "Unable to get addapter";

[armansito, 2015/10/21 23:19:19]

s/addapter/adapter/

[stevenjb, 2015/10/21 23:36:40]

Done.

+    } else {
+      LOG(ERROR) << "Adapter mismatch";

[armansito, 2015/10/21 23:19:19]

Also this logic is extremely convoluted for a mostly uninformative log message.
Just simplify it all entirely like this:

if (!adapter_.get()) {
  LOG(ERROR) << "|adapter_| was not initialized".
  error_callback.Run();
  return;
}

if (adapter != adapter_.get()) {
  LOG(ERROR) << "Adapters don't match";
  error_callback.Run();
  return;
}

Again for the second part, I'm not sure if it's better to just have
DCHECK(adapter == adapter_.get());

[stevenjb, 2015/10/21 23:36:40]

Logging clarified.

+    }
     error_callback.Run();
     return;
   }
@@ -161,7 +183,7 @@ BluetoothApiPairingDelegate* BluetoothEventRouter::GetPairingDelegate(
     const std::string& extension_id) {
   return ContainsKey(pairing_delegate_map_, extension_id)
              ? pairing_delegate_map_[extension_id]
-             : NULL;
+             : nullptr;
 }
 
 void BluetoothEventRouter::OnAdapterInitialized(
@@ -178,23 +200,30 @@ void BluetoothEventRouter::OnAdapterInitialized(
 void BluetoothEventRouter::MaybeReleaseAdapter() {
   if (adapter_.get() && num_event_listeners_ == 0 &&
       pairing_delegate_map_.empty()) {
+    VLOG(1) << "Releasing Adapter.";
     adapter_->RemoveObserver(this);
-    adapter_ = NULL;
+    adapter_ = nullptr;
   }
 }
 
 void BluetoothEventRouter::AddPairingDelegate(const std::string& extension_id) {
-  if (!adapter_.get()) {
-    base::Closure self_callback =
-        base::Bind(&BluetoothEventRouter::AddPairingDelegate,
+  if (!adapter_.get() && IsBluetoothSupported()) {
+    GetAdapter(
+        base::Bind(&BluetoothEventRouter::OnAdapterInitialized,
                    weak_ptr_factory_.GetWeakPtr(),
-                   extension_id);
-    GetAdapter(base::Bind(&BluetoothEventRouter::OnAdapterInitialized,
-                          weak_ptr_factory_.GetWeakPtr(),
-                          self_callback));
+                   base::Bind(&BluetoothEventRouter::AddPairingDelegateImpl,
+                              weak_ptr_factory_.GetWeakPtr(), extension_id)));
     return;
   }
+  AddPairingDelegateImpl(extension_id);
+}
 
+void BluetoothEventRouter::AddPairingDelegateImpl(
+    const std::string& extension_id) {
+  if (!adapter_.get()) {
+    LOG(ERROR) << "Unable to get adatper.";
+    return;
+  }
   if (!ContainsKey(pairing_delegate_map_, extension_id)) {
     BluetoothApiPairingDelegate* delegate =
         new BluetoothApiPairingDelegate(extension_id, browser_context_);
@@ -266,10 +295,13 @@ void BluetoothEventRouter::AdapterDiscoveringChanged(
       delete session;
     }
     discovery_session_map_.swap(active_session_map);
-    MaybeReleaseAdapter();
   }
 
   DispatchAdapterStateEvent();
+
+  // Release the adapter after dispatching the event.
+  if (!discovering)
+    MaybeReleaseAdapter();
 }
 
 void BluetoothEventRouter::DeviceAdded(device::BluetoothAdapter* adapter,
@@ -327,6 +359,7 @@ void BluetoothEventRouter::OnListenerRemoved() {
 void BluetoothEventRouter::DispatchAdapterStateEvent() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   api::bluetooth::AdapterState state;
+  CHECK(adapter_.get());
   PopulateAdapterState(*adapter_.get(), &state);
 
   scoped_ptr<base::ListValue> args =
@@ -342,6 +375,7 @@ void BluetoothEventRouter::DispatchDeviceEvent(
     const std::string& event_name,
     device::BluetoothDevice* device) {
   bluetooth::Device extension_device;
+  CHECK(device);
   bluetooth::BluetoothDeviceToApiDevice(*device, &extension_device);
 
   scoped_ptr<base::ListValue> args =