diff options
Diffstat (limited to 'chrome/browser')
7 files changed, 133 insertions, 199 deletions
diff --git a/chrome/browser/extensions/api/socket/socket_api.cc b/chrome/browser/extensions/api/socket/socket_api.cc index 4d18ff9..f2cf86c 100644 --- a/chrome/browser/extensions/api/socket/socket_api.cc +++ b/chrome/browser/extensions/api/socket/socket_api.cc @@ -7,6 +7,8 @@ #include "base/bind.h" #include "base/values.h" #include "chrome/browser/extensions/api/socket/socket_api_controller.h" +#include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/profiles/profile.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h" @@ -18,13 +20,36 @@ const char kBytesWrittenKey[] = "bytesWritten"; const char kSocketIdKey[] = "socketId"; const char kUDPSocketType[] = "udp"; -SocketCreateFunction::SocketCreateFunction() { +SocketController* SocketApiFunction::controller() { + return profile()->GetExtensionService()->socket_controller(); } -SocketCreateFunction::~SocketCreateFunction() { +bool SocketApiFunction::RunImpl() { + if (!Prepare()) { + return false; + } + bool rv = BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&SocketApiFunction::WorkOnIOThread, this)); + DCHECK(rv); + return true; +} + +void SocketApiFunction::WorkOnIOThread() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + Work(); + bool rv = BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(&SocketApiFunction::RespondOnUIThread, this)); + DCHECK(rv); +} + +void SocketApiFunction::RespondOnUIThread() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + SendResponse(Respond()); } -bool SocketCreateFunction::RunImpl() { +bool SocketCreateFunction::Prepare() { std::string socket_type; EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &socket_type)); @@ -36,172 +61,81 @@ bool SocketCreateFunction::RunImpl() { if (socket_type != kUDPSocketType) { return false; } - - bool rv = BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&SocketCreateFunction::WorkOnIOThread, this)); - DCHECK(rv); return true; } -void SocketCreateFunction::WorkOnIOThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); +void SocketCreateFunction::Work() { DictionaryValue* result = new DictionaryValue(); - SocketController* controller = SocketController::GetInstance(); - - int socket_id = controller->CreateUdp(profile(), extension_id(), - source_url()); + int socket_id = controller()->CreateUdp(profile(), extension_id(), + source_url()); result->SetInteger(kSocketIdKey, socket_id); result_.reset(result); - bool rv = BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&SocketCreateFunction::RespondOnUIThread, this)); - DCHECK(rv); -} - -void SocketCreateFunction::RespondOnUIThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - SendResponse(true); } -SocketDestroyFunction::SocketDestroyFunction() { -} - -SocketDestroyFunction::~SocketDestroyFunction() { +bool SocketCreateFunction::Respond() { + return true; } -bool SocketDestroyFunction::RunImpl() { +bool SocketDestroyFunction::Prepare() { EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &socket_id_)); - - bool rv = BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&SocketDestroyFunction::WorkOnIOThread, this)); - DCHECK(rv); return true; } -void SocketDestroyFunction::WorkOnIOThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - SocketController* controller = SocketController::GetInstance(); - controller->DestroyUdp(socket_id_); - - bool rv = BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&SocketDestroyFunction::RespondOnUIThread, this)); - DCHECK(rv); -} - -void SocketDestroyFunction::RespondOnUIThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - SendResponse(true); -} - -SocketConnectFunction::SocketConnectFunction() { +void SocketDestroyFunction::Work() { + controller()->DestroyUdp(socket_id_); } -SocketConnectFunction::~SocketConnectFunction() { +bool SocketDestroyFunction::Respond() { + return true; } -bool SocketConnectFunction::RunImpl() { +bool SocketConnectFunction::Prepare() { EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &socket_id_)); EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &address_)); EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(2, &port_)); - - bool rv = BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&SocketConnectFunction::WorkOnIOThread, this)); - DCHECK(rv); return true; } -void SocketConnectFunction::WorkOnIOThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - - SocketController* controller = SocketController::GetInstance(); - bool result = controller->ConnectUdp(socket_id_, address_, port_); +void SocketConnectFunction::Work() { + bool result = controller()->ConnectUdp(socket_id_, address_, port_); result_.reset(Value::CreateBooleanValue(result)); - - bool rv = BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&SocketConnectFunction::RespondOnUIThread, this)); - DCHECK(rv); } -void SocketConnectFunction::RespondOnUIThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - SendResponse(true); -} - -SocketCloseFunction::SocketCloseFunction() { -} - -SocketCloseFunction::~SocketCloseFunction() { +bool SocketConnectFunction::Respond() { + return true; } -bool SocketCloseFunction::RunImpl() { +bool SocketCloseFunction::Prepare() { EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &socket_id_)); - - bool rv = BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&SocketCloseFunction::WorkOnIOThread, this)); - DCHECK(rv); return true; } -void SocketCloseFunction::WorkOnIOThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - - SocketController* controller = SocketController::GetInstance(); - controller->CloseUdp(socket_id_); +void SocketCloseFunction::Work() { + controller()->CloseUdp(socket_id_); result_.reset(Value::CreateNullValue()); - - bool rv = BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&SocketCloseFunction::RespondOnUIThread, this)); - DCHECK(rv); } -void SocketCloseFunction::RespondOnUIThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - SendResponse(true); -} - -SocketWriteFunction::SocketWriteFunction() { -} - -SocketWriteFunction::~SocketWriteFunction() { +bool SocketCloseFunction::Respond() { + return true; } -bool SocketWriteFunction::RunImpl() { +bool SocketWriteFunction::Prepare() { EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &socket_id_)); EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &message_)); - - bool rv = BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&SocketWriteFunction::WorkOnIOThread, this)); - DCHECK(rv); return true; } -void SocketWriteFunction::WorkOnIOThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - - SocketController* controller = SocketController::GetInstance(); - int bytesWritten = controller->WriteUdp(socket_id_, message_); +void SocketWriteFunction::Work() { + int bytesWritten = controller()->WriteUdp(socket_id_, message_); DictionaryValue* result = new DictionaryValue(); result->SetInteger(kBytesWrittenKey, bytesWritten); result_.reset(result); - bool rv = BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&SocketWriteFunction::RespondOnUIThread, this)); - DCHECK(rv); } -void SocketWriteFunction::RespondOnUIThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - SendResponse(true); +bool SocketWriteFunction::Respond() { + return true; } } // namespace extensions diff --git a/chrome/browser/extensions/api/socket/socket_api.h b/chrome/browser/extensions/api/socket/socket_api.h index d28c497..8d1a7a2 100644 --- a/chrome/browser/extensions/api/socket/socket_api.h +++ b/chrome/browser/extensions/api/socket/socket_api.h @@ -12,38 +12,52 @@ namespace extensions { +class SocketController; + extern const char kBytesWrittenKey[]; extern const char kSocketIdKey[]; extern const char kUdpSocketType[]; +class SocketApiFunction : public AsyncExtensionFunction { + protected: + // Set up for work. Guaranteed to happen on UI thread. + virtual bool Prepare() = 0; + + // Do actual work. Guaranteed to happen on IO thread. + virtual void Work() = 0; + + // Respond. Guaranteed to happen on UI thread. + virtual bool Respond() = 0; + + virtual bool RunImpl() OVERRIDE; + + SocketController* controller(); + + private: + void WorkOnIOThread(); + void RespondOnUIThread(); +}; + // Many of these socket functions are synchronous in the sense that // they don't involve blocking operations, but we've made them all // AsyncExtensionFunctions because the underlying UDPClientSocket // library wants all operations to happen on the same thread as the // one that created the socket. Too bad. -class SocketCreateFunction : public AsyncExtensionFunction { - public: - SocketCreateFunction(); - virtual ~SocketCreateFunction(); - virtual bool RunImpl() OVERRIDE; - +class SocketCreateFunction : public SocketApiFunction { protected: - void WorkOnIOThread(); - void RespondOnUIThread(); + virtual bool Prepare() OVERRIDE; + virtual void Work() OVERRIDE; + virtual bool Respond() OVERRIDE; DECLARE_EXTENSION_FUNCTION_NAME("experimental.socket.create") }; -class SocketDestroyFunction : public AsyncExtensionFunction { - public: - SocketDestroyFunction(); - virtual ~SocketDestroyFunction(); - virtual bool RunImpl() OVERRIDE; - +class SocketDestroyFunction : public SocketApiFunction { protected: - void WorkOnIOThread(); - void RespondOnUIThread(); + virtual bool Prepare() OVERRIDE; + virtual void Work() OVERRIDE; + virtual bool Respond() OVERRIDE; private: int socket_id_; @@ -51,15 +65,11 @@ class SocketDestroyFunction : public AsyncExtensionFunction { DECLARE_EXTENSION_FUNCTION_NAME("experimental.socket.destroy") }; -class SocketConnectFunction : public AsyncExtensionFunction { - public: - SocketConnectFunction(); - virtual ~SocketConnectFunction(); - virtual bool RunImpl() OVERRIDE; - +class SocketConnectFunction : public SocketApiFunction { protected: - void WorkOnIOThread(); - void RespondOnUIThread(); + virtual bool Prepare() OVERRIDE; + virtual void Work() OVERRIDE; + virtual bool Respond() OVERRIDE; private: int socket_id_; @@ -69,15 +79,11 @@ class SocketConnectFunction : public AsyncExtensionFunction { DECLARE_EXTENSION_FUNCTION_NAME("experimental.socket.connect") }; -class SocketCloseFunction : public AsyncExtensionFunction { - public: - SocketCloseFunction(); - virtual ~SocketCloseFunction(); - virtual bool RunImpl() OVERRIDE; - +class SocketCloseFunction : public SocketApiFunction { protected: - void WorkOnIOThread(); - void RespondOnUIThread(); + virtual bool Prepare() OVERRIDE; + virtual void Work() OVERRIDE; + virtual bool Respond() OVERRIDE; private: int socket_id_; @@ -85,16 +91,13 @@ class SocketCloseFunction : public AsyncExtensionFunction { DECLARE_EXTENSION_FUNCTION_NAME("experimental.socket.close") }; -class SocketWriteFunction : public AsyncExtensionFunction { - public: - SocketWriteFunction(); - virtual ~SocketWriteFunction(); - virtual bool RunImpl() OVERRIDE; - +class SocketWriteFunction : public SocketApiFunction { protected: - void WorkOnIOThread(); - void RespondOnUIThread(); + virtual bool Prepare() OVERRIDE; + virtual void Work() OVERRIDE; + virtual bool Respond() OVERRIDE; + private: int socket_id_; std::string message_; diff --git a/chrome/browser/extensions/api/socket/socket_api_controller.cc b/chrome/browser/extensions/api/socket/socket_api_controller.cc index 2195297..8758719 100644 --- a/chrome/browser/extensions/api/socket/socket_api_controller.cc +++ b/chrome/browser/extensions/api/socket/socket_api_controller.cc @@ -22,8 +22,8 @@ namespace extensions { // we need to manage it in the context of an extension. class Socket { public: - explicit Socket(const Profile* profile, const std::string& src_extension_id, - const GURL& src_url); + Socket(const Profile* profile, const std::string& src_extension_id, + const GURL& src_url); ~Socket(); bool Connect(const net::IPEndPoint& ip_end_point); @@ -38,7 +38,7 @@ class Socket { std::string src_extension_id_; GURL src_url_; - net::UDPClientSocket* udp_client_socket_; + scoped_ptr<net::UDPClientSocket> udp_client_socket_; bool is_connected_; net::OldCompletionCallbackImpl<Socket> io_callback_; @@ -101,30 +101,25 @@ int Socket::Write(const std::string message) { return bytes_sent; } -SocketController* SocketController::GetInstance() { - return Singleton<SocketController>::get(); +SocketController::SocketController() : next_socket_id_(1) { } -SocketController::SocketController() : next_socket_id_(1) {} - -SocketController::~SocketController() { - STLDeleteValues(&socket_map_); -} +SocketController::~SocketController() {} Socket* SocketController::GetSocket(int socket_id) { // TODO(miket): we should verify that the extension asking for the // socket is the same one that created it. SocketMap::iterator i = socket_map_.find(socket_id); if (i != socket_map_.end()) - return i->second; + return i->second.get(); return NULL; } int SocketController::CreateUdp(const Profile* profile, const std::string& extension_id, const GURL& src_url) { - Socket* socket = new Socket(profile, extension_id, src_url); - CHECK(socket); + linked_ptr<Socket> socket(new Socket(profile, extension_id, src_url)); + CHECK(socket.get()); socket_map_[next_socket_id_] = socket; return next_socket_id_++; } diff --git a/chrome/browser/extensions/api/socket/socket_api_controller.h b/chrome/browser/extensions/api/socket/socket_api_controller.h index f35b10a..ff50bc0 100644 --- a/chrome/browser/extensions/api/socket/socket_api_controller.h +++ b/chrome/browser/extensions/api/socket/socket_api_controller.h @@ -9,8 +9,8 @@ #include <string> #include <map> +#include "base/memory/linked_ptr.h" #include "base/memory/scoped_ptr.h" -#include "base/memory/singleton.h" #include "googleurl/src/gurl.h" #include "net/base/completion_callback.h" @@ -30,25 +30,19 @@ namespace extensions { class Socket; -// The SocketController singleton keeps track of all our Sockets, and provides -// a convenient set of methods to manipulate them. +// SocketController keeps track of a collection of Sockets and provides a +// convenient set of methods to manipulate them. class SocketController { public: - static SocketController* GetInstance(); - SocketController(); virtual ~SocketController(); // Create/Destroy are a pair. They represent the allocation and deallocation // of the Socket object in memory. // - // TODO(miket): we currently require the app developer to remember to call - // Destroy, which is a buzzkill in JavaScript. I believe that to solve this, - // we'll have to associate each Socket with a creator extension, and then - // clean up when the extension goes out of scope. As the API is defined - // today, we're exposing only primitive socketIds to JavaScript, which seems - // to imply that we won't be able to garbage-collect when individual sockets - // "go out of scope" (in quotes because they never do). + // TODO(miket): aa's suggestion to track lifetime of callbacks associated + // with each socket, which will then let us clean up when we go out of scope + // rather than requiring that the app developer remember to call Destroy. int CreateUdp(const Profile* profile, const std::string& extension_id, const GURL& src_url); bool DestroyUdp(int socket_id); @@ -68,14 +62,12 @@ class SocketController { private: int next_socket_id_; - typedef std::map<int, Socket*> SocketMap; + typedef std::map<int, linked_ptr<Socket> > SocketMap; SocketMap socket_map_; // Convenience method for accessing SocketMap. Socket* GetSocket(int socket_id); - friend struct DefaultSingletonTraits<SocketController>; - DISALLOW_COPY_AND_ASSIGN(SocketController); }; diff --git a/chrome/browser/extensions/api/socket/socket_api_controller_unittest.cc b/chrome/browser/extensions/api/socket/socket_api_controller_unittest.cc index 97961e4..376b66d 100644 --- a/chrome/browser/extensions/api/socket/socket_api_controller_unittest.cc +++ b/chrome/browser/extensions/api/socket/socket_api_controller_unittest.cc @@ -4,6 +4,7 @@ #include "testing/gtest/include/gtest/gtest.h" +#include "base/memory/scoped_ptr.h" #include "base/values.h" #include "chrome/browser/extensions/api/socket/socket_api.h" #include "chrome/browser/extensions/api/socket/socket_api_controller.h" @@ -16,7 +17,7 @@ class SocketApiControllerTest : public testing::Test { TEST_F(SocketApiControllerTest, TestSocketControllerLifetime) { // We want to make sure that killing the controller while a bunch of // sockets are alive doesn't crash. - SocketController* controller = SocketController::GetInstance(); + scoped_ptr<SocketController> controller(new SocketController()); // Create some sockets but don't do anything with them. Profile* profile = NULL; @@ -36,16 +37,6 @@ TEST_F(SocketApiControllerTest, TestSocketControllerLifetime) { ASSERT_TRUE(socket_id != 0); ASSERT_TRUE(controller->ConnectUdp(socket_id, address, kPort)); } - - // At this point, we're done, and we're relying on the RAE mechanism - // of the Singleton class to delete the controller at process exit. - // We'd have to jump through some icky hoops to turn off RAE and - // manually delete in this test method, so we'll instead take it on - // faith that the singleton will indeed delete itself, and that if - // we had any heap management problems in the controller, they'd - // show up later in this test process. I (miket) hereby confirm that - // I manually added a temporary double-free in the controller - // destructor and verified that the unit_tests process segfaulted. } } // namespace extensions diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index 3a6ff8b..094a028 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -21,6 +21,7 @@ #include "base/string_number_conversions.h" #include "base/string_util.h" #include "base/stringprintf.h" +#include "base/task.h" #include "base/threading/thread_restrictions.h" #include "base/time.h" #include "base/utf_string_conversions.h" @@ -31,6 +32,7 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_plugin_service_filter.h" #include "chrome/browser/download/download_extension_api.h" +#include "chrome/browser/extensions/api/socket/socket_api_controller.h" #include "chrome/browser/extensions/app_notification_manager.h" #include "chrome/browser/extensions/apps_promo.h" #include "chrome/browser/extensions/component_loader.h" @@ -386,7 +388,8 @@ ExtensionService::ExtensionService(Profile* profile, permissions_manager_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), apps_promo_(profile->GetPrefs()), event_routers_initialized_(false), - extension_warnings_(profile) { + extension_warnings_(profile), + socket_controller_(new extensions::SocketController()) { CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); // Figure out if extension installation should be enabled. @@ -470,6 +473,13 @@ ExtensionService::~ExtensionService() { ExternalExtensionProviderInterface* provider = i->get(); provider->ServiceShutdown(); } + + // TODO(miket): if we find ourselves adding more and more per-API + // controllers, we should manage them all with an + // APIControllerController (still working on that name). + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + new DeleteTask<extensions::SocketController>(socket_controller_)); } void ExtensionService::InitEventRoutersAfterImport() { diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h index 67b0f4d..1b288df 100644 --- a/chrome/browser/extensions/extension_service.h +++ b/chrome/browser/extensions/extension_service.h @@ -75,6 +75,7 @@ class ExtensionInputMethodEventRouter; namespace extensions { class ComponentLoader; class SettingsFrontend; +class SocketController; } // This is an interface class to encapsulate the dependencies that @@ -569,6 +570,10 @@ class ExtensionService return &extension_warnings_; } + extensions::SocketController* socket_controller() { + return socket_controller_; + } + private: // Bundle of type (app or extension)-specific sync stuff. struct SyncBundle { @@ -819,6 +824,10 @@ class ExtensionService // Contains an entry for each warning that shall be currently shown. ExtensionWarningSet extension_warnings_; + // We need to control destruction of this object (it needs to happen on the + // IO thread), so we don't get to use any RAII devices with it. + extensions::SocketController* socket_controller_; + extensions::ProcessMap process_map_; FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest, |