arc: bluetooth: Add/Remove BT observer only when ARC is ready

components/arc/bluetooth/arc_bluetooth_bridge.cc

Index: components/arc/bluetooth/arc_bluetooth_bridge.cc
diff --git a/components/arc/bluetooth/arc_bluetooth_bridge.cc b/components/arc/bluetooth/arc_bluetooth_bridge.cc
index dbc6747b662299696c5c1ebb56d1a9f069e3b04c..dbe73b9f903d39aba55c5d85dee4d63793ee1310 100644
--- a/components/arc/bluetooth/arc_bluetooth_bridge.cc
+++ b/components/arc/bluetooth/arc_bluetooth_bridge.cc
@@ -180,13 +180,12 @@ namespace arc {
 ArcBluetoothBridge::ArcBluetoothBridge(ArcBridgeService* bridge_service)
     : ArcService(bridge_service), binding_(this), weak_factory_(this) {
   if (BluetoothAdapterFactory::IsBluetoothAdapterAvailable()) {

[rkc, 2016/08/08 21:58:29]

This is a useless check. This only checks if Bluetooth is available on this
'platform'. We won't be running ARC++ ever on a platform that doesn't support
Bluetooth (currently only IOS?).

[puthik_chromium, 2016/08/08 22:33:32]

It's out of scope of this CL so I will leave it here.

-    VLOG(1) << "registering bluetooth adapter";
+    VLOG(1) << "Registering bluetooth adapter.";
     BluetoothAdapterFactory::GetAdapter(base::Bind(
         &ArcBluetoothBridge::OnAdapterInitialized, weak_factory_.GetWeakPtr()));
   } else {
-    VLOG(1) << "no bluetooth adapter available";
+    VLOG(1) << "No bluetooth adapter available.";
   }
-
   arc_bridge_service()->bluetooth()->AddObserver(this);
 }
 
@@ -207,7 +206,14 @@ void ArcBluetoothBridge::OnAdapterInitialized(
   // so our adapter uses BlueZ.
   bluetooth_adapter_ =
       static_cast<bluez::BluetoothAdapterBlueZ*>(adapter.get());
-  bluetooth_adapter_->AddObserver(this);
+
+  // Arc instance is ready before Bluetooth adapter.
+  // We should register ourselves to bluetooth observer here.
+  if (!bluetooth_observer_flag_ &&

[Luis Héctor Chávez, 2016/08/08 21:54:16]

s/bluetooth_observer_flag_/bluetooth_adapter_->HasObserver(this)/

across all the file.

[puthik_chromium, 2016/08/08 22:33:32]

There is no HasObserver(this) for bluetooth_adapter_.

[rkc, 2016/08/08 22:36:32]

Yes, it is trivial to add it though.

[puthik_chromium, 2016/08/08 22:46:51]

Done.

+      arc_bridge_service()->bluetooth()->instance()) {
+    bluetooth_observer_flag_ = true;
+    bluetooth_adapter_->AddObserver(this);
+  }
 }
 
 void ArcBluetoothBridge::OnInstanceReady() {
@@ -218,7 +224,21 @@ void ArcBluetoothBridge::OnInstanceReady() {
                << "but no bluetooth instance found";
     return;
   }
+
   bluetooth_instance->Init(binding_.CreateInterfacePtrAndBind());
+
+  // Bluetooth adapter is ready before Arc instance.
+  // We should register ourselves to bluetooth observer here.

[rkc, 2016/08/08 21:58:29]

This comment isn't clear. Instead, it should read something like,

The Bluetooth adapter was ready before the ARC instance, hence we didn't
register ourselves as an observer with it then. Since our instance is ready, we
should register it now.

[puthik_chromium, 2016/08/08 22:33:32]

Done.

+  if (!bluetooth_observer_flag_ && bluetooth_adapter_) {
+    bluetooth_observer_flag_ = true;
+    bluetooth_adapter_->AddObserver(this);
+  }
+}
+
+void ArcBluetoothBridge::OnInstanceClosed() {
+  if (bluetooth_observer_flag_ && bluetooth_adapter_)
+    bluetooth_adapter_->RemoveObserver(this);
+  bluetooth_observer_flag_ = false;
 }
 
 void ArcBluetoothBridge::AdapterPoweredChanged(BluetoothAdapter* adapter,