bluetooth: Standardize Bluetooth adapter access in Adapter service.

device/bluetooth/adapter.cc

 // Copyright 2016 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 <string>
 #include <utility>
 #include <vector>
 
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 12 matching lines @@
     adapter_ = nullptr;
   }
 }
 
 // static
 void Adapter::Create(mojom::AdapterRequest request) {
   mojo::MakeStrongBinding(base::MakeUnique<Adapter>(), std::move(request));
 }
 
 void Adapter::GetDevices(const GetDevicesCallback& callback) {
-  if (!adapter_.get()) {
-    if (device::BluetoothAdapterFactory::IsBluetoothAdapterAvailable()) {
-      base::Closure c = base::Bind(&Adapter::GetDevicesImpl,
-                                   weak_ptr_factory_.GetWeakPtr(), callback);
+  WithAdapter(base::Bind(

[ortuno, 2016/09/29 22:55:54]

We can't use Lambda Expressions if the lambda expression is called outside the
stack frame of the function.
Source: https://chromium-cpp.appspot.com/

[mbrunson, 2016/09/30 00:15:07]

Doesn't this only apply to capturing lambdas? This lambda doesn't capture any
variables.

[scheib, 2016/09/30 06:35:42]

I /think/ this is OK, as mbrunson states because it is not capturing. The ::Bind
is currying the 'callback' parameter and managing its lifetime.

If we can get away with this, it will make the many methods of Adapter:: much
easier to read.

[scheib, 2016/09/30 06:50:27]

I've asked the lambda thread for thoughts.
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/D9UnnxBnciQ/nrQGE...

[scheib, 2016/09/30 21:52:01]

Thread approved, citing previous thread, and style guide is updated with this
pattern explicitly included.

+      [](const GetDevicesCallback& callback,
+         scoped_refptr<device::BluetoothAdapter> adapter) {
+        if (adapter) {
+          std::vector<mojom::DeviceInfoPtr> devices;
 
-      device::BluetoothAdapterFactory::GetAdapter(base::Bind(
-          &Adapter::OnGetAdapter, weak_ptr_factory_.GetWeakPtr(), c));
-      return;
-    }
-    callback.Run(std::vector<mojom::DeviceInfoPtr>());
-    return;
-  }
-  GetDevicesImpl(callback);
+          for (const device::BluetoothDevice* device : adapter->GetDevices()) {
+            mojom::DeviceInfoPtr device_info =
+                ConstructDeviceInfoStruct(device);
+            devices.push_back(std::move(device_info));
+          }
+
+          callback.Run(std::move(devices));
+        } else {
+          callback.Run(std::vector<mojom::DeviceInfoPtr>());
+        }
+      },
+      callback));
 }
 
 void Adapter::SetClient(mojom::AdapterClientPtr client) {
   client_ = std::move(client);
 }
 
 void Adapter::DeviceAdded(device::BluetoothAdapter* adapter,
                           device::BluetoothDevice* device) {
   if (client_) {
     auto device_info = ConstructDeviceInfoStruct(device);
     client_->DeviceAdded(std::move(device_info));
   }
 }
 
 void Adapter::DeviceRemoved(device::BluetoothAdapter* adapter,
                             device::BluetoothDevice* device) {
   if (client_) {
     auto device_info = ConstructDeviceInfoStruct(device);
     client_->DeviceRemoved(std::move(device_info));
   }
 }
 
+// static
 mojom::DeviceInfoPtr Adapter::ConstructDeviceInfoStruct(
-    const device::BluetoothDevice* device) const {
+    const device::BluetoothDevice* const device) {
   mojom::DeviceInfoPtr device_info = mojom::DeviceInfo::New();
 
   device_info->name = device->GetName();
   device_info->name_for_display =
       base::UTF16ToUTF8(device->GetNameForDisplay());
   device_info->id = device->GetIdentifier();
   device_info->address = device->GetAddress();
 
   return device_info;
 }
 
-void Adapter::GetDevicesImpl(const GetDevicesCallback& callback) {
-  std::vector<mojom::DeviceInfoPtr> devices;
-
-  for (const device::BluetoothDevice* device : adapter_->GetDevices()) {
-    mojom::DeviceInfoPtr device_info = ConstructDeviceInfoStruct(device);
-    devices.push_back(std::move(device_info));
+void Adapter::WithAdapter(
+    const device::BluetoothAdapterFactory::AdapterCallback& action) {
+  if (!adapter_.get()) {

[scheib, 2016/09/30 06:35:42]

Keep logic simple if both if and else are run:
if (adapter_.get() { action.Run(Adapter_); return; }

[mbrunson, 2016/09/30 21:14:38]

Done.

+    if (device::BluetoothAdapterFactory::IsBluetoothAdapterAvailable()) {

[scheib, 2016/09/30 06:35:42]

If the adapter isn't available run the action with nullptr. In all methods above
we'll need to check and handle that case.

[mbrunson, 2016/09/30 21:14:38]

Done.

+      device::BluetoothAdapterFactory::GetAdapter(base::Bind(
+          &Adapter::OnGetAdapter, weak_ptr_factory_.GetWeakPtr(), action));
+      return;
+    }
   }
-
-  callback.Run(std::move(devices));
+  action.Run(adapter_);
 }
 
-void Adapter::OnGetAdapter(const base::Closure& continuation,
-                           scoped_refptr<device::BluetoothAdapter> adapter) {
+void Adapter::OnGetAdapter(
+    const device::BluetoothAdapterFactory::AdapterCallback& continuation,
+    scoped_refptr<device::BluetoothAdapter> adapter) {
   if (!adapter_.get()) {
     VLOG(1) << "Adapter acquired";
     adapter_ = adapter;
     adapter_->AddObserver(this);
   }
-  continuation.Run();
+  continuation.Run(adapter);
 }
 
 }  // namespace bluetooth