Delayed adding BluetoothEventRouter as BluetoothAdapter observer until when the

chrome/browser/extensions/bluetooth_event_router.cc

Index: chrome/browser/extensions/bluetooth_event_router.cc
diff --git a/chrome/browser/extensions/bluetooth_event_router.cc b/chrome/browser/extensions/bluetooth_event_router.cc
index d38db9e8b925a6438b7a199889ee2f576b1ea5dc..185685904f3b6d7c63adf459e4d568dc38349afc 100644
--- a/chrome/browser/extensions/bluetooth_event_router.cc
+++ b/chrome/browser/extensions/bluetooth_event_router.cc
@@ -29,8 +29,16 @@ ExtensionBluetoothEventRouter::ExtensionBluetoothEventRouter(Profile* profile)
       adapter_(device::BluetoothAdapterFactory::DefaultAdapter()),
       next_socket_id_(1) {
   DCHECK(profile_);
-  if (adapter_.get())
-    adapter_->AddObserver(this);
+}
+
+ExtensionBluetoothEventRouter::ExtensionBluetoothEventRouter(
+    Profile* profile, device::BluetoothAdapter* adapter)
+    : send_discovery_events_(false),
+      responsible_for_discovery_(false),
+      profile_(profile),
+      adapter_(adapter),
+      next_socket_id_(1) {
+  DCHECK(profile_);
 }
 
 ExtensionBluetoothEventRouter::~ExtensionBluetoothEventRouter() {
@@ -40,6 +48,13 @@ ExtensionBluetoothEventRouter::~ExtensionBluetoothEventRouter() {
   socket_map_.clear();
 }
 
+device::BluetoothAdapter* ExtensionBluetoothEventRouter::GetMutableAdapter() {
+  if (adapter_.get()) {
+    adapter_->AddObserver(this);

[miket_OOO, 2012/11/06 02:04:09]

Braces not needed.

I might answer this question for myself later in the review, but is it OK to
repeatedly add the observer if the caller calls GetMutableAdapter more than
once?

[youngki, 2012/11/06 16:18:54]

Done.

+  }
+  return adapter_.get();
+}
+
 int ExtensionBluetoothEventRouter::RegisterSocket(
     scoped_refptr<device::BluetoothSocket> socket) {
   // If there is a socket registered with the same fd, just return it's id
@@ -62,6 +77,21 @@ bool ExtensionBluetoothEventRouter::ReleaseSocket(int id) {
   return true;
 }
 
+bool ExtensionBluetoothEventRouter::IsBluetoothEvent(
+    const std::string& event_name) {

[miket_OOO, 2012/11/06 02:04:09]

this method could be const.

[youngki, 2012/11/06 16:18:54]

This method is actually static, so no need to be const.

+  return
+      event_name == extensions::event_names::kBluetoothOnAvailabilityChanged ||
+      event_name == extensions::event_names::kBluetoothOnDiscoveringChanged ||
+      event_name == extensions::event_names::kBluetoothOnPowerChanged;
+}
+
+void ExtensionBluetoothEventRouter::OnEventListenerAdded(
+    const std::string& event_name) {
+  if (IsBluetoothEvent(event_name) && adapter_.get()) {
+    adapter_->AddObserver(this);

[miket_OOO, 2012/11/06 02:04:09]

Would it be correct, or more elegant, to call GetMutableAdapter() here instead
of doing the get() check & AddObserver()?

[youngki, 2012/11/06 16:18:54]

Hm.. Although they do the same thing, we are not trying to get the mutable
adapter here; I think I will keep this implementation.

Also giving a second thought, I reverted my change on GetMutableAdapter(). It
looks wrong to add an observer when the purpose of the function is just to
return the GetMutableAdapter(). Also BluetoothEventRouter needs to add observers
only when the bluetooth event listeners are added anyways.

+  }
+}
+
 scoped_refptr<device::BluetoothSocket>
 ExtensionBluetoothEventRouter::GetSocket(int id) {
   SocketMap::iterator socket_entry = socket_map_.find(id);