diff options
author | dmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-03-11 19:58:09 +0000 |
---|---|---|
committer | dmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-03-11 19:58:09 +0000 |
commit | a65fe6ed7d949e55e9fca008ec7805e4f1f19a23 (patch) | |
tree | c3f575a3c9911f48a0c145806272ab225ae6a0d3 /ppapi/proxy | |
parent | 5102497f1d1dc0cc29967f66ac103185e279b5c6 (diff) | |
download | chromium_src-a65fe6ed7d949e55e9fca008ec7805e4f1f19a23.zip chromium_src-a65fe6ed7d949e55e9fca008ec7805e4f1f19a23.tar.gz chromium_src-a65fe6ed7d949e55e9fca008ec7805e4f1f19a23.tar.bz2 |
Revert 187340 "PPAPI: Remove threading options; it's always on"
> PPAPI: Remove threading options; it's always on
>
> This also re-enables thread checking for the host side resource and var trackers. Before, checking was disabled everywhere.
>
> BUG=159240,92909
>
>
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=186925
> Reverted: https://src.chromium.org/viewvc/chrome?view=rev&revision=186939 due to build errors
>
> Review URL: https://chromiumcodereview.appspot.com/12378050
TBR=dmichael@chromium.org
Review URL: https://codereview.chromium.org/12476028
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@187346 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'ppapi/proxy')
-rw-r--r-- | ppapi/proxy/device_enumeration_resource_helper_unittest.cc | 97 | ||||
-rw-r--r-- | ppapi/proxy/file_chooser_resource_unittest.cc | 8 | ||||
-rw-r--r-- | ppapi/proxy/flash_resource_unittest.cc | 4 | ||||
-rw-r--r-- | ppapi/proxy/locking_resource_releaser.h | 41 | ||||
-rw-r--r-- | ppapi/proxy/plugin_globals.cc | 42 | ||||
-rw-r--r-- | ppapi/proxy/plugin_globals.h | 5 | ||||
-rw-r--r-- | ppapi/proxy/plugin_resource_tracker.cc | 2 | ||||
-rw-r--r-- | ppapi/proxy/plugin_resource_tracker_unittest.cc | 3 | ||||
-rw-r--r-- | ppapi/proxy/plugin_var_tracker.cc | 16 | ||||
-rw-r--r-- | ppapi/proxy/ppapi_proxy_test.cc | 15 | ||||
-rw-r--r-- | ppapi/proxy/ppb_var_unittest.cc | 4 | ||||
-rw-r--r-- | ppapi/proxy/ppp_instance_private_proxy_unittest.cc | 8 | ||||
-rw-r--r-- | ppapi/proxy/ppp_instance_proxy_unittest.cc | 7 | ||||
-rw-r--r-- | ppapi/proxy/printing_resource_unittest.cc | 7 | ||||
-rw-r--r-- | ppapi/proxy/websocket_resource_unittest.cc | 29 |
15 files changed, 119 insertions, 169 deletions
diff --git a/ppapi/proxy/device_enumeration_resource_helper_unittest.cc b/ppapi/proxy/device_enumeration_resource_helper_unittest.cc index d578a9e..330bca3 100644 --- a/ppapi/proxy/device_enumeration_resource_helper_unittest.cc +++ b/ppapi/proxy/device_enumeration_resource_helper_unittest.cc @@ -14,7 +14,6 @@ #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/ppapi_proxy_test.h" #include "ppapi/shared_impl/ppb_device_ref_shared.h" -#include "ppapi/shared_impl/proxy_lock.h" #include "ppapi/shared_impl/var.h" #include "ppapi/thunk/enter.h" #include "ppapi/thunk/ppb_device_ref_api.h" @@ -38,7 +37,7 @@ Connection GetConnection(PluginProxyTestHarness* harness) { bool CompareDeviceRef(PluginVarTracker* var_tracker, PP_Resource resource, const DeviceRefData& expected) { - thunk::EnterResourceNoLock<thunk::PPB_DeviceRef_API> enter(resource, true); + thunk::EnterResource<thunk::PPB_DeviceRef_API> enter(resource, true); if (enter.failed()) return false; @@ -190,7 +189,6 @@ class TestMonitorDeviceChange { static void MonitorDeviceChangeCallback(void* user_data, uint32_t device_count, const PP_Resource devices[]) { - ProxyAutoLock lock; TestMonitorDeviceChange* helper = static_cast<TestMonitorDeviceChange*>(user_data); CHECK(!helper->called_); @@ -220,8 +218,6 @@ class TestMonitorDeviceChange { } // namespace TEST_F(DeviceEnumerationResourceHelperTest, EnumerateDevices) { - ProxyAutoLock lock; - scoped_refptr<TestResource> resource( new TestResource(GetConnection(this), pp_instance())); DeviceEnumerationResourceHelper& device_enumeration = @@ -256,13 +252,11 @@ TEST_F(DeviceEnumerationResourceHelperTest, EnumerateDevices) { data_item.id = "id_2"; data.push_back(data_item); - { - ProxyAutoUnlock unlock; - ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( - PpapiPluginMsg_ResourceReply( - reply_params, - PpapiPluginMsg_DeviceEnumeration_EnumerateDevicesReply(data)))); - } + ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( + PpapiPluginMsg_ResourceReply( + reply_params, + PpapiPluginMsg_DeviceEnumeration_EnumerateDevicesReply(data)))); + EXPECT_TRUE(callback.called()); EXPECT_EQ(PP_OK, callback.result()); EXPECT_EQ(2U, output.count()); @@ -271,8 +265,6 @@ TEST_F(DeviceEnumerationResourceHelperTest, EnumerateDevices) { } TEST_F(DeviceEnumerationResourceHelperTest, MonitorDeviceChange) { - ProxyAutoLock lock; - scoped_refptr<TestResource> resource( new TestResource(GetConnection(this), pp_instance())); DeviceEnumerationResourceHelper& device_enumeration = @@ -301,15 +293,12 @@ TEST_F(DeviceEnumerationResourceHelperTest, MonitorDeviceChange) { helper.SetExpectedResult(data); - { - ProxyAutoUnlock unlock; - // Synthesize a response with no device. - ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( - PpapiPluginMsg_ResourceReply( - reply_params, - PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange( - callback_id, data)))); - } + // Synthesize a response with no device. + ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( + PpapiPluginMsg_ResourceReply( + reply_params, + PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange( + callback_id, data)))); EXPECT_TRUE(helper.called() && helper.same_as_expected()); DeviceRefData data_item; @@ -324,15 +313,12 @@ TEST_F(DeviceEnumerationResourceHelperTest, MonitorDeviceChange) { helper.SetExpectedResult(data); - { - ProxyAutoUnlock unlock; - // Synthesize a response with some devices. - ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( - PpapiPluginMsg_ResourceReply( - reply_params, - PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange( - callback_id, data)))); - } + // Synthesize a response with some devices. + ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( + PpapiPluginMsg_ResourceReply( + reply_params, + PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange( + callback_id, data)))); EXPECT_TRUE(helper.called() && helper.same_as_expected()); TestMonitorDeviceChange helper2(&var_tracker()); @@ -354,30 +340,24 @@ TEST_F(DeviceEnumerationResourceHelperTest, MonitorDeviceChange) { helper.SetExpectedResult(data); helper2.SetExpectedResult(data); - { - ProxyAutoUnlock unlock; - // |helper2| should receive the result while |helper| shouldn't. - ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( - PpapiPluginMsg_ResourceReply( - reply_params, - PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange( - callback_id2, data)))); - } + // |helper2| should receive the result while |helper| shouldn't. + ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( + PpapiPluginMsg_ResourceReply( + reply_params, + PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange( + callback_id2, data)))); EXPECT_TRUE(helper2.called() && helper2.same_as_expected()); EXPECT_FALSE(helper.called()); helper.SetExpectedResult(data); helper2.SetExpectedResult(data); - { - ProxyAutoUnlock unlock; - // Even if a message with |callback_id| arrives. |helper| shouldn't receive - // the result. - ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( - PpapiPluginMsg_ResourceReply( - reply_params, - PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange( - callback_id, data)))); - } + // Even if a message with |callback_id| arrives. |helper| shouldn't receive + // the result. + ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( + PpapiPluginMsg_ResourceReply( + reply_params, + PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange( + callback_id, data)))); EXPECT_FALSE(helper2.called()); EXPECT_FALSE(helper.called()); @@ -393,15 +373,12 @@ TEST_F(DeviceEnumerationResourceHelperTest, MonitorDeviceChange) { sink().ClearMessages(); helper2.SetExpectedResult(data); - { - ProxyAutoUnlock unlock; - // |helper2| shouldn't receive any result any more. - ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( - PpapiPluginMsg_ResourceReply( - reply_params, - PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange( - callback_id2, data)))); - } + // |helper2| shouldn't receive any result any more. + ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( + PpapiPluginMsg_ResourceReply( + reply_params, + PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange( + callback_id2, data)))); EXPECT_FALSE(helper2.called()); } diff --git a/ppapi/proxy/file_chooser_resource_unittest.cc b/ppapi/proxy/file_chooser_resource_unittest.cc index 3ff1022..49c95da 100644 --- a/ppapi/proxy/file_chooser_resource_unittest.cc +++ b/ppapi/proxy/file_chooser_resource_unittest.cc @@ -6,10 +6,10 @@ #include "ppapi/c/dev/ppb_file_chooser_dev.h" #include "ppapi/c/pp_errors.h" #include "ppapi/proxy/file_chooser_resource.h" -#include "ppapi/proxy/locking_resource_releaser.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/ppapi_proxy_test.h" #include "ppapi/shared_impl/proxy_lock.h" +#include "ppapi/shared_impl/scoped_pp_resource.h" #include "ppapi/shared_impl/scoped_pp_var.h" #include "ppapi/shared_impl/var.h" #include "ppapi/thunk/thunk.h" @@ -67,7 +67,7 @@ bool CheckParseAcceptType(const std::string& input, TEST_F(FileChooserResourceTest, Show) { const PPB_FileChooser_Dev_0_6* chooser_iface = thunk::GetPPB_FileChooser_Dev_0_6_Thunk(); - LockingResourceReleaser res( + ScopedPPResource res(ScopedPPResource::PassRef(), chooser_iface->Create(pp_instance(), PP_FILECHOOSERMODE_OPEN, PP_MakeUndefined())); @@ -77,7 +77,7 @@ TEST_F(FileChooserResourceTest, Show) { output.user_data = &dest; int32_t result = chooser_iface->Show( - res.get(), output, PP_MakeCompletionCallback(&DoNothingCallback, NULL)); + res, output, PP_MakeCompletionCallback(&DoNothingCallback, NULL)); ASSERT_EQ(PP_OK_COMPLETIONPENDING, result); // Should have sent a "show" message. @@ -105,7 +105,7 @@ TEST_F(FileChooserResourceTest, Show) { // Should have populated our vector. ASSERT_EQ(1u, dest.size()); - LockingResourceReleaser dest_deletor(dest[0]); // Ensure it's cleaned up. + ScopedPPResource dest_deletor(dest[0]); // Ensure it's cleaned up. const PPB_FileRef_1_0* file_ref_iface = thunk::GetPPB_FileRef_1_0_Thunk(); EXPECT_EQ(PP_FILESYSTEMTYPE_EXTERNAL, diff --git a/ppapi/proxy/flash_resource_unittest.cc b/ppapi/proxy/flash_resource_unittest.cc index 1a5e7c6..ab22077 100644 --- a/ppapi/proxy/flash_resource_unittest.cc +++ b/ppapi/proxy/flash_resource_unittest.cc @@ -5,9 +5,9 @@ #include "ppapi/c/dev/ppb_video_capture_dev.h" #include "ppapi/c/pp_errors.h" #include "ppapi/c/private/ppb_flash.h" -#include "ppapi/proxy/locking_resource_releaser.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/ppapi_proxy_test.h" +#include "ppapi/shared_impl/scoped_pp_resource.h" #include "ppapi/thunk/thunk.h" namespace ppapi { @@ -43,7 +43,7 @@ TEST_F(FlashResourceTest, EnumerateVideoCaptureDevices) { sink().AddFilter(&enumerate_video_devices_handler); // Set up the arguments to the call. - LockingResourceReleaser video_capture( + ScopedPPResource video_capture(ScopedPPResource::PassRef(), ::ppapi::thunk::GetPPB_VideoCapture_Dev_0_3_Thunk()->Create( pp_instance())); std::vector<PP_Resource> unused; diff --git a/ppapi/proxy/locking_resource_releaser.h b/ppapi/proxy/locking_resource_releaser.h deleted file mode 100644 index d390ac4..0000000 --- a/ppapi/proxy/locking_resource_releaser.h +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) 2013 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. - -#ifndef PPAPI_PROXY_LOCKING_RESOURCE_RELEASER_H_ -#define PPAPI_PROXY_LOCKING_RESOURCE_RELEASER_H_ - -#include "ppapi/shared_impl/ppapi_globals.h" -#include "ppapi/shared_impl/proxy_lock.h" -#include "ppapi/shared_impl/resource_tracker.h" - -namespace ppapi { -namespace proxy { - -// LockingResourceReleaser is a simple RAII class for releasing a resource at -// the end of scope. This acquires the ProxyLock before releasing the resource. -// It is for use in unit tests. Most proxy or implementation code should use -// ScopedPPResource instead. Unit tests sometimes can't use ScopedPPResource -// because it asserts that the ProxyLock is already held. -class LockingResourceReleaser { - public: - explicit LockingResourceReleaser(PP_Resource resource) - : resource_(resource) { - } - ~LockingResourceReleaser() { - ProxyAutoLock lock; - PpapiGlobals::Get()->GetResourceTracker()->ReleaseResource(resource_); - } - - PP_Resource get() { return resource_; } - - private: - PP_Resource resource_; - - DISALLOW_COPY_AND_ASSIGN(LockingResourceReleaser); -}; - -} // namespace proxy -} // namespace ppapi - -#endif // PPAPI_PROXY_LOCKING_RESOURCE_RELEASER_H_ diff --git a/ppapi/proxy/plugin_globals.cc b/ppapi/proxy/plugin_globals.cc index 3083832..2e18ac5f 100644 --- a/ppapi/proxy/plugin_globals.cc +++ b/ppapi/proxy/plugin_globals.cc @@ -49,36 +49,40 @@ PluginGlobals* PluginGlobals::plugin_globals_ = NULL; PluginGlobals::PluginGlobals() : ppapi::PpapiGlobals(), plugin_proxy_delegate_(NULL), - callback_tracker_(new CallbackTracker) { + callback_tracker_(new CallbackTracker), + loop_for_main_thread_( + new MessageLoopResource(MessageLoopResource::ForMainThread())) { +#if defined(ENABLE_PEPPER_THREADING) + enable_threading_ = true; +#else + enable_threading_ = false; +#endif + DCHECK(!plugin_globals_); plugin_globals_ = this; - - // ResourceTracker asserts that we have the lock when we add new resources, - // so we lock when creating the MessageLoopResource even though there is no - // chance of race conditions. - ProxyAutoLock lock; - loop_for_main_thread_ = - new MessageLoopResource(MessageLoopResource::ForMainThread()); } PluginGlobals::PluginGlobals(PerThreadForTest per_thread_for_test) : ppapi::PpapiGlobals(per_thread_for_test), plugin_proxy_delegate_(NULL), callback_tracker_(new CallbackTracker) { +#if defined(ENABLE_PEPPER_THREADING) + enable_threading_ = true; +#else + enable_threading_ = false; +#endif DCHECK(!plugin_globals_); } PluginGlobals::~PluginGlobals() { DCHECK(plugin_globals_ == this || !plugin_globals_); - { - ProxyAutoLock lock; - // Release the main-thread message loop. We should have the last reference - // count, so this will delete the MessageLoop resource. We do this before - // we clear plugin_globals_, because the Resource destructor tries to access - // this PluginGlobals. - DCHECK(!loop_for_main_thread_ || loop_for_main_thread_->HasOneRef()); - loop_for_main_thread_ = NULL; - } + // Release the main-thread message loop. We should have the last reference + // count, so this will delete the MessageLoop resource. We do this before + // we clear plugin_globals_, because the Resource destructor tries to access + // this PluginGlobals. + DCHECK(!loop_for_main_thread_ || loop_for_main_thread_->HasOneRef()); + loop_for_main_thread_ = NULL; + plugin_globals_ = NULL; } @@ -127,7 +131,9 @@ void PluginGlobals::PreCacheFontForFlash(const void* logfontw) { } base::Lock* PluginGlobals::GetProxyLock() { - return &proxy_lock_; + if (enable_threading_) + return &proxy_lock_; + return NULL; } void PluginGlobals::LogWithSource(PP_Instance instance, diff --git a/ppapi/proxy/plugin_globals.h b/ppapi/proxy/plugin_globals.h index 37fdc1a..4da6d5f 100644 --- a/ppapi/proxy/plugin_globals.h +++ b/ppapi/proxy/plugin_globals.h @@ -114,6 +114,10 @@ class PPAPI_PROXY_EXPORT PluginGlobals : public PpapiGlobals { // The embedder should call this function when the command line is known. void set_command_line(const std::string& c) { command_line_ = c; } + // Sets whether threadsafety is supported. Defaults to whether the + // ENABLE_PEPPER_THREADING build flag is set. + void set_enable_threading(bool enable) { enable_threading_ = enable; } + private: class BrowserSender; @@ -127,6 +131,7 @@ class PPAPI_PROXY_EXPORT PluginGlobals : public PpapiGlobals { PluginVarTracker plugin_var_tracker_; scoped_refptr<CallbackTracker> callback_tracker_; + bool enable_threading_; // Indicates whether we'll use the lock. base::Lock proxy_lock_; scoped_ptr<base::ThreadLocalStorage::Slot> msg_loop_slot_; diff --git a/ppapi/proxy/plugin_resource_tracker.cc b/ppapi/proxy/plugin_resource_tracker.cc index 12e9d3f..fb81857 100644 --- a/ppapi/proxy/plugin_resource_tracker.cc +++ b/ppapi/proxy/plugin_resource_tracker.cc @@ -17,7 +17,7 @@ namespace ppapi { namespace proxy { -PluginResourceTracker::PluginResourceTracker() : ResourceTracker(THREAD_SAFE) { +PluginResourceTracker::PluginResourceTracker() { } PluginResourceTracker::~PluginResourceTracker() { diff --git a/ppapi/proxy/plugin_resource_tracker_unittest.cc b/ppapi/proxy/plugin_resource_tracker_unittest.cc index 9a60864..59b64db 100644 --- a/ppapi/proxy/plugin_resource_tracker_unittest.cc +++ b/ppapi/proxy/plugin_resource_tracker_unittest.cc @@ -9,7 +9,6 @@ #include "ppapi/proxy/plugin_resource_tracker.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/ppapi_proxy_test.h" -#include "ppapi/shared_impl/proxy_lock.h" namespace ppapi { namespace proxy { @@ -41,8 +40,6 @@ class PluginResourceTrackerTest : public PluginProxyTest { }; TEST_F(PluginResourceTrackerTest, PluginResourceForHostResource) { - ProxyAutoLock lock; - PP_Resource host_resource = 0x5678; HostResource serialized; diff --git a/ppapi/proxy/plugin_var_tracker.cc b/ppapi/proxy/plugin_var_tracker.cc index 9aa88b6..384eb3ec 100644 --- a/ppapi/proxy/plugin_var_tracker.cc +++ b/ppapi/proxy/plugin_var_tracker.cc @@ -31,7 +31,7 @@ bool PluginVarTracker::HostVar::operator<(const HostVar& other) const { return host_object_id < other.host_object_id; } -PluginVarTracker::PluginVarTracker() : VarTracker(THREAD_SAFE) { +PluginVarTracker::PluginVarTracker() { } PluginVarTracker::~PluginVarTracker() { @@ -39,7 +39,7 @@ PluginVarTracker::~PluginVarTracker() { PP_Var PluginVarTracker::ReceiveObjectPassRef(const PP_Var& host_var, PluginDispatcher* dispatcher) { - CheckThreadingPreconditions(); + DCHECK(CalledOnValidThread()); DCHECK(host_var.type == PP_VARTYPE_OBJECT); // Get the object. @@ -65,7 +65,7 @@ PP_Var PluginVarTracker::ReceiveObjectPassRef(const PP_Var& host_var, PP_Var PluginVarTracker::TrackObjectWithNoReference( const PP_Var& host_var, PluginDispatcher* dispatcher) { - CheckThreadingPreconditions(); + DCHECK(CalledOnValidThread()); DCHECK(host_var.type == PP_VARTYPE_OBJECT); // Get the object. @@ -83,7 +83,7 @@ PP_Var PluginVarTracker::TrackObjectWithNoReference( void PluginVarTracker::StopTrackingObjectWithNoReference( const PP_Var& plugin_var) { - CheckThreadingPreconditions(); + DCHECK(CalledOnValidThread()); DCHECK(plugin_var.type == PP_VARTYPE_OBJECT); VarMap::iterator found = GetLiveVar(plugin_var); @@ -98,7 +98,8 @@ void PluginVarTracker::StopTrackingObjectWithNoReference( } PP_Var PluginVarTracker::GetHostObject(const PP_Var& plugin_object) const { - CheckThreadingPreconditions(); + DCHECK(CalledOnValidThread()); + if (plugin_object.type != PP_VARTYPE_OBJECT) { NOTREACHED(); return PP_MakeUndefined(); @@ -119,7 +120,8 @@ PP_Var PluginVarTracker::GetHostObject(const PP_Var& plugin_object) const { PluginDispatcher* PluginVarTracker::DispatcherForPluginObject( const PP_Var& plugin_object) const { - CheckThreadingPreconditions(); + DCHECK(CalledOnValidThread()); + if (plugin_object.type != PP_VARTYPE_OBJECT) return NULL; @@ -135,7 +137,7 @@ PluginDispatcher* PluginVarTracker::DispatcherForPluginObject( void PluginVarTracker::ReleaseHostObject(PluginDispatcher* dispatcher, const PP_Var& host_object) { - CheckThreadingPreconditions(); + DCHECK(CalledOnValidThread()); DCHECK(host_object.type == PP_VARTYPE_OBJECT); // Convert the host object to a normal var valid in the plugin. diff --git a/ppapi/proxy/ppapi_proxy_test.cc b/ppapi/proxy/ppapi_proxy_test.cc index 9034d8f..00947e3 100644 --- a/ppapi/proxy/ppapi_proxy_test.cc +++ b/ppapi/proxy/ppapi_proxy_test.cc @@ -171,8 +171,6 @@ Dispatcher* PluginProxyTestHarness::GetDispatcher() { void PluginProxyTestHarness::SetUpHarness() { // These must be first since the dispatcher set-up uses them. CreatePluginGlobals(); - // Some of the methods called during set-up check that the lock is held. - ProxyAutoLock lock; resource_tracker().DidCreateInstance(pp_instance()); @@ -198,8 +196,6 @@ void PluginProxyTestHarness::SetUpHarnessWithChannel( bool is_client) { // These must be first since the dispatcher set-up uses them. CreatePluginGlobals(); - // Some of the methods called during set-up check that the lock is held. - ProxyAutoLock lock; resource_tracker().DidCreateInstance(pp_instance()); plugin_delegate_mock_.Init(ipc_message_loop, shutdown_event); @@ -218,15 +214,10 @@ void PluginProxyTestHarness::SetUpHarnessWithChannel( } void PluginProxyTestHarness::TearDownHarness() { - { - // Some of the methods called during tear-down check that the lock is held. - ProxyAutoLock lock; - - plugin_dispatcher_->DidDestroyInstance(pp_instance()); - plugin_dispatcher_.reset(); + plugin_dispatcher_->DidDestroyInstance(pp_instance()); + plugin_dispatcher_.reset(); - resource_tracker().DidDeleteInstance(pp_instance()); - } + resource_tracker().DidDeleteInstance(pp_instance()); plugin_globals_.reset(); } diff --git a/ppapi/proxy/ppb_var_unittest.cc b/ppapi/proxy/ppb_var_unittest.cc index 0ed0f47..bf6147d 100644 --- a/ppapi/proxy/ppb_var_unittest.cc +++ b/ppapi/proxy/ppb_var_unittest.cc @@ -164,7 +164,11 @@ class RemoveRefVarThreadDelegate : public base::PlatformThread::Delegate { } // namespace +#ifdef ENABLE_PEPPER_THREADING TEST_F(PPB_VarTest, Threads) { +#else +TEST_F(PPB_VarTest, DISABLED_Threads) { +#endif std::vector<base::PlatformThreadHandle> create_var_threads(kNumThreads); std::vector<CreateVarThreadDelegate> create_var_delegates; // The strings that the threads will re-extract from Vars (so we can check diff --git a/ppapi/proxy/ppp_instance_private_proxy_unittest.cc b/ppapi/proxy/ppp_instance_private_proxy_unittest.cc index 317f4d4..df1af0c 100644 --- a/ppapi/proxy/ppp_instance_private_proxy_unittest.cc +++ b/ppapi/proxy/ppp_instance_private_proxy_unittest.cc @@ -147,7 +147,15 @@ class PPP_Instance_Private_ProxyTest : public TwoWayTest { } // namespace +// TODO(raymes): This #ifdef is only here because we check the state of the +// plugin globals on the main thread, rather than the plugin thread which causes +// the thread checker to fail. Once ENABLE_PEPPER_THREADING is the default, +// this will be safe to do anyway, so we can remove this. +#ifdef ENABLE_PEPPER_THREADING TEST_F(PPP_Instance_Private_ProxyTest, PPPInstancePrivate) { +#else +TEST_F(PPP_Instance_Private_ProxyTest, DISABLED_PPPInstancePrivate) { +#endif // This test controls its own instance; we can't use the one that // PluginProxyTestHarness provides. ASSERT_NE(kInstance, pp_instance()); diff --git a/ppapi/proxy/ppp_instance_proxy_unittest.cc b/ppapi/proxy/ppp_instance_proxy_unittest.cc index e2df26e..91775b0 100644 --- a/ppapi/proxy/ppp_instance_proxy_unittest.cc +++ b/ppapi/proxy/ppp_instance_proxy_unittest.cc @@ -10,10 +10,10 @@ #include "ppapi/c/ppb_url_loader.h" #include "ppapi/c/ppp_instance.h" #include "ppapi/c/private/ppb_flash_fullscreen.h" -#include "ppapi/proxy/locking_resource_releaser.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/ppapi_proxy_test.h" #include "ppapi/shared_impl/ppb_view_shared.h" +#include "ppapi/shared_impl/scoped_pp_resource.h" namespace ppapi { namespace proxy { @@ -163,10 +163,11 @@ TEST_F(PPP_Instance_ProxyTest, PPPInstance1_0) { data.clip_rect = expected_clip; data.device_scale = 1.0f; ResetReceived(); - LockingResourceReleaser view_resource( + ScopedPPResource view_resource( + ScopedPPResource::PassRef(), (new PPB_View_Shared(OBJECT_IS_IMPL, expected_instance, data))->GetReference()); - ppp_instance->DidChangeView(expected_instance, view_resource.get()); + ppp_instance->DidChangeView(expected_instance, view_resource); did_change_view_called.Wait(); EXPECT_EQ(received_instance, expected_instance); EXPECT_EQ(received_position.point.x, expected_position.point.x); diff --git a/ppapi/proxy/printing_resource_unittest.cc b/ppapi/proxy/printing_resource_unittest.cc index ed96364..980147f 100644 --- a/ppapi/proxy/printing_resource_unittest.cc +++ b/ppapi/proxy/printing_resource_unittest.cc @@ -7,11 +7,11 @@ #include "base/message_loop.h" #include "ppapi/c/dev/ppb_printing_dev.h" #include "ppapi/c/pp_errors.h" -#include "ppapi/proxy/locking_resource_releaser.h" #include "ppapi/proxy/printing_resource.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/ppapi_proxy_test.h" #include "ppapi/thunk/thunk.h" +#include "ppapi/shared_impl/scoped_pp_resource.h" namespace ppapi { namespace proxy { @@ -47,12 +47,13 @@ TEST_F(PrintingResourceTest, GetDefaultPrintSettings) { const PPB_Printing_Dev_0_7* printing_iface = thunk::GetPPB_Printing_Dev_0_7_Thunk(); - LockingResourceReleaser res(printing_iface->Create(pp_instance())); + ScopedPPResource res(ScopedPPResource::PassRef(), + printing_iface->Create(pp_instance())); PP_PrintSettings_Dev output_settings; int32_t result = printing_iface->GetDefaultPrintSettings( - res.get(), &output_settings, PP_MakeCompletionCallback(&Callback, NULL)); + res, &output_settings, PP_MakeCompletionCallback(&Callback, NULL)); ASSERT_EQ(PP_OK_COMPLETIONPENDING, result); // Should have sent a "GetDefaultPrintSettings" message. diff --git a/ppapi/proxy/websocket_resource_unittest.cc b/ppapi/proxy/websocket_resource_unittest.cc index 89ea831..56012b3 100644 --- a/ppapi/proxy/websocket_resource_unittest.cc +++ b/ppapi/proxy/websocket_resource_unittest.cc @@ -7,14 +7,10 @@ #include "ppapi/c/pp_errors.h" #include "ppapi/c/ppb_websocket.h" #include "ppapi/c/ppb_var.h" -#include "ppapi/proxy/locking_resource_releaser.h" #include "ppapi/proxy/websocket_resource.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/ppapi_proxy_test.h" -#include "ppapi/shared_impl/ppapi_globals.h" #include "ppapi/shared_impl/ppb_var_shared.h" -#include "ppapi/shared_impl/proxy_lock.h" -#include "ppapi/shared_impl/resource_tracker.h" #include "ppapi/shared_impl/scoped_pp_resource.h" #include "ppapi/shared_impl/scoped_pp_var.h" #include "ppapi/shared_impl/tracked_callback.h" @@ -63,10 +59,11 @@ TEST_F(WebSocketResourceTest, Connect) { PP_Var url_var = MakeStringVar(url); PP_Var protocols[] = { MakeStringVar(protocol0), MakeStringVar(protocol1) }; - LockingResourceReleaser res(websocket_iface->Create(pp_instance())); + ScopedPPResource res(ScopedPPResource::PassRef(), + websocket_iface->Create(pp_instance())); - int32_t result = websocket_iface->Connect(res.get(), url_var, protocols, 2, - MakeCallback()); + int32_t result = + websocket_iface->Connect(res, url_var, protocols, 2, MakeCallback()); ASSERT_EQ(PP_OK_COMPLETIONPENDING, result); // Should be sent a "Connect" message. @@ -97,17 +94,18 @@ TEST_F(WebSocketResourceTest, UnsolicitedReplies) { const PPB_WebSocket_1_0* websocket_iface = thunk::GetPPB_WebSocket_1_0_Thunk(); - LockingResourceReleaser res(websocket_iface->Create(pp_instance())); + ScopedPPResource res(ScopedPPResource::PassRef(), + websocket_iface->Create(pp_instance())); // Check if BufferedAmountReply is handled. - ResourceMessageReplyParams reply_params(res.get(), 0); + ResourceMessageReplyParams reply_params(res, 0); reply_params.set_result(PP_OK); ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( PpapiPluginMsg_ResourceReply( reply_params, PpapiPluginMsg_WebSocket_BufferedAmountReply(19760227u)))); - uint64_t amount = websocket_iface->GetBufferedAmount(res.get()); + uint64_t amount = websocket_iface->GetBufferedAmount(res); EXPECT_EQ(19760227u, amount); // Check if StateReply is handled. @@ -117,7 +115,7 @@ TEST_F(WebSocketResourceTest, UnsolicitedReplies) { PpapiPluginMsg_WebSocket_StateReply( static_cast<int32_t>(PP_WEBSOCKETREADYSTATE_CLOSING))))); - PP_WebSocketReadyState state = websocket_iface->GetReadyState(res.get()); + PP_WebSocketReadyState state = websocket_iface->GetReadyState(res); EXPECT_EQ(PP_WEBSOCKETREADYSTATE_CLOSING, state); } @@ -128,11 +126,12 @@ TEST_F(WebSocketResourceTest, MessageError) { std::string url("ws://ws.google.com"); PP_Var url_var = MakeStringVar(url); - LockingResourceReleaser res(websocket_iface->Create(pp_instance())); + ScopedPPResource res(ScopedPPResource::PassRef(), + websocket_iface->Create(pp_instance())); // Establish the connection virtually. int32_t result = - websocket_iface->Connect(res.get(), url_var, NULL, 0, MakeCallback()); + websocket_iface->Connect(res, url_var, NULL, 0, MakeCallback()); ASSERT_EQ(PP_OK_COMPLETIONPENDING, result); ResourceMessageCallParams params; @@ -151,11 +150,11 @@ TEST_F(WebSocketResourceTest, MessageError) { EXPECT_TRUE(g_callback_called); PP_Var message; - result = websocket_iface->ReceiveMessage(res.get(), &message, MakeCallback()); + result = websocket_iface->ReceiveMessage(res, &message, MakeCallback()); EXPECT_FALSE(g_callback_called); // Synthesize a WebSocket_ErrorReply message. - ResourceMessageReplyParams error_reply_params(res.get(), 0); + ResourceMessageReplyParams error_reply_params(res, 0); error_reply_params.set_result(PP_OK); ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived( PpapiPluginMsg_ResourceReply(error_reply_params, |