diff options
4 files changed, 102 insertions, 5 deletions
diff --git a/chrome/browser/extensions/api/api_resource_manager.h b/chrome/browser/extensions/api/api_resource_manager.h index 212d1e9..d954a1d 100644 --- a/chrome/browser/extensions/api/api_resource_manager.h +++ b/chrome/browser/extensions/api/api_resource_manager.h @@ -10,7 +10,12 @@ #include "base/memory/linked_ptr.h" #include "base/threading/non_thread_safe.h" #include "chrome/browser/profiles/profile_keyed_service.h" +#include "chrome/common/chrome_notification_types.h" +#include "chrome/common/extensions/extension.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/notification_observer.h" +#include "content/public/browser/notification_registrar.h" +#include "content/public/browser/notification_service.h" using content::BrowserThread; @@ -20,12 +25,20 @@ namespace extensions { // ApiFunctions use. Examples are sockets or USB connections. template <class T> class ApiResourceManager : public ProfileKeyedService, - public base::NonThreadSafe { + public base::NonThreadSafe, + public content::NotificationObserver { public: - explicit ApiResourceManager(BrowserThread::ID thread_id) + typedef std::map<int, linked_ptr<T> > ApiResourceMap; + // Lookup map from extension id's to allocated resource id's. + typedef std::map<std::string, base::hash_set<int> > ExtensionToResourceMap; + + explicit ApiResourceManager(const BrowserThread::ID thread_id) : next_id_(1), thread_id_(thread_id), - api_resource_map_(new std::map<int, linked_ptr<T> >()) { + api_resource_map_(new ApiResourceMap()) { + registrar_.Add(this, + chrome::NOTIFICATION_EXTENSION_UNLOADED, + content::NotificationService::AllSources()); } virtual ~ApiResourceManager() { @@ -46,6 +59,13 @@ class ApiResourceManager : public ProfileKeyedService, if (id > 0) { linked_ptr<T> resource_ptr(api_resource); (*api_resource_map_)[id] = resource_ptr; + + const std::string& extension_id = api_resource->owner_extension_id(); + if (extension_resource_map_.find(extension_id) == + extension_resource_map_.end()) + extension_resource_map_[extension_id] = base::hash_set<int>(); + extension_resource_map_[extension_id].insert(id); + return id; } return 0; @@ -54,6 +74,9 @@ class ApiResourceManager : public ProfileKeyedService, void Remove(const std::string& extension_id, int api_resource_id) { DCHECK(BrowserThread::CurrentlyOn(thread_id_)); if (GetOwnedResource(extension_id, api_resource_id) != NULL) { + DCHECK(extension_resource_map_.find(extension_id) != + extension_resource_map_.end()); + extension_resource_map_[extension_id].erase(api_resource_id); api_resource_map_->erase(api_resource_id); } } @@ -63,6 +86,24 @@ class ApiResourceManager : public ProfileKeyedService, return GetOwnedResource(extension_id, api_resource_id); } + protected: + // content::NotificationObserver: + virtual void Observe(int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) OVERRIDE { + switch (type) { + case chrome::NOTIFICATION_EXTENSION_UNLOADED: { + std::string id = + content::Details<extensions::UnloadedExtensionInfo>(details)-> + extension->id(); + BrowserThread::PostTask(thread_id_, FROM_HERE, + base::Bind(&ApiResourceManager::CleanupResourcesFromExtension, + base::Unretained(this), id)); + break; + } + } + } + private: int GenerateId() { return next_id_++; @@ -77,12 +118,26 @@ class ApiResourceManager : public ProfileKeyedService, return NULL; } + void CleanupResourcesFromExtension(const std::string& extension_id) { + if (extension_resource_map_.find(extension_id) != + extension_resource_map_.end()) { + base::hash_set<int>& resource_ids = extension_resource_map_[extension_id]; + for (base::hash_set<int>::iterator it = resource_ids.begin(); + it != resource_ids.end(); ++it) { + api_resource_map_->erase(*it); + } + extension_resource_map_.erase(extension_id); + } + } + int next_id_; - BrowserThread::ID thread_id_; + const BrowserThread::ID thread_id_; + content::NotificationRegistrar registrar_; // We need finer-grained control over the lifetime of this instance than RAII // can give us. - std::map<int, linked_ptr<T> >* api_resource_map_; + ApiResourceMap* api_resource_map_; + ExtensionToResourceMap extension_resource_map_; }; } // namespace extensions diff --git a/chrome/browser/extensions/api/socket/socket_apitest.cc b/chrome/browser/extensions/api/socket/socket_apitest.cc index 2c40f78..2124eb3 100644 --- a/chrome/browser/extensions/api/socket/socket_apitest.cc +++ b/chrome/browser/extensions/api/socket/socket_apitest.cc @@ -189,3 +189,8 @@ IN_PROC_BROWSER_TEST_F(SocketApiTest, SocketExperimentalPermissionTest) { EXPECT_TRUE(catcher.GetNextResult()) << catcher.message(); } + +IN_PROC_BROWSER_TEST_F(SocketApiTest, SocketTCPServerUnbindOnUnload) { + ASSERT_TRUE(RunExtensionTest("socket/unload")) << message_; + ASSERT_TRUE(RunExtensionTest("socket/unload")) << message_; +} diff --git a/chrome/test/data/extensions/api_test/socket/unload/background.js b/chrome/test/data/extensions/api_test/socket/unload/background.js new file mode 100644 index 0000000..902451e --- /dev/null +++ b/chrome/test/data/extensions/api_test/socket/unload/background.js @@ -0,0 +1,21 @@ +// Copyright (c) 2012 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. + +const socket = chrome.socket; + +var onListen = function(result) { + chrome.test.assertEq(0, result); + chrome.test.succeed(); +}; + +var onCreate = function(socketInfo) { + sid = socketInfo.socketId; + socket.listen(sid, '127.0.0.1', 1234, onListen); +}; + +chrome.test.runTests([ + function bind() { + socket.create('tcp', {}, onCreate); + } +]); diff --git a/chrome/test/data/extensions/api_test/socket/unload/manifest.json b/chrome/test/data/extensions/api_test/socket/unload/manifest.json new file mode 100644 index 0000000..c61d0c1 --- /dev/null +++ b/chrome/test/data/extensions/api_test/socket/unload/manifest.json @@ -0,0 +1,16 @@ +{ + "name": "chrome.socket", + "version": "0.1", + "description": "browser test for chrome.socket API to make sure sockets are free'd when extension is reloaded", + "app": { + "background": { + "scripts": ["background.js"] + } + }, + "permissions": [ + "experimental", + {"socket": [ + "tcp-listen" + ]} + ] +} |