Unregister Bluetooth profile when the extension that added it is unloaded.

BUG=332177
TEST=manually and new unit test

Review URL: https://codereview.chromium.org/128523003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@244000 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/api/bluetooth/bluetooth_api.cc

@@ -194,7 +194,8 @@ void BluetoothAddProfileFunction::OnProfileRegistered(
                  base::Unretained(GetEventRouter(GetProfile())),
                  extension_id(),
                  uuid_));
-  GetEventRouter(GetProfile())->AddProfile(uuid_, bluetooth_profile);
+  GetEventRouter(GetProfile())->AddProfile(
+      uuid_, extension_id(), bluetooth_profile);
   SendResponse(true);
 }
 

chrome/browser/extensions/api/bluetooth/bluetooth_apitest.cc

@@ -500,7 +500,7 @@ IN_PROC_BROWSER_TEST_F(BluetoothApiTest, OnConnection) {
   scoped_refptr<device::MockBluetoothSocket> socket =
       new device::MockBluetoothSocket();
 
-  event_router()->AddProfile("1234", profile1_.get());
+  event_router()->AddProfile("1234", extension->id(), profile1_.get());
   event_router()->DispatchConnectionEvent(
       extension->id(), "1234", device1_.get(), socket);
 

chrome/browser/extensions/api/bluetooth/bluetooth_event_router.cc

@@ -13,10 +13,13 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_vector.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/bluetooth/bluetooth_api_utils.h"
 #include "chrome/browser/extensions/event_names.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/common/extensions/api/bluetooth.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_source.h"
 #include "device/bluetooth/bluetooth_adapter.h"
 #include "device/bluetooth/bluetooth_adapter_factory.h"
 #include "device/bluetooth/bluetooth_device.h"
@@ -28,6 +31,18 @@ namespace extensions {
 
 namespace bluetooth = api::bluetooth;
 
+// A struct storing a Bluetooth socket and the extension that added it.
+struct ExtensionBluetoothSocketRecord {
+  std::string extension_id;
+  scoped_refptr<device::BluetoothSocket> socket;
+};
+
+// A struct storing a Bluetooth profile and the extension that added it.
+struct ExtensionBluetoothProfileRecord {
+  std::string extension_id;
+  device::BluetoothProfile* profile;
+};
+
 ExtensionBluetoothEventRouter::ExtensionBluetoothEventRouter(Profile* profile)
     : send_discovery_events_(false),
       responsible_for_discovery_(false),
@@ -37,6 +52,8 @@ ExtensionBluetoothEventRouter::ExtensionBluetoothEventRouter(Profile* profile)
       next_socket_id_(1),
       weak_ptr_factory_(this) {
   DCHECK(profile_);
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
+                 content::Source<Profile>(profile_));
 }
 
 ExtensionBluetoothEventRouter::~ExtensionBluetoothEventRouter() {
@@ -51,7 +68,7 @@ ExtensionBluetoothEventRouter::~ExtensionBluetoothEventRouter() {
   for (BluetoothProfileMap::iterator iter = bluetooth_profile_map_.begin();
        iter != bluetooth_profile_map_.end();
        ++iter) {
-    iter->second->Unregister();
+    iter->second.profile->Unregister();
   }
 }
 
@@ -82,15 +99,17 @@ void ExtensionBluetoothEventRouter::OnListenerRemoved() {
 }
 
 int ExtensionBluetoothEventRouter::RegisterSocket(
+    const std::string& extension_id,
     scoped_refptr<device::BluetoothSocket> socket) {
   // If there is a socket registered with the same fd, just return it's id
   for (SocketMap::const_iterator i = socket_map_.begin();
       i != socket_map_.end(); ++i) {
-    if (i->second.get() == socket.get())
+    if (i->second.socket.get() == socket.get())
       return i->first;
   }
   int return_id = next_socket_id_++;
-  socket_map_[return_id] = socket;
+  ExtensionBluetoothSocketRecord record = { extension_id, socket };
+  socket_map_[return_id] = record;
   return return_id;
 }
 
@@ -104,15 +123,17 @@ bool ExtensionBluetoothEventRouter::ReleaseSocket(int id) {
 
 void ExtensionBluetoothEventRouter::AddProfile(
     const std::string& uuid,
+    const std::string& extension_id,
     device::BluetoothProfile* bluetooth_profile) {
   DCHECK(!HasProfile(uuid));
-  bluetooth_profile_map_[uuid] = bluetooth_profile;
+  ExtensionBluetoothProfileRecord record = { extension_id, bluetooth_profile };
+  bluetooth_profile_map_[uuid] = record;
 }
 
 void ExtensionBluetoothEventRouter::RemoveProfile(const std::string& uuid) {
   BluetoothProfileMap::iterator iter = bluetooth_profile_map_.find(uuid);
   if (iter != bluetooth_profile_map_.end()) {
-    device::BluetoothProfile* bluetooth_profile = iter->second;
+    device::BluetoothProfile* bluetooth_profile = iter->second.profile;
     bluetooth_profile_map_.erase(iter);
     bluetooth_profile->Unregister();
   }
@@ -126,7 +147,7 @@ device::BluetoothProfile* ExtensionBluetoothEventRouter::GetProfile(
     const std::string& uuid) const {
   BluetoothProfileMap::const_iterator iter = bluetooth_profile_map_.find(uuid);
   if (iter != bluetooth_profile_map_.end())
-    return iter->second;
+    return iter->second.profile;
 
   return NULL;
 }
@@ -136,7 +157,7 @@ ExtensionBluetoothEventRouter::GetSocket(int id) {
   SocketMap::iterator socket_entry = socket_map_.find(id);
   if (socket_entry == socket_map_.end())
     return NULL;
-  return socket_entry->second;
+  return socket_entry->second.socket;;
 }
 
 void ExtensionBluetoothEventRouter::SetResponsibleForDiscovery(
@@ -178,7 +199,7 @@ void ExtensionBluetoothEventRouter::DispatchConnectionEvent(
   if (!HasProfile(uuid))
     return;
 
-  int socket_id = RegisterSocket(socket);
+  int socket_id = RegisterSocket(extension_id, socket);
   api::bluetooth::Socket result_socket;
   api::bluetooth::BluetoothDeviceToApiDevice(*device, &result_socket.device);
   result_socket.profile.uuid = uuid;
@@ -281,4 +302,44 @@ void ExtensionBluetoothEventRouter::DispatchAdapterStateEvent() {
   ExtensionSystem::Get(profile_)->event_router()->BroadcastEvent(event.Pass());
 }
 
+void ExtensionBluetoothEventRouter::CleanUpForExtension(
+    const std::string& extension_id) {
+  // Remove all profiles added by the extension.
+  BluetoothProfileMap::iterator profile_iter = bluetooth_profile_map_.begin();
+  while (profile_iter != bluetooth_profile_map_.end()) {
+    ExtensionBluetoothProfileRecord record = profile_iter->second;
+    if (record.extension_id == extension_id) {
+      bluetooth_profile_map_.erase(profile_iter++);
+      record.profile->Unregister();
+    } else {
+      profile_iter++;
+    }
+  }
+
+  // Remove all sockets opened by the extension.
+  SocketMap::iterator socket_iter = socket_map_.begin();
+  while (socket_iter != socket_map_.end()) {
+    int socket_id = socket_iter->first;
+    ExtensionBluetoothSocketRecord record = socket_iter->second;
+    socket_iter++;
+    if (record.extension_id == extension_id) {
+      ReleaseSocket(socket_id);
+    }
+  }
+}
+
+void ExtensionBluetoothEventRouter::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  switch (type) {
+    case chrome::NOTIFICATION_EXTENSION_UNLOADED: {
+      extensions::UnloadedExtensionInfo* info =
+          content::Details<extensions::UnloadedExtensionInfo>(details).ptr();
+      CleanUpForExtension(info->extension->id());
+      break;
+    }
+  }
+}
+
 }  // namespace extensions

chrome/browser/extensions/api/bluetooth/bluetooth_event_router.h

@@ -11,6 +11,8 @@
 #include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/common/extensions/api/bluetooth.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
 #include "device/bluetooth/bluetooth_adapter.h"
 #include "device/bluetooth/bluetooth_adapter_factory.h"
 #include "device/bluetooth/bluetooth_socket.h"
@@ -26,8 +28,13 @@ class BluetoothProfile;
 
 namespace extensions {
 
+// Foward declarations of internal structs.
+struct ExtensionBluetoothSocketRecord;
+struct ExtensionBluetoothProfileRecord;
+
 class ExtensionBluetoothEventRouter
-    : public device::BluetoothAdapter::Observer {
+    : public device::BluetoothAdapter::Observer,
+      public content::NotificationObserver {
  public:
   explicit ExtensionBluetoothEventRouter(Profile* profile);
   virtual ~ExtensionBluetoothEventRouter();
@@ -46,18 +53,22 @@ class ExtensionBluetoothEventRouter
   void OnListenerRemoved();
 
   // Register the BluetoothSocket |socket| for use by the extensions system.
-  // This class will hold onto the socket for its lifetime, or until
-  // ReleaseSocket is called for the socket.  Returns an id for the socket.
-  int RegisterSocket(scoped_refptr<device::BluetoothSocket> socket);
+  // This class will hold onto the socket for its lifetime until
+  // ReleaseSocket is called for the socket, or until the extension associated
+  // with the socket is disabled/ reloaded. Returns an id for the socket.
+  int RegisterSocket(const std::string& extension_id,
+                     scoped_refptr<device::BluetoothSocket> socket);
 
   // Release the BluetoothSocket corresponding to |id|.  Returns true if
   // the socket was found and released, false otherwise.
   bool ReleaseSocket(int id);
 
   // Add the BluetoothProfile |bluetooth_profile| for use by the extension
-  // system. This class will hold onto the profile for its lifetime, or until
-  // RemoveProfile is called for the profile.
+  // system. This class will hold onto the profile until RemoveProfile is
+  // called for the profile, or until the extension that added the profile
+  // is disabled/reloaded.
   void AddProfile(const std::string& uuid,
+                  const std::string& extension_id,
                   device::BluetoothProfile* bluetooth_profile);
 
   // Unregister the BluetoothProfile corersponding to |uuid| and release the
@@ -105,6 +116,11 @@ class ExtensionBluetoothEventRouter
   virtual void DeviceAdded(device::BluetoothAdapter* adapter,
                            device::BluetoothDevice* device) OVERRIDE;
 
+  // Overridden from content::NotificationObserver
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
   // Exposed for testing.
   void SetAdapterForTest(device::BluetoothAdapter* adapter) {
     adapter_ = adapter;
@@ -114,6 +130,7 @@ class ExtensionBluetoothEventRouter
   void InitializeAdapter(scoped_refptr<device::BluetoothAdapter> adapter);
   void MaybeReleaseAdapter();
   void DispatchAdapterStateEvent();
+  void CleanUpForExtension(const std::string& extension_id);
 
   bool send_discovery_events_;
   bool responsible_for_discovery_;
@@ -128,17 +145,20 @@ class ExtensionBluetoothEventRouter
   // the extension javascript.
   int next_socket_id_;
 
-  typedef std::map<int, scoped_refptr<device::BluetoothSocket> > SocketMap;
+  typedef std::map<int, ExtensionBluetoothSocketRecord> SocketMap;
   SocketMap socket_map_;
 
   typedef ScopedVector<extensions::api::bluetooth::Device>
       DeviceList;
   DeviceList discovered_devices_;
 
-  // A map that maps uuids to the BluetoothProfile objects.
-  typedef std::map<std::string, device::BluetoothProfile*> BluetoothProfileMap;
+  // A map that maps uuids to ExtensionBluetoothProfileRecord.
+  typedef std::map<std::string, ExtensionBluetoothProfileRecord>
+      BluetoothProfileMap;
   BluetoothProfileMap bluetooth_profile_map_;
 
+  content::NotificationRegistrar registrar_;
+
   base::WeakPtrFactory<ExtensionBluetoothEventRouter> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionBluetoothEventRouter);

chrome/browser/extensions/api/bluetooth/bluetooth_event_router_unittest.cc

@@ -8,23 +8,27 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/bluetooth/bluetooth_event_router.h"
 #include "chrome/browser/extensions/event_names.h"
 #include "chrome/browser/extensions/extension_system_factory.h"
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/common/extensions/api/bluetooth.h"
 #include "chrome/test/base/testing_profile.h"
+#include "content/public/browser/notification_service.h"
 #include "content/public/test/test_browser_thread.h"
 #include "device/bluetooth/test/mock_bluetooth_adapter.h"
 #include "device/bluetooth/test/mock_bluetooth_device.h"
 #include "device/bluetooth/test/mock_bluetooth_profile.h"
 #include "device/bluetooth/test/mock_bluetooth_socket.h"
 #include "extensions/browser/event_router.h"
+#include "extensions/common/extension_builder.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
+const char kTestExtensionId[] = "test extension id";
 const char kAudioProfileUuid[] = "audio profile uuid";
 const char kHealthProfileUuid[] = "health profile uuid";
 
@@ -131,8 +135,10 @@ TEST_F(ExtensionBluetoothEventRouterTest, Profiles) {
   EXPECT_FALSE(router_.HasProfile(kAudioProfileUuid));
   EXPECT_FALSE(router_.HasProfile(kHealthProfileUuid));
 
-  router_.AddProfile(kAudioProfileUuid, &mock_audio_profile_);
-  router_.AddProfile(kHealthProfileUuid, &mock_health_profile_);
+  router_.AddProfile(
+      kAudioProfileUuid, kTestExtensionId, &mock_audio_profile_);
+  router_.AddProfile(
+      kHealthProfileUuid, kTestExtensionId, &mock_health_profile_);
   EXPECT_TRUE(router_.HasProfile(kAudioProfileUuid));
   EXPECT_TRUE(router_.HasProfile(kHealthProfileUuid));
 
@@ -146,21 +152,55 @@ TEST_F(ExtensionBluetoothEventRouterTest, Profiles) {
   EXPECT_CALL(*mock_adapter_, RemoveObserver(testing::_)).Times(1);
 }
 
+TEST_F(ExtensionBluetoothEventRouterTest, UnloadExtension) {
+  scoped_refptr<const extensions::Extension> extension =
+      extensions::ExtensionBuilder()
+          .SetManifest(extensions::DictionaryBuilder()
+              .Set("name", "BT event router test")
+              .Set("version", "1.0")
+              .Set("manifest_version", 2))
+          .SetID(kTestExtensionId)
+          .Build();
+
+  router_.AddProfile(
+      kAudioProfileUuid, kTestExtensionId, &mock_audio_profile_);
+  router_.AddProfile(
+      kHealthProfileUuid, kTestExtensionId, &mock_health_profile_);
+  EXPECT_TRUE(router_.HasProfile(kAudioProfileUuid));
+  EXPECT_TRUE(router_.HasProfile(kHealthProfileUuid));
+
+  // Unloading the extension should unregister all profiles added by it.
+  EXPECT_CALL(mock_audio_profile_, Unregister()).Times(1);
+  EXPECT_CALL(mock_health_profile_, Unregister()).Times(1);
+
+  content::NotificationService* notifier =
+      content::NotificationService::current();
+  UnloadedExtensionInfo details(
+      extension, UnloadedExtensionInfo::REASON_DISABLE);
+  notifier->Notify(chrome::NOTIFICATION_EXTENSION_UNLOADED,
+                   content::Source<Profile>(test_profile_.get()),
+                   content::Details<UnloadedExtensionInfo>(&details));
+
+  EXPECT_FALSE(router_.HasProfile(kAudioProfileUuid));
+  EXPECT_FALSE(router_.HasProfile(kHealthProfileUuid));
+  EXPECT_CALL(*mock_adapter_, RemoveObserver(testing::_)).Times(1);
+}
+
 TEST_F(ExtensionBluetoothEventRouterTest, DispatchConnectionEvent) {
-  router_.AddProfile(kAudioProfileUuid, &mock_audio_profile_);
+  router_.AddProfile(
+      kAudioProfileUuid, kTestExtensionId, &mock_audio_profile_);
 
   FakeExtensionSystem* fake_extension_system =
       static_cast<FakeExtensionSystem*>(ExtensionSystemFactory::GetInstance()->
           SetTestingFactoryAndUse(test_profile_.get(),
                                   &BuildFakeExtensionSystem));
 
-  const char test_extension_id[] = "test extension id";
   testing::NiceMock<device::MockBluetoothDevice> mock_device(
       mock_adapter_, 0, "device name", "device address", true, false);
   scoped_refptr<testing::NiceMock<device::MockBluetoothSocket> > mock_socket(
       new testing::NiceMock<device::MockBluetoothSocket>());
 
-  router_.DispatchConnectionEvent(test_extension_id,
+  router_.DispatchConnectionEvent(kTestExtensionId,
                                   kAudioProfileUuid,
                                   &mock_device,
                                   mock_socket);
@@ -168,7 +208,7 @@ TEST_F(ExtensionBluetoothEventRouterTest, DispatchConnectionEvent) {
   FakeEventRouter* fake_event_router =
       static_cast<FakeEventRouter*>(fake_extension_system->event_router());
 
-  EXPECT_STREQ(test_extension_id, fake_event_router->extension_id().c_str());
+  EXPECT_STREQ(kTestExtensionId, fake_event_router->extension_id().c_str());
   EXPECT_STREQ(bluetooth::OnConnection::kEventName,
                fake_event_router->event()->event_name.c_str());