Add Device Policy Handler for Bluetooth, and allow disabling the Bluetooth adapter on Chrome OS

device/bluetooth/bluetooth_adapter_bluez.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_bluez.h"
 
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 135 matching lines @@
                         base::Bind(&OnUnregisterAgentError));
 
   agent_.reset();
   dbus_is_shutdown_ = true;
 }
 
 BluetoothAdapterBlueZ::BluetoothAdapterBlueZ(const InitCallback& init_callback)
     : init_callback_(init_callback),
       initialized_(false),
       dbus_is_shutdown_(false),
+      is_disabled_(false),
       num_discovery_sessions_(0),
       discovery_request_pending_(false),
       weak_ptr_factory_(this) {
   ui_task_runner_ = base::ThreadTaskRunnerHandle::Get();
   socket_thread_ = device::BluetoothSocketThread::Get();
 
   // Can't initialize the adapter until DBus clients are ready.
   if (bluez::BluezDBusManager::Get()->IsObjectManagerSupportKnown()) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(&BluetoothAdapterBlueZ::Init,
@@ skipping 81 matching lines @@
           name,
           base::Bind(&BluetoothAdapterBlueZ::OnPropertyChangeCompleted,
                      weak_ptr_factory_.GetWeakPtr(), callback, error_callback));
 }
 
 bool BluetoothAdapterBlueZ::IsInitialized() const {
   return initialized_;
 }
 
 bool BluetoothAdapterBlueZ::IsPresent() const {
-  return !dbus_is_shutdown_ && !object_path_.value().empty();
+  return !is_disabled_ && !dbus_is_shutdown_ && !object_path_.value().empty();
+}
+
+void BluetoothAdapterBlueZ::SetDisabled(bool disabled) {
+  if (disabled) {
+    SetPowered(false, base::Bind(&base::DoNothing),

[Andrew T Wilson (Slow), 2016/04/13 13:55:11]

I am slightly nervous that any operations that are in progress when
SetDisabled() is called may continue - maybe this is fine, but would want to
make sure that the bluetooth folks are OK with how we're disabling the interface
here and that things like active discovery sessions will appropriately shut
down.

[Ivan Šandrk, 2016/04/14 18:08:47]

https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/b...

Methods tolerate a shutdown scenario where BluetoothAdapterBlueZ::Shutdown
causes IsPresent to return false just before the dbus system is shutdown but
while references to the BluetoothAdapterBlueZ object still exists.

So I'm assuming it's fine because I also use IsPresent. Would like to hear what
the Bluetooth folks have to say :)

[scheib, 2016/04/15 17:46:53]

At shutdown we believe things won't crash -- handlers should all simply early
exit. But, Shutdown was implemented with the assumption we'd never need to bring
things back to a working state. Shutdown also clears data structures, where
currently SetDisabled does not.

I'm tempted to request that the policy simply call 'Shutdown' and require a
restart to re-enable. This leaves the implementation much simpler, and would
seem to achieve the product goal. In almost all cases the device policy will be
set and devices will discover it at start up, and the device policy won't
change. The policy isn't expected to change frequently. If it does change,
requiring a restart doesn't seem so onerous. 

[Ivan Šandrk, 2016/04/15 17:52:28]

Okay thanks for your input. I'll ask my supervisors on Monday and see what they
think.

+                      base::Bind(&base::DoNothing));

[Andrew T Wilson (Slow), 2016/04/13 13:55:11]

This line is indented wrong.

[Ivan Šandrk, 2016/04/13 17:19:04]

Done.

+  }
+  is_disabled_ = disabled;
 }
 
 bool BluetoothAdapterBlueZ::IsPowered() const {
   if (!IsPresent())
     return false;
 
   bluez::BluetoothAdapterClient::Properties* properties =
       bluez::BluezDBusManager::Get()
           ->GetBluetoothAdapterClient()
           ->GetProperties(object_path_);
@@ skipping 58 matching lines @@
           ->GetProperties(object_path_);
 
   return properties->discovering.value();
 }
 
 void BluetoothAdapterBlueZ::CreateRfcommService(
     const BluetoothUUID& uuid,
     const ServiceOptions& options,
     const CreateServiceCallback& callback,
     const CreateServiceErrorCallback& error_callback) {
   DCHECK(!dbus_is_shutdown_);

[Andrew T Wilson (Slow), 2016/04/13 13:55:11]

Should this (and code below) also be checking if the device is present? Seems
like anyone can call CreateXXXXService() regardless of whether there's any
bluetooth adapter? Maybe add some DCHECKs() here and below?

[Ivan Šandrk, 2016/04/14 18:08:47]

That's a very good point.

Bluetooth guys, what do you think of this?

   VLOG(1) << object_path_.value()
           << ": Creating RFCOMM service: " << uuid.canonical_value();
   scoped_refptr<BluetoothSocketBlueZ> socket =
       BluetoothSocketBlueZ::CreateBluetoothSocket(ui_task_runner_,
                                                   socket_thread_);
   socket->Listen(this, BluetoothSocketBlueZ::kRfcomm, uuid, options,
                  base::Bind(callback, socket), error_callback);
 }
 
 void BluetoothAdapterBlueZ::CreateL2capService(
@@ skipping 1090 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 bluez