diff options
27 files changed, 296 insertions, 77 deletions
diff --git a/chrome/browser/extensions/api/api_resource.cc b/chrome/browser/extensions/api/api_resource.cc index 400acde..19b7b91 100644 --- a/chrome/browser/extensions/api/api_resource.cc +++ b/chrome/browser/extensions/api/api_resource.cc @@ -7,8 +7,15 @@ namespace extensions { -ApiResource::ApiResource(ApiResourceEventNotifier* event_notifier) - : event_notifier_(event_notifier) { +ApiResource::ApiResource(const std::string& owner_extension_id, + ApiResourceEventNotifier* event_notifier) + : owner_extension_id_(owner_extension_id), + event_notifier_(event_notifier) { + + CHECK(!owner_extension_id_.empty()); + if (event_notifier) + CHECK(event_notifier->src_extension_id() == owner_extension_id_); + // scoped_refptr<> constructor does the initial AddRef() for us on // event_notifier_. } diff --git a/chrome/browser/extensions/api/api_resource.h b/chrome/browser/extensions/api/api_resource.h index 86b9180..1a38cce 100644 --- a/chrome/browser/extensions/api/api_resource.h +++ b/chrome/browser/extensions/api/api_resource.h @@ -7,6 +7,7 @@ #include "base/basictypes.h" #include "base/memory/ref_counted.h" +#include "chrome/common/extensions/extension.h" namespace extensions { @@ -19,12 +20,25 @@ class ApiResource { public: virtual ~ApiResource(); + const std::string& owner_extension_id() const { + return owner_extension_id_; + } + protected: - ApiResource(ApiResourceEventNotifier* event_notifier); + ApiResource(const std::string& owner_extension_id, + ApiResourceEventNotifier* event_notifier); ApiResourceEventNotifier* event_notifier() const; private: + // The extension that owns this resource. This could be derived from + // event_notifier, but that's a little too cute; to make future code + // maintenance easier, we'll require callers to explicitly specify the owner, + // and for now we'll assert that the owner implied by the event_notifier is + // consistent with the explicit one. + const std::string& owner_extension_id_; + + // The object that lets this resource report events to the owner application. scoped_refptr<ApiResourceEventNotifier> event_notifier_; DISALLOW_COPY_AND_ASSIGN(ApiResource); diff --git a/chrome/browser/extensions/api/api_resource_event_notifier.h b/chrome/browser/extensions/api/api_resource_event_notifier.h index f987663..1a98a4a 100644 --- a/chrome/browser/extensions/api/api_resource_event_notifier.h +++ b/chrome/browser/extensions/api/api_resource_event_notifier.h @@ -33,8 +33,7 @@ extern const char kSrcIdKey[]; class ApiResourceEventNotifier : public base::RefCountedThreadSafe<ApiResourceEventNotifier> { public: - ApiResourceEventNotifier(EventRouter* router, - Profile* profile, + ApiResourceEventNotifier(EventRouter* router, Profile* profile, const std::string& src_extension_id, int src_id, const GURL& src_url); @@ -45,6 +44,8 @@ class ApiResourceEventNotifier static std::string ApiResourceEventTypeToString( ApiResourceEventType event_type); + const std::string& src_extension_id() const { return src_extension_id_; } + private: friend class base::RefCountedThreadSafe<ApiResourceEventNotifier>; friend class MockApiResourceEventNotifier; diff --git a/chrome/browser/extensions/api/api_resource_manager.h b/chrome/browser/extensions/api/api_resource_manager.h index ded4dd5..212d1e9 100644 --- a/chrome/browser/extensions/api/api_resource_manager.h +++ b/chrome/browser/extensions/api/api_resource_manager.h @@ -51,24 +51,32 @@ class ApiResourceManager : public ProfileKeyedService, return 0; } - void Remove(int api_resource_id) { + void Remove(const std::string& extension_id, int api_resource_id) { DCHECK(BrowserThread::CurrentlyOn(thread_id_)); - api_resource_map_->erase(api_resource_id); + if (GetOwnedResource(extension_id, api_resource_id) != NULL) { + api_resource_map_->erase(api_resource_id); + } } - T* Get(int api_resource_id) { + T* Get(const std::string& extension_id, int api_resource_id) { DCHECK(BrowserThread::CurrentlyOn(thread_id_)); - linked_ptr<T> ptr = (*api_resource_map_)[api_resource_id]; - return ptr.get(); + return GetOwnedResource(extension_id, api_resource_id); } private: - // TODO(miket): consider partitioning the ID space by extension ID to make it - // harder for extensions to peek into each others' resources. int GenerateId() { return next_id_++; } + T* GetOwnedResource(const std::string& extension_id, + int api_resource_id) { + linked_ptr<T> ptr = (*api_resource_map_)[api_resource_id]; + T* resource = ptr.get(); + if (resource && extension_id == resource->owner_extension_id()) + return resource; + return NULL; + } + int next_id_; BrowserThread::ID thread_id_; diff --git a/chrome/browser/extensions/api/api_resource_manager_unittest.cc b/chrome/browser/extensions/api/api_resource_manager_unittest.cc new file mode 100644 index 0000000..9ab46de --- /dev/null +++ b/chrome/browser/extensions/api/api_resource_manager_unittest.cc @@ -0,0 +1,79 @@ +// 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. + +#include "base/file_path.h" +#include "base/string_util.h" +#include "chrome/browser/extensions/api/api_resource.h" +#include "chrome/browser/extensions/api/api_resource_manager.h" +#include "chrome/browser/extensions/extension_function_test_utils.h" +#include "chrome/common/extensions/extension.h" +#include "chrome/common/extensions/extension_test_util.h" +#include "chrome/test/base/browser_with_test_window_test.h" +#include "content/public/browser/browser_thread.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "chrome/browser/extensions/api/api_resource_event_notifier.h" +#include "chrome/browser/profiles/profile.h" +#include "googleurl/src/gurl.h" + +namespace utils = extension_function_test_utils; + +using content::BrowserThread; + +namespace extensions { + +class ApiResourceManagerUnitTest : public BrowserWithTestWindowTest { + public: + virtual void SetUp() { + BrowserWithTestWindowTest::SetUp(); + } +}; + +class FakeApiResource : public ApiResource { + public: + FakeApiResource(const std::string& owner_extension_id, + ApiResourceEventNotifier* event_notifier) : + ApiResource(owner_extension_id, event_notifier) {} + ~FakeApiResource() {} +}; + +TEST_F(ApiResourceManagerUnitTest, TwoAppsCannotShareResources) { + scoped_ptr<ApiResourceManager<FakeApiResource> > manager( + new ApiResourceManager<FakeApiResource>(BrowserThread::UI)); + scoped_refptr<extensions::Extension> extension_one( + utils::CreateEmptyExtension("one")); + scoped_refptr<extensions::Extension> extension_two( + utils::CreateEmptyExtension("two")); + + const std::string extension_one_id(extension_one->id()); + const std::string extension_two_id(extension_two->id()); + + GURL url_one("url-one"); + GURL url_two("url-two"); + scoped_refptr<ApiResourceEventNotifier> event_notifier_one( + new ApiResourceEventNotifier( + NULL, NULL, extension_one_id, 1111, url_one)); + scoped_refptr<ApiResourceEventNotifier> event_notifier_two( + new ApiResourceEventNotifier( + NULL, NULL, extension_two_id, 2222, url_two)); + + int resource_one_id = manager->Add(new FakeApiResource( + extension_one_id, event_notifier_one.get())); + int resource_two_id = manager->Add(new FakeApiResource( + extension_two_id, event_notifier_two.get())); + CHECK(resource_one_id); + CHECK(resource_two_id); + + // Confirm each extension can get its own resource. + ASSERT_TRUE(manager->Get(extension_one_id, resource_one_id) != NULL); + ASSERT_TRUE(manager->Get(extension_two_id, resource_two_id) != NULL); + + // Confirm neither extension can get the other's resource. + ASSERT_TRUE(manager->Get(extension_one_id, resource_two_id) == NULL); + ASSERT_TRUE(manager->Get(extension_two_id, resource_one_id) == NULL); + + // And make sure we're not susceptible to any Jedi mind tricks. + ASSERT_TRUE(manager->Get(std::string(), resource_one_id) == NULL); +} + +} // namespace extensions diff --git a/chrome/browser/extensions/api/serial/serial_api.cc b/chrome/browser/extensions/api/serial/serial_api.cc index 87a61c2..8ffb834 100644 --- a/chrome/browser/extensions/api/serial/serial_api.cc +++ b/chrome/browser/extensions/api/serial/serial_api.cc @@ -45,6 +45,15 @@ bool SerialAsyncApiFunction::PrePrepare() { return manager_ != NULL; } +SerialConnection* SerialAsyncApiFunction::GetSerialConnection( + int api_resource_id) { + return manager_->Get(extension_->id(), api_resource_id); +} + +void SerialAsyncApiFunction::RemoveSerialConnection(int api_resource_id) { + manager_->Remove(extension_->id(), api_resource_id); +} + SerialGetPortsFunction::SerialGetPortsFunction() {} bool SerialGetPortsFunction::Prepare() { @@ -116,6 +125,7 @@ void SerialOpenFunction::Work() { SerialConnection* serial_connection = CreateSerialConnection( params_->port, bitrate_, + extension_->id(), event_notifier_); CHECK(serial_connection); int id = manager_->Add(serial_connection); @@ -124,7 +134,7 @@ void SerialOpenFunction::Work() { bool open_result = serial_connection->Open(); if (!open_result) { serial_connection->Close(); - manager_->Remove(id); + RemoveSerialConnection(id); id = -1; } @@ -143,8 +153,10 @@ void SerialOpenFunction::Work() { SerialConnection* SerialOpenFunction::CreateSerialConnection( const std::string& port, int bitrate, + const std::string& owner_extension_id, ApiResourceEventNotifier* event_notifier) { - return new SerialConnection(port, bitrate, event_notifier); + return new SerialConnection(port, bitrate, owner_extension_id, + event_notifier); } bool SerialOpenFunction::DoesPortExist(const std::string& port) { @@ -174,10 +186,11 @@ bool SerialCloseFunction::Prepare() { void SerialCloseFunction::Work() { bool close_result = false; - SerialConnection* serial_connection = manager_->Get(params_->connection_id); + SerialConnection* serial_connection = GetSerialConnection( + params_->connection_id); if (serial_connection) { serial_connection->Close(); - manager_->Remove(params_->connection_id); + RemoveSerialConnection(params_->connection_id); close_result = true; } @@ -211,7 +224,8 @@ void SerialReadFunction::Work() { int bytes_read = -1; scoped_refptr<net::IOBufferWithSize> io_buffer( new net::IOBufferWithSize(params_->bytes_to_read)); - SerialConnection* serial_connection(manager_->Get(params_->connection_id)); + SerialConnection* serial_connection(GetSerialConnection( + params_->connection_id)); if (serial_connection) bytes_read = serial_connection->Read(io_buffer); @@ -253,7 +267,8 @@ bool SerialWriteFunction::Prepare() { void SerialWriteFunction::Work() { int bytes_written = -1; - SerialConnection* serial_connection = manager_->Get(params_->connection_id); + SerialConnection* serial_connection = GetSerialConnection( + params_->connection_id); if (serial_connection) bytes_written = serial_connection->Write(io_buffer_, io_buffer_size_); else @@ -284,7 +299,8 @@ bool SerialFlushFunction::Prepare() { void SerialFlushFunction::Work() { bool flush_result = false; - SerialConnection* serial_connection = manager_->Get(params_->connection_id); + SerialConnection* serial_connection = GetSerialConnection( + params_->connection_id); if (serial_connection) { serial_connection->Flush(); flush_result = true; @@ -316,7 +332,8 @@ bool SerialGetControlSignalsFunction::Prepare() { void SerialGetControlSignalsFunction::Work() { DictionaryValue *result = new DictionaryValue(); - SerialConnection* serial_connection = manager_->Get(params_->connection_id); + SerialConnection* serial_connection = GetSerialConnection( + params_->connection_id); if (serial_connection) { SerialConnection::ControlSignals control_signals = { 0 }; if (serial_connection->GetControlSignals(control_signals)) { @@ -355,7 +372,8 @@ bool SerialSetControlSignalsFunction::Prepare() { } void SerialSetControlSignalsFunction::Work() { - SerialConnection* serial_connection = manager_->Get(params_->connection_id); + SerialConnection* serial_connection = GetSerialConnection( + params_->connection_id); if (serial_connection) { SerialConnection::ControlSignals control_signals = { 0 }; control_signals.should_set_dtr = params_->options.dtr.get() != NULL; diff --git a/chrome/browser/extensions/api/serial/serial_api.h b/chrome/browser/extensions/api/serial/serial_api.h index 7e43b63..5c10290 100644 --- a/chrome/browser/extensions/api/serial/serial_api.h +++ b/chrome/browser/extensions/api/serial/serial_api.h @@ -30,6 +30,9 @@ class SerialAsyncApiFunction : public AsyncApiFunction { // AsyncApiFunction: virtual bool PrePrepare() OVERRIDE; + SerialConnection* GetSerialConnection(int api_resource_id); + void RemoveSerialConnection(int api_resource_id); + ApiResourceManager<SerialConnection>* manager_; }; @@ -67,6 +70,7 @@ class SerialOpenFunction : public SerialAsyncApiFunction { virtual SerialConnection* CreateSerialConnection( const std::string& port, int bitrate, + const std::string& owner_extension_id, ApiResourceEventNotifier* event_notifier); virtual bool DoesPortExist(const std::string& port); diff --git a/chrome/browser/extensions/api/serial/serial_apitest.cc b/chrome/browser/extensions/api/serial/serial_apitest.cc index 0bd26ab..2fbee49 100644 --- a/chrome/browser/extensions/api/serial/serial_apitest.cc +++ b/chrome/browser/extensions/api/serial/serial_apitest.cc @@ -53,8 +53,9 @@ class FakeEchoSerialConnection : public SerialConnection { explicit FakeEchoSerialConnection( const std::string& port, int bitrate, + const std::string& owner_extension_id, ApiResourceEventNotifier* event_notifier) - : SerialConnection(port, bitrate, event_notifier), + : SerialConnection(port, bitrate, owner_extension_id, event_notifier), opened_(true) { Flush(); opened_ = false; @@ -119,9 +120,11 @@ class FakeSerialOpenFunction : public SerialOpenFunction { virtual SerialConnection* CreateSerialConnection( const std::string& port, int bitrate, + const std::string& owner_extension_id, ApiResourceEventNotifier* event_notifier) OVERRIDE { FakeEchoSerialConnection* serial_connection = - new FakeEchoSerialConnection(port, bitrate, event_notifier); + new FakeEchoSerialConnection(port, bitrate, owner_extension_id, + event_notifier); EXPECT_CALL(*serial_connection, GetControlSignals(_)). Times(1).WillOnce(Return(true)); EXPECT_CALL(*serial_connection, SetControlSignals(_)). diff --git a/chrome/browser/extensions/api/serial/serial_connection.cc b/chrome/browser/extensions/api/serial/serial_connection.cc index 6834a61..ec34a5a 100644 --- a/chrome/browser/extensions/api/serial/serial_connection.cc +++ b/chrome/browser/extensions/api/serial/serial_connection.cc @@ -15,8 +15,9 @@ const char kSerialConnectionNotFoundError[] = "Serial connection not found"; SerialConnection::SerialConnection(const std::string& port, int bitrate, + const std::string& owner_extension_id, ApiResourceEventNotifier* event_notifier) - : ApiResource(event_notifier), + : ApiResource(owner_extension_id, event_notifier), port_(port), bitrate_(bitrate), file_(base::kInvalidPlatformFileValue) { diff --git a/chrome/browser/extensions/api/serial/serial_connection.h b/chrome/browser/extensions/api/serial/serial_connection.h index 4b72a13..c828cfc 100644 --- a/chrome/browser/extensions/api/serial/serial_connection.h +++ b/chrome/browser/extensions/api/serial/serial_connection.h @@ -29,6 +29,7 @@ class SerialConnection : public ApiResource { public: SerialConnection(const std::string& port, int bitrate, + const std::string& owner_extension_id, ApiResourceEventNotifier* event_notifier); virtual ~SerialConnection(); diff --git a/chrome/browser/extensions/api/socket/socket.cc b/chrome/browser/extensions/api/socket/socket.cc index e9f9b28..3d29721 100644 --- a/chrome/browser/extensions/api/socket/socket.cc +++ b/chrome/browser/extensions/api/socket/socket.cc @@ -14,8 +14,9 @@ namespace extensions { -Socket::Socket(ApiResourceEventNotifier* event_notifier) - : ApiResource(event_notifier), +Socket::Socket(const std::string& owner_extension_id, + ApiResourceEventNotifier* event_notifier) + : ApiResource(owner_extension_id, event_notifier), port_(0), is_connected_(false) { } diff --git a/chrome/browser/extensions/api/socket/socket.h b/chrome/browser/extensions/api/socket/socket.h index 951ce67..27e2685 100644 --- a/chrome/browser/extensions/api/socket/socket.h +++ b/chrome/browser/extensions/api/socket/socket.h @@ -87,7 +87,8 @@ class Socket : public ApiResource { int* port); protected: - explicit Socket(ApiResourceEventNotifier* event_notifier); + Socket(const std::string& owner_extension_id_, + ApiResourceEventNotifier* event_notifier); void WriteData(); virtual int WriteImpl(net::IOBuffer* io_buffer, diff --git a/chrome/browser/extensions/api/socket/socket_api.cc b/chrome/browser/extensions/api/socket/socket_api.cc index fb23691..ce400e5 100644 --- a/chrome/browser/extensions/api/socket/socket_api.cc +++ b/chrome/browser/extensions/api/socket/socket_api.cc @@ -58,6 +58,14 @@ bool SocketAsyncApiFunction::Respond() { return error_.empty(); } +Socket* SocketAsyncApiFunction::GetSocket(int api_resource_id) { + return manager_->Get(extension_->id(), api_resource_id); +} + +void SocketAsyncApiFunction::RemoveSocket(int api_resource_id) { + manager_->Remove(extension_->id(), api_resource_id); +} + SocketExtensionWithDnsLookupFunction::SocketExtensionWithDnsLookupFunction() : io_thread_(g_browser_process->io_thread()), request_handle_(new net::HostResolver::RequestHandle), @@ -132,9 +140,9 @@ bool SocketCreateFunction::Prepare() { void SocketCreateFunction::Work() { Socket* socket = NULL; if (socket_type_ == kSocketTypeTCP) { - socket = new TCPSocket(event_notifier_); + socket = new TCPSocket(extension_->id(), event_notifier_); } else if (socket_type_== kSocketTypeUDP) { - socket = new UDPSocket(event_notifier_); + socket = new UDPSocket(extension_->id(), event_notifier_); } DCHECK(socket); @@ -149,7 +157,7 @@ bool SocketDestroyFunction::Prepare() { } void SocketDestroyFunction::Work() { - manager_->Remove(socket_id_); + RemoveSocket(socket_id_); } SocketConnectFunction::SocketConnectFunction() @@ -168,7 +176,7 @@ bool SocketConnectFunction::Prepare() { } void SocketConnectFunction::AsyncWorkStart() { - socket_ = manager_->Get(socket_id_); + socket_ = GetSocket(socket_id_); if (!socket_) { error_ = kSocketNotFoundError; SetResult(Value::CreateIntegerValue(-1)); @@ -227,7 +235,7 @@ bool SocketDisconnectFunction::Prepare() { } void SocketDisconnectFunction::Work() { - Socket* socket = manager_->Get(socket_id_); + Socket* socket = GetSocket(socket_id_); if (socket) socket->Disconnect(); else @@ -244,7 +252,7 @@ bool SocketBindFunction::Prepare() { void SocketBindFunction::Work() { int result = -1; - Socket* socket = manager_->Get(socket_id_); + Socket* socket = GetSocket(socket_id_); if (!socket) { error_ = kSocketNotFoundError; @@ -280,7 +288,7 @@ bool SocketReadFunction::Prepare() { } void SocketReadFunction::AsyncWorkStart() { - Socket* socket = manager_->Get(params_->socket_id); + Socket* socket = GetSocket(params_->socket_id); if (!socket) { error_ = kSocketNotFoundError; OnCompleted(-1, NULL); @@ -328,7 +336,7 @@ bool SocketWriteFunction::Prepare() { } void SocketWriteFunction::AsyncWorkStart() { - Socket* socket = manager_->Get(socket_id_); + Socket* socket = GetSocket(socket_id_); if (!socket) { error_ = kSocketNotFoundError; @@ -361,7 +369,7 @@ bool SocketRecvFromFunction::Prepare() { } void SocketRecvFromFunction::AsyncWorkStart() { - Socket* socket = manager_->Get(params_->socket_id); + Socket* socket = GetSocket(params_->socket_id); if (!socket) { error_ = kSocketNotFoundError; OnCompleted(-1, NULL, std::string(), 0); @@ -416,7 +424,7 @@ bool SocketSendToFunction::Prepare() { } void SocketSendToFunction::AsyncWorkStart() { - socket_ = manager_->Get(socket_id_); + socket_ = GetSocket(socket_id_); if (!socket_) { error_ = kSocketNotFoundError; SetResult(Value::CreateIntegerValue(-1)); @@ -475,7 +483,7 @@ bool SocketSetKeepAliveFunction::Prepare() { void SocketSetKeepAliveFunction::Work() { bool result = false; - Socket* socket = manager_->Get(params_->socket_id); + Socket* socket = GetSocket(params_->socket_id); if (socket) { int delay = 0; if (params_->delay.get()) @@ -501,7 +509,7 @@ bool SocketSetNoDelayFunction::Prepare() { void SocketSetNoDelayFunction::Work() { bool result = false; - Socket* socket = manager_->Get(params_->socket_id); + Socket* socket = GetSocket(params_->socket_id); if (socket) result = socket->SetNoDelay(params_->no_delay); else @@ -522,7 +530,7 @@ bool SocketGetInfoFunction::Prepare() { void SocketGetInfoFunction::Work() { api::socket::SocketInfo info; - Socket* socket = manager_->Get(params_->socket_id); + Socket* socket = GetSocket(params_->socket_id); if (socket) { // This represents what we know about the socket, and does not call through // to the system. diff --git a/chrome/browser/extensions/api/socket/socket_api.h b/chrome/browser/extensions/api/socket/socket_api.h index 4b440cb..3b5e835 100644 --- a/chrome/browser/extensions/api/socket/socket_api.h +++ b/chrome/browser/extensions/api/socket/socket_api.h @@ -37,6 +37,9 @@ class SocketAsyncApiFunction : public AsyncApiFunction { virtual bool PrePrepare() OVERRIDE; virtual bool Respond() OVERRIDE; + Socket* GetSocket(int api_resource_id); + void RemoveSocket(int api_resource_id); + ApiResourceManager<Socket>* manager_; }; diff --git a/chrome/browser/extensions/api/socket/tcp_socket.cc b/chrome/browser/extensions/api/socket/tcp_socket.cc index 4eb9b61..ecda98b 100644 --- a/chrome/browser/extensions/api/socket/tcp_socket.cc +++ b/chrome/browser/extensions/api/socket/tcp_socket.cc @@ -14,22 +14,25 @@ namespace extensions { -TCPSocket::TCPSocket(ApiResourceEventNotifier* event_notifier) - : Socket(event_notifier) { +TCPSocket::TCPSocket(const std::string& owner_extension_id, + ApiResourceEventNotifier* event_notifier) + : Socket(owner_extension_id, event_notifier) { } // For testing. TCPSocket::TCPSocket(net::TCPClientSocket* tcp_client_socket, + const std::string& owner_extension_id, ApiResourceEventNotifier* event_notifier) - : Socket(event_notifier), + : Socket(owner_extension_id, event_notifier), socket_(tcp_client_socket) { } // static TCPSocket* TCPSocket::CreateSocketForTesting( net::TCPClientSocket* tcp_client_socket, + const std::string& owner_extension_id, ApiResourceEventNotifier* event_notifier) { - return new TCPSocket(tcp_client_socket, event_notifier); + return new TCPSocket(tcp_client_socket, owner_extension_id, event_notifier); } TCPSocket::~TCPSocket() { diff --git a/chrome/browser/extensions/api/socket/tcp_socket.h b/chrome/browser/extensions/api/socket/tcp_socket.h index 4d5b944..fc0922b 100644 --- a/chrome/browser/extensions/api/socket/tcp_socket.h +++ b/chrome/browser/extensions/api/socket/tcp_socket.h @@ -23,7 +23,8 @@ class ApiResourceEventNotifier; class TCPSocket : public Socket { public: - explicit TCPSocket(ApiResourceEventNotifier* event_notifier); + TCPSocket(const std::string& owner_extension_id, + ApiResourceEventNotifier* event_notifier); virtual ~TCPSocket(); virtual void Connect(const std::string& address, @@ -48,6 +49,7 @@ class TCPSocket : public Socket { static TCPSocket* CreateSocketForTesting( net::TCPClientSocket* tcp_client_socket, + const std::string& owner_extension_id, ApiResourceEventNotifier* event_notifier); protected: @@ -61,6 +63,7 @@ class TCPSocket : public Socket { int result); TCPSocket(net::TCPClientSocket* tcp_client_socket, + const std::string& owner_extension_id, ApiResourceEventNotifier* event_notifier); scoped_ptr<net::TCPClientSocket> socket_; diff --git a/chrome/browser/extensions/api/socket/tcp_socket_unittest.cc b/chrome/browser/extensions/api/socket/tcp_socket_unittest.cc index 16ae879..54fd8a9 100644 --- a/chrome/browser/extensions/api/socket/tcp_socket_unittest.cc +++ b/chrome/browser/extensions/api/socket/tcp_socket_unittest.cc @@ -43,9 +43,10 @@ class MockTCPSocket : public net::TCPClientSocket { class MockApiResourceEventNotifier : public ApiResourceEventNotifier { public: - MockApiResourceEventNotifier() : ApiResourceEventNotifier(NULL, NULL, - std::string(), - 0, GURL()) {} + MockApiResourceEventNotifier(const std::string& owner_extension_id) + : ApiResourceEventNotifier(NULL, NULL, + owner_extension_id, + 0, GURL()) {} MOCK_METHOD2(OnReadComplete, void(int result_code, const std::string& message)); @@ -65,15 +66,17 @@ class CompleteHandler { DISALLOW_COPY_AND_ASSIGN(CompleteHandler); }; +const std::string FAKE_ID = "abcdefghijklmnopqrst"; TEST(SocketTest, TestTCPSocketRead) { net::AddressList address_list; MockTCPSocket* tcp_client_socket = new MockTCPSocket(address_list); - ApiResourceEventNotifier* notifier = new MockApiResourceEventNotifier(); + ApiResourceEventNotifier* notifier = new MockApiResourceEventNotifier( + FAKE_ID); CompleteHandler handler; scoped_ptr<TCPSocket> socket(TCPSocket::CreateSocketForTesting( - tcp_client_socket, notifier)); + tcp_client_socket, FAKE_ID, notifier)); EXPECT_CALL(*tcp_client_socket, Read(_, _, _)) .Times(1); @@ -88,11 +91,12 @@ TEST(SocketTest, TestTCPSocketRead) { TEST(SocketTest, TestTCPSocketWrite) { net::AddressList address_list; MockTCPSocket* tcp_client_socket = new MockTCPSocket(address_list); - ApiResourceEventNotifier* notifier = new MockApiResourceEventNotifier(); + ApiResourceEventNotifier* notifier = new MockApiResourceEventNotifier( + FAKE_ID); CompleteHandler handler; scoped_ptr<TCPSocket> socket(TCPSocket::CreateSocketForTesting( - tcp_client_socket, notifier)); + tcp_client_socket, FAKE_ID, notifier)); net::CompletionCallback callback; EXPECT_CALL(*tcp_client_socket, Write(_, _, _)) @@ -111,11 +115,12 @@ TEST(SocketTest, TestTCPSocketWrite) { TEST(SocketTest, TestTCPSocketBlockedWrite) { net::AddressList address_list; MockTCPSocket* tcp_client_socket = new MockTCPSocket(address_list); - MockApiResourceEventNotifier* notifier = new MockApiResourceEventNotifier(); + MockApiResourceEventNotifier* notifier = new MockApiResourceEventNotifier( + FAKE_ID); CompleteHandler handler; scoped_ptr<TCPSocket> socket(TCPSocket::CreateSocketForTesting( - tcp_client_socket, notifier)); + tcp_client_socket, FAKE_ID, notifier)); net::CompletionCallback callback; EXPECT_CALL(*tcp_client_socket, Write(_, _, _)) @@ -137,11 +142,12 @@ TEST(SocketTest, TestTCPSocketBlockedWrite) { TEST(SocketTest, TestTCPSocketBlockedWriteReentry) { net::AddressList address_list; MockTCPSocket* tcp_client_socket = new MockTCPSocket(address_list); - MockApiResourceEventNotifier* notifier = new MockApiResourceEventNotifier(); + MockApiResourceEventNotifier* notifier = new MockApiResourceEventNotifier( + FAKE_ID); CompleteHandler handlers[5]; scoped_ptr<TCPSocket> socket(TCPSocket::CreateSocketForTesting( - tcp_client_socket, notifier)); + tcp_client_socket, FAKE_ID, notifier)); net::CompletionCallback callback; EXPECT_CALL(*tcp_client_socket, Write(_, _, _)) @@ -170,10 +176,11 @@ TEST(SocketTest, TestTCPSocketBlockedWriteReentry) { TEST(SocketTest, TestTCPSocketSetNoDelay) { net::AddressList address_list; MockTCPSocket* tcp_client_socket = new MockTCPSocket(address_list); - MockApiResourceEventNotifier* notifier = new MockApiResourceEventNotifier(); + MockApiResourceEventNotifier* notifier = new MockApiResourceEventNotifier( + FAKE_ID); scoped_ptr<TCPSocket> socket(TCPSocket::CreateSocketForTesting( - tcp_client_socket, notifier)); + tcp_client_socket, FAKE_ID, notifier)); bool no_delay = false; EXPECT_CALL(*tcp_client_socket, SetNoDelay(_)) @@ -193,10 +200,11 @@ TEST(SocketTest, TestTCPSocketSetNoDelay) { TEST(SocketTest, TestTCPSocketSetKeepAlive) { net::AddressList address_list; MockTCPSocket* tcp_client_socket = new MockTCPSocket(address_list); - MockApiResourceEventNotifier* notifier = new MockApiResourceEventNotifier(); + MockApiResourceEventNotifier* notifier = new MockApiResourceEventNotifier( + FAKE_ID); scoped_ptr<TCPSocket> socket(TCPSocket::CreateSocketForTesting( - tcp_client_socket, notifier)); + tcp_client_socket, FAKE_ID, notifier)); bool enable = false; int delay = 0; diff --git a/chrome/browser/extensions/api/socket/udp_socket.cc b/chrome/browser/extensions/api/socket/udp_socket.cc index 0ecfe2a..7e7494b 100644 --- a/chrome/browser/extensions/api/socket/udp_socket.cc +++ b/chrome/browser/extensions/api/socket/udp_socket.cc @@ -13,8 +13,9 @@ namespace extensions { -UDPSocket::UDPSocket(ApiResourceEventNotifier* event_notifier) - : Socket(event_notifier), +UDPSocket::UDPSocket(const std::string& owner_extension_id, + ApiResourceEventNotifier* event_notifier) + : Socket(owner_extension_id, event_notifier), socket_(net::DatagramSocket::DEFAULT_BIND, net::RandIntCallback(), NULL, diff --git a/chrome/browser/extensions/api/socket/udp_socket.h b/chrome/browser/extensions/api/socket/udp_socket.h index 9a305fc..6df95f6 100644 --- a/chrome/browser/extensions/api/socket/udp_socket.h +++ b/chrome/browser/extensions/api/socket/udp_socket.h @@ -16,7 +16,8 @@ class ApiResourceEventNotifier; class UDPSocket : public Socket { public: - explicit UDPSocket(ApiResourceEventNotifier* event_notifier); + UDPSocket(const std::string& owner_extension_id, + ApiResourceEventNotifier* event_notifier); virtual ~UDPSocket(); virtual void Connect(const std::string& address, diff --git a/chrome/browser/extensions/api/socket/udp_socket_unittest.cc b/chrome/browser/extensions/api/socket/udp_socket_unittest.cc index 08a864f..2bfed32 100644 --- a/chrome/browser/extensions/api/socket/udp_socket_unittest.cc +++ b/chrome/browser/extensions/api/socket/udp_socket_unittest.cc @@ -32,7 +32,7 @@ static void OnCompleted(int bytes_read, TEST(UDPSocketUnitTest, TestUDPSocketRecvFrom) { MessageLoopForIO io_loop; // for RecvFrom to do its threaded work. - UDPSocket socket(NULL); + UDPSocket socket("abcdefghijklmnopqrst", NULL); // Confirm that we can call two RecvFroms in quick succession without // triggering crbug.com/146606. diff --git a/chrome/browser/extensions/api/usb/usb_api.cc b/chrome/browser/extensions/api/usb/usb_api.cc index 71e4c32..b125afb 100644 --- a/chrome/browser/extensions/api/usb/usb_api.cc +++ b/chrome/browser/extensions/api/usb/usb_api.cc @@ -45,6 +45,15 @@ bool UsbAsyncApiFunction::PrePrepare() { return manager_ != NULL; } +UsbDeviceResource* UsbAsyncApiFunction::GetUsbDeviceResource( + int api_resource_id) { + return manager_->Get(extension_->id(), api_resource_id); +} + +void UsbAsyncApiFunction::RemoveUsbDeviceResource(int api_resource_id) { + manager_->Remove(extension_->id(), api_resource_id); +} + UsbFindDeviceFunction::UsbFindDeviceFunction() : event_notifier_(NULL) {} UsbFindDeviceFunction::~UsbFindDeviceFunction() {} @@ -76,7 +85,8 @@ void UsbFindDeviceFunction::Work() { return; } - UsbDeviceResource* const resource = new UsbDeviceResource(event_notifier_, + UsbDeviceResource* const resource = new UsbDeviceResource(extension_->id(), + event_notifier_, device); Device result; @@ -101,11 +111,13 @@ bool UsbCloseDeviceFunction::Prepare() { } void UsbCloseDeviceFunction::Work() { - UsbDeviceResource* const device = manager_->Get(parameters_->device.handle); + UsbDeviceResource* const device = GetUsbDeviceResource( + parameters_->device.handle); + if (device) device->Close(); - manager_->Remove(parameters_->device.handle); + RemoveUsbDeviceResource(parameters_->device.handle); } bool UsbCloseDeviceFunction::Respond() { @@ -123,7 +135,9 @@ bool UsbControlTransferFunction::Prepare() { } void UsbControlTransferFunction::Work() { - UsbDeviceResource* const device = manager_->Get(parameters_->device.handle); + UsbDeviceResource* const device = GetUsbDeviceResource( + parameters_->device.handle); + if (device) { device->ControlTransfer(parameters_->transfer_info); } @@ -144,7 +158,9 @@ UsbBulkTransferFunction::UsbBulkTransferFunction() {} UsbBulkTransferFunction::~UsbBulkTransferFunction() {} void UsbBulkTransferFunction::Work() { - UsbDeviceResource* const device = manager_->Get(parameters_->device.handle); + UsbDeviceResource* const device = GetUsbDeviceResource( + parameters_->device.handle); + if (device) { device->BulkTransfer(parameters_->transfer_info); } @@ -165,7 +181,9 @@ bool UsbInterruptTransferFunction::Prepare() { } void UsbInterruptTransferFunction::Work() { - UsbDeviceResource* const device = manager_->Get(parameters_->device.handle); + UsbDeviceResource* const device = GetUsbDeviceResource( + parameters_->device.handle); + if (device) { device->InterruptTransfer(parameters_->transfer_info); } @@ -186,7 +204,9 @@ bool UsbIsochronousTransferFunction::Prepare() { } void UsbIsochronousTransferFunction::Work() { - UsbDeviceResource* const device = manager_->Get(parameters_->device.handle); + UsbDeviceResource* const device = GetUsbDeviceResource( + parameters_->device.handle); + if (device) { device->IsochronousTransfer(parameters_->transfer_info); } diff --git a/chrome/browser/extensions/api/usb/usb_api.h b/chrome/browser/extensions/api/usb/usb_api.h index 276a492..22242bd 100644 --- a/chrome/browser/extensions/api/usb/usb_api.h +++ b/chrome/browser/extensions/api/usb/usb_api.h @@ -27,6 +27,9 @@ class UsbAsyncApiFunction : public AsyncApiFunction { virtual bool PrePrepare() OVERRIDE; + UsbDeviceResource* GetUsbDeviceResource(int api_resource_id); + void RemoveUsbDeviceResource(int api_resource_id); + ApiResourceManager<UsbDeviceResource>* manager_; }; diff --git a/chrome/browser/extensions/api/usb/usb_device_resource.cc b/chrome/browser/extensions/api/usb/usb_device_resource.cc index b9112ea..ac4884d 100644 --- a/chrome/browser/extensions/api/usb/usb_device_resource.cc +++ b/chrome/browser/extensions/api/usb/usb_device_resource.cc @@ -153,9 +153,10 @@ static const char* ConvertTransferStatusToErrorString( namespace extensions { -UsbDeviceResource::UsbDeviceResource(ApiResourceEventNotifier* notifier, +UsbDeviceResource::UsbDeviceResource(const std::string& owner_extension_id, + ApiResourceEventNotifier* notifier, UsbDevice* device) - : ApiResource(notifier), device_(device) {} + : ApiResource(owner_extension_id, notifier), device_(device) {} UsbDeviceResource::~UsbDeviceResource() { Close(); diff --git a/chrome/browser/extensions/api/usb/usb_device_resource.h b/chrome/browser/extensions/api/usb/usb_device_resource.h index 9b78c87..a757304 100644 --- a/chrome/browser/extensions/api/usb/usb_device_resource.h +++ b/chrome/browser/extensions/api/usb/usb_device_resource.h @@ -29,7 +29,8 @@ class ApiResourceEventNotifier; // associated with the underlying ApiResource to deliver completion messages. class UsbDeviceResource : public ApiResource { public: - UsbDeviceResource(ApiResourceEventNotifier* notifier, UsbDevice* device); + UsbDeviceResource(const std::string& owner_extension_id, + ApiResourceEventNotifier* notifier, UsbDevice* device); virtual ~UsbDeviceResource(); void Close(); diff --git a/chrome/browser/extensions/extension_function_test_utils.cc b/chrome/browser/extensions/extension_function_test_utils.cc index 2f8bfc6..d352966 100644 --- a/chrome/browser/extensions/extension_function_test_utils.cc +++ b/chrome/browser/extensions/extension_function_test_utils.cc @@ -109,24 +109,38 @@ scoped_refptr<Extension> CreateEmptyExtensionWithLocation( Extension::Location location) { scoped_ptr<base::DictionaryValue> test_extension_value( ParseDictionary("{\"name\": \"Test\", \"version\": \"1.0\"}")); - return CreateExtension(location, test_extension_value.get()); + return CreateExtension(location, test_extension_value.get(), std::string()); +} + +scoped_refptr<Extension> CreateEmptyExtension( + const std::string& id_input) { + scoped_ptr<base::DictionaryValue> test_extension_value( + ParseDictionary("{\"name\": \"Test\", \"version\": \"1.0\"}")); + return CreateExtension(Extension::INTERNAL, test_extension_value.get(), + id_input); } scoped_refptr<Extension> CreateExtension( base::DictionaryValue* test_extension_value) { - return CreateExtension(Extension::INTERNAL, test_extension_value); + return CreateExtension(Extension::INTERNAL, test_extension_value, + std::string()); } scoped_refptr<Extension> CreateExtension( Extension::Location location, - base::DictionaryValue* test_extension_value) { + base::DictionaryValue* test_extension_value, + const std::string& id_input) { std::string error; const FilePath test_extension_path; + std::string id; + if (!id_input.empty()) + CHECK(Extension::GenerateId(id_input, &id)); scoped_refptr<Extension> extension(Extension::Create( test_extension_path, location, *test_extension_value, Extension::NO_FLAGS, + id, &error)); EXPECT_TRUE(error.empty()) << "Could not parse test extension " << error; return extension; diff --git a/chrome/browser/extensions/extension_function_test_utils.h b/chrome/browser/extensions/extension_function_test_utils.h index cea6e78..e42c1d5 100644 --- a/chrome/browser/extensions/extension_function_test_utils.h +++ b/chrome/browser/extensions/extension_function_test_utils.h @@ -54,6 +54,20 @@ scoped_refptr<extensions::Extension> CreateEmptyExtension(); scoped_refptr<extensions::Extension> CreateEmptyExtensionWithLocation( extensions::Extension::Location location); +// Creates an empty extension with a variable ID, for tests that require +// multiple extensions side-by-side having distinct IDs. If not empty, then +// id_input is passed directly to Extension::CreateId() and thus has the same +// behavior as that method. If id_input is empty, then Extension::Create() +// receives an empty explicit ID and generates an appropriate ID for a blank +// extension. +scoped_refptr<extensions::Extension> CreateEmptyExtension( + const std::string& id_input); + +scoped_refptr<extensions::Extension> CreateExtension( + extensions::Extension::Location location, + base::DictionaryValue* test_extension_value, + const std::string& id_input); + // Creates an extension instance with a specified extension value that can be // attached to an ExtensionFunction before running. scoped_refptr<extensions::Extension> CreateExtension( diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 76f3c86..874d393 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1217,6 +1217,7 @@ 'browser/extensions/active_tab_unittest.cc', 'browser/extensions/admin_policy_unittest.cc', 'browser/extensions/api/alarms/alarms_api_unittest.cc', + 'browser/extensions/api/api_resource_manager_unittest.cc', 'browser/extensions/api/content_settings/content_settings_store_unittest.cc', 'browser/extensions/api/content_settings/content_settings_unittest.cc', 'browser/extensions/api/cookies/cookies_unittest.cc', |