summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-03-21 03:58:09 +0000
committerdmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-03-21 03:58:09 +0000
commitf9bc5791e39b5602b7f41716a177c8dc64ca6f00 (patch)
tree05057334fdd68e952fa47a401d7076241063b3fb
parent205e88d6758032258564a8f514b27dd1fa3c5c5e (diff)
downloadchromium_src-f9bc5791e39b5602b7f41716a177c8dc64ca6f00.zip
chromium_src-f9bc5791e39b5602b7f41716a177c8dc64ca6f00.tar.gz
chromium_src-f9bc5791e39b5602b7f41716a177c8dc64ca6f00.tar.bz2
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 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=187340 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=187427 Reverted: https://src.chromium.org/viewvc/chrome?view=rev&revision=187668 due to a failing check in Canary, which was fixed here: https://src.chromium.org/viewvc/chrome?view=rev&revision=187681 Review URL: https://chromiumcodereview.appspot.com/12378050 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@189518 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--build/common.gypi9
-rw-r--r--content/browser/ppapi_plugin_process_host.cc3
-rw-r--r--content/ppapi_plugin/ppapi_thread.cc2
-rw-r--r--ppapi/ppapi_ipc_untrusted.gyp5
-rw-r--r--ppapi/ppapi_proxy.gypi1
-rw-r--r--ppapi/ppapi_proxy_untrusted.gyp5
-rw-r--r--ppapi/ppapi_shared_untrusted.gyp5
-rw-r--r--ppapi/proxy/device_enumeration_resource_helper_unittest.cc97
-rw-r--r--ppapi/proxy/file_chooser_resource_unittest.cc8
-rw-r--r--ppapi/proxy/flash_resource_unittest.cc4
-rw-r--r--ppapi/proxy/locking_resource_releaser.h41
-rw-r--r--ppapi/proxy/plugin_globals.cc42
-rw-r--r--ppapi/proxy/plugin_globals.h5
-rw-r--r--ppapi/proxy/plugin_resource_tracker.cc2
-rw-r--r--ppapi/proxy/plugin_resource_tracker_unittest.cc3
-rw-r--r--ppapi/proxy/plugin_var_tracker.cc16
-rw-r--r--ppapi/proxy/ppapi_proxy_test.cc15
-rw-r--r--ppapi/proxy/ppb_var_unittest.cc4
-rw-r--r--ppapi/proxy/ppp_instance_private_proxy_unittest.cc8
-rw-r--r--ppapi/proxy/ppp_instance_proxy_unittest.cc7
-rw-r--r--ppapi/proxy/printing_resource_unittest.cc7
-rw-r--r--ppapi/proxy/websocket_resource_unittest.cc29
-rw-r--r--ppapi/shared_impl/resource_tracker.cc26
-rw-r--r--ppapi/shared_impl/resource_tracker.h30
-rw-r--r--ppapi/shared_impl/test_globals.cc2
-rw-r--r--ppapi/shared_impl/test_globals.h2
-rw-r--r--ppapi/shared_impl/var_tracker.cc50
-rw-r--r--ppapi/shared_impl/var_tracker.h29
-rw-r--r--ppapi/tests/test_case.h5
-rw-r--r--webkit/plugins/plugin_switches.cc3
-rw-r--r--webkit/plugins/plugin_switches.h1
-rw-r--r--webkit/plugins/ppapi/host_globals.cc8
-rw-r--r--webkit/plugins/ppapi/host_var_tracker.cc14
33 files changed, 261 insertions, 227 deletions
diff --git a/build/common.gypi b/build/common.gypi
index 1f4e9fa..f267679 100644
--- a/build/common.gypi
+++ b/build/common.gypi
@@ -322,11 +322,6 @@
# Webrtc compilation is enabled by default. Set to 0 to disable.
'enable_webrtc%': 1,
- # PPAPI by default does not support plugins making calls off the main
- # thread. Set to 1 to turn on experimental support for out-of-process
- # plugins to make call of the main thread.
- 'enable_pepper_threading%': 1,
-
# Enables use of the session service, which is enabled by default.
# Support for disabling depends on the platform.
'enable_session_service%': 1,
@@ -680,7 +675,6 @@
'use_x11%': '<(use_x11)',
'use_gnome_keyring%': '<(use_gnome_keyring)',
'linux_fpic%': '<(linux_fpic)',
- 'enable_pepper_threading%': '<(enable_pepper_threading)',
'chromeos%': '<(chromeos)',
'enable_viewport%': '<(enable_viewport)',
'enable_hidpi%': '<(enable_hidpi)',
@@ -1783,9 +1777,6 @@
['proprietary_codecs==1', {
'defines': ['USE_PROPRIETARY_CODECS'],
}],
- ['enable_pepper_threading==1', {
- 'defines': ['ENABLE_PEPPER_THREADING'],
- }],
['enable_viewport==1', {
'defines': ['ENABLE_VIEWPORT'],
}],
diff --git a/content/browser/ppapi_plugin_process_host.cc b/content/browser/ppapi_plugin_process_host.cc
index 236477a..e91b2b4 100644
--- a/content/browser/ppapi_plugin_process_host.cc
+++ b/content/browser/ppapi_plugin_process_host.cc
@@ -275,10 +275,9 @@ bool PpapiPluginProcessHost::Init(const PepperPluginInfo& info) {
arraysize(kCommonForwardSwitches));
if (!is_broker_) {
- // TODO(vtl): Stop passing flash args in the command line, on windows is
+ // TODO(vtl): Stop passing flash args in the command line, or windows is
// going to explode.
static const char* kPluginForwardSwitches[] = {
- switches::kDisablePepperThreading,
switches::kDisableSeccompFilterSandbox,
#if defined(OS_MACOSX)
switches::kEnableSandboxLogging,
diff --git a/content/ppapi_plugin/ppapi_thread.cc b/content/ppapi_plugin/ppapi_thread.cc
index e0bfb78..0f718af 100644
--- a/content/ppapi_plugin/ppapi_thread.cc
+++ b/content/ppapi_plugin/ppapi_thread.cc
@@ -93,8 +93,6 @@ PpapiThread::PpapiThread(const CommandLine& command_line, bool is_broker)
globals->set_plugin_proxy_delegate(this);
globals->set_command_line(
command_line.GetSwitchValueASCII(switches::kPpapiFlashArgs));
- globals->set_enable_threading(
- !command_line.HasSwitch(switches::kDisablePepperThreading));
webkit_platform_support_.reset(new PpapiWebKitPlatformSupportImpl);
WebKit::initialize(webkit_platform_support_.get());
diff --git a/ppapi/ppapi_ipc_untrusted.gyp b/ppapi/ppapi_ipc_untrusted.gyp
index fd55a58..45ffd0f 100644
--- a/ppapi/ppapi_ipc_untrusted.gyp
+++ b/ppapi/ppapi_ipc_untrusted.gyp
@@ -23,11 +23,6 @@
'nlib_target': 'libppapi_ipc_untrusted.a',
'build_glibc': 0,
'build_newlib': 1,
- 'defines': [
- # Enable threading for the untrusted side of the proxy.
- # TODO(bbudge) remove when this is the default.
- 'ENABLE_PEPPER_THREADING',
- ],
},
'include_dirs': [
'..',
diff --git a/ppapi/ppapi_proxy.gypi b/ppapi/ppapi_proxy.gypi
index 838bbc6..cdc222a 100644
--- a/ppapi/ppapi_proxy.gypi
+++ b/ppapi/ppapi_proxy.gypi
@@ -69,6 +69,7 @@
'proxy/interface_list.h',
'proxy/interface_proxy.cc',
'proxy/interface_proxy.h',
+ 'proxy/locking_resource_releaser.h',
'proxy/plugin_array_buffer_var.cc',
'proxy/plugin_array_buffer_var.h',
'proxy/plugin_dispatcher.cc',
diff --git a/ppapi/ppapi_proxy_untrusted.gyp b/ppapi/ppapi_proxy_untrusted.gyp
index 49fae82..2b87d7b 100644
--- a/ppapi/ppapi_proxy_untrusted.gyp
+++ b/ppapi/ppapi_proxy_untrusted.gyp
@@ -22,11 +22,6 @@
'nlib_target': 'libppapi_proxy_untrusted.a',
'build_glibc': 0,
'build_newlib': 1,
- 'defines': [
- # Enable threading for the untrusted side of the proxy.
- # TODO(bbudge) remove when this is the default.
- 'ENABLE_PEPPER_THREADING',
- ],
},
'include_dirs': [
'..',
diff --git a/ppapi/ppapi_shared_untrusted.gyp b/ppapi/ppapi_shared_untrusted.gyp
index e085f60..2c0f8bd 100644
--- a/ppapi/ppapi_shared_untrusted.gyp
+++ b/ppapi/ppapi_shared_untrusted.gyp
@@ -23,11 +23,6 @@
'nlib_target': 'libppapi_shared_untrusted.a',
'build_glibc': 0,
'build_newlib': 1,
- 'defines': [
- # Enable threading for the untrusted side of the proxy.
- # TODO(bbudge) remove when this is the default.
- 'ENABLE_PEPPER_THREADING',
- ],
},
'include_dirs': [
'..',
diff --git a/ppapi/proxy/device_enumeration_resource_helper_unittest.cc b/ppapi/proxy/device_enumeration_resource_helper_unittest.cc
index 330bca3..d578a9e 100644
--- a/ppapi/proxy/device_enumeration_resource_helper_unittest.cc
+++ b/ppapi/proxy/device_enumeration_resource_helper_unittest.cc
@@ -14,6 +14,7 @@
#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"
@@ -37,7 +38,7 @@ Connection GetConnection(PluginProxyTestHarness* harness) {
bool CompareDeviceRef(PluginVarTracker* var_tracker,
PP_Resource resource,
const DeviceRefData& expected) {
- thunk::EnterResource<thunk::PPB_DeviceRef_API> enter(resource, true);
+ thunk::EnterResourceNoLock<thunk::PPB_DeviceRef_API> enter(resource, true);
if (enter.failed())
return false;
@@ -189,6 +190,7 @@ 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_);
@@ -218,6 +220,8 @@ class TestMonitorDeviceChange {
} // namespace
TEST_F(DeviceEnumerationResourceHelperTest, EnumerateDevices) {
+ ProxyAutoLock lock;
+
scoped_refptr<TestResource> resource(
new TestResource(GetConnection(this), pp_instance()));
DeviceEnumerationResourceHelper& device_enumeration =
@@ -252,11 +256,13 @@ TEST_F(DeviceEnumerationResourceHelperTest, EnumerateDevices) {
data_item.id = "id_2";
data.push_back(data_item);
- ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived(
- PpapiPluginMsg_ResourceReply(
- reply_params,
- PpapiPluginMsg_DeviceEnumeration_EnumerateDevicesReply(data))));
-
+ {
+ ProxyAutoUnlock unlock;
+ 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());
@@ -265,6 +271,8 @@ TEST_F(DeviceEnumerationResourceHelperTest, EnumerateDevices) {
}
TEST_F(DeviceEnumerationResourceHelperTest, MonitorDeviceChange) {
+ ProxyAutoLock lock;
+
scoped_refptr<TestResource> resource(
new TestResource(GetConnection(this), pp_instance()));
DeviceEnumerationResourceHelper& device_enumeration =
@@ -293,12 +301,15 @@ TEST_F(DeviceEnumerationResourceHelperTest, MonitorDeviceChange) {
helper.SetExpectedResult(data);
- // Synthesize a response with no device.
- ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived(
- PpapiPluginMsg_ResourceReply(
- reply_params,
- PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange(
- callback_id, data))));
+ {
+ ProxyAutoUnlock unlock;
+ // 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;
@@ -313,12 +324,15 @@ TEST_F(DeviceEnumerationResourceHelperTest, MonitorDeviceChange) {
helper.SetExpectedResult(data);
- // Synthesize a response with some devices.
- ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived(
- PpapiPluginMsg_ResourceReply(
- reply_params,
- PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange(
- callback_id, data))));
+ {
+ ProxyAutoUnlock unlock;
+ // 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());
@@ -340,24 +354,30 @@ TEST_F(DeviceEnumerationResourceHelperTest, MonitorDeviceChange) {
helper.SetExpectedResult(data);
helper2.SetExpectedResult(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))));
+ {
+ 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))));
+ }
EXPECT_TRUE(helper2.called() && helper2.same_as_expected());
EXPECT_FALSE(helper.called());
helper.SetExpectedResult(data);
helper2.SetExpectedResult(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))));
+ {
+ 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))));
+ }
EXPECT_FALSE(helper2.called());
EXPECT_FALSE(helper.called());
@@ -373,12 +393,15 @@ TEST_F(DeviceEnumerationResourceHelperTest, MonitorDeviceChange) {
sink().ClearMessages();
helper2.SetExpectedResult(data);
- // |helper2| shouldn't receive any result any more.
- ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived(
- PpapiPluginMsg_ResourceReply(
- reply_params,
- PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange(
- callback_id2, 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))));
+ }
EXPECT_FALSE(helper2.called());
}
diff --git a/ppapi/proxy/file_chooser_resource_unittest.cc b/ppapi/proxy/file_chooser_resource_unittest.cc
index 49c95da..3ff1022 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();
- ScopedPPResource res(ScopedPPResource::PassRef(),
+ LockingResourceReleaser res(
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, output, PP_MakeCompletionCallback(&DoNothingCallback, NULL));
+ res.get(), 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());
- ScopedPPResource dest_deletor(dest[0]); // Ensure it's cleaned up.
+ LockingResourceReleaser 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 ab22077..1a5e7c6 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.
- ScopedPPResource video_capture(ScopedPPResource::PassRef(),
+ LockingResourceReleaser video_capture(
::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
new file mode 100644
index 0000000..d390ac4
--- /dev/null
+++ b/ppapi/proxy/locking_resource_releaser.h
@@ -0,0 +1,41 @@
+// 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 2e18ac5f..3083832 100644
--- a/ppapi/proxy/plugin_globals.cc
+++ b/ppapi/proxy/plugin_globals.cc
@@ -49,40 +49,36 @@ PluginGlobals* PluginGlobals::plugin_globals_ = NULL;
PluginGlobals::PluginGlobals()
: ppapi::PpapiGlobals(),
plugin_proxy_delegate_(NULL),
- 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
-
+ callback_tracker_(new CallbackTracker) {
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_);
- // 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;
-
+ {
+ 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;
+ }
plugin_globals_ = NULL;
}
@@ -131,9 +127,7 @@ void PluginGlobals::PreCacheFontForFlash(const void* logfontw) {
}
base::Lock* PluginGlobals::GetProxyLock() {
- if (enable_threading_)
- return &proxy_lock_;
- return NULL;
+ return &proxy_lock_;
}
void PluginGlobals::LogWithSource(PP_Instance instance,
diff --git a/ppapi/proxy/plugin_globals.h b/ppapi/proxy/plugin_globals.h
index 4da6d5f..37fdc1a 100644
--- a/ppapi/proxy/plugin_globals.h
+++ b/ppapi/proxy/plugin_globals.h
@@ -114,10 +114,6 @@ 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;
@@ -131,7 +127,6 @@ 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 fb81857..12e9d3f 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() {
+PluginResourceTracker::PluginResourceTracker() : ResourceTracker(THREAD_SAFE) {
}
PluginResourceTracker::~PluginResourceTracker() {
diff --git a/ppapi/proxy/plugin_resource_tracker_unittest.cc b/ppapi/proxy/plugin_resource_tracker_unittest.cc
index 59b64db..9a60864 100644
--- a/ppapi/proxy/plugin_resource_tracker_unittest.cc
+++ b/ppapi/proxy/plugin_resource_tracker_unittest.cc
@@ -9,6 +9,7 @@
#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 {
@@ -40,6 +41,8 @@ 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 cff8fa1..14a8584 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() {
+PluginVarTracker::PluginVarTracker() : VarTracker(THREAD_SAFE) {
}
PluginVarTracker::~PluginVarTracker() {
@@ -39,7 +39,7 @@ PluginVarTracker::~PluginVarTracker() {
PP_Var PluginVarTracker::ReceiveObjectPassRef(const PP_Var& host_var,
PluginDispatcher* dispatcher) {
- DCHECK(CalledOnValidThread());
+ CheckThreadingPreconditions();
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) {
- DCHECK(CalledOnValidThread());
+ CheckThreadingPreconditions();
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) {
- DCHECK(CalledOnValidThread());
+ CheckThreadingPreconditions();
DCHECK(plugin_var.type == PP_VARTYPE_OBJECT);
VarMap::iterator found = GetLiveVar(plugin_var);
@@ -98,8 +98,7 @@ void PluginVarTracker::StopTrackingObjectWithNoReference(
}
PP_Var PluginVarTracker::GetHostObject(const PP_Var& plugin_object) const {
- DCHECK(CalledOnValidThread());
-
+ CheckThreadingPreconditions();
if (plugin_object.type != PP_VARTYPE_OBJECT) {
NOTREACHED();
return PP_MakeUndefined();
@@ -120,8 +119,7 @@ PP_Var PluginVarTracker::GetHostObject(const PP_Var& plugin_object) const {
PluginDispatcher* PluginVarTracker::DispatcherForPluginObject(
const PP_Var& plugin_object) const {
- DCHECK(CalledOnValidThread());
-
+ CheckThreadingPreconditions();
if (plugin_object.type != PP_VARTYPE_OBJECT)
return NULL;
@@ -137,7 +135,7 @@ PluginDispatcher* PluginVarTracker::DispatcherForPluginObject(
void PluginVarTracker::ReleaseHostObject(PluginDispatcher* dispatcher,
const PP_Var& host_object) {
- DCHECK(CalledOnValidThread());
+ CheckThreadingPreconditions();
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 00947e3..9034d8f 100644
--- a/ppapi/proxy/ppapi_proxy_test.cc
+++ b/ppapi/proxy/ppapi_proxy_test.cc
@@ -171,6 +171,8 @@ 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());
@@ -196,6 +198,8 @@ 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);
@@ -214,10 +218,15 @@ void PluginProxyTestHarness::SetUpHarnessWithChannel(
}
void PluginProxyTestHarness::TearDownHarness() {
- plugin_dispatcher_->DidDestroyInstance(pp_instance());
- plugin_dispatcher_.reset();
+ {
+ // Some of the methods called during tear-down check that the lock is held.
+ ProxyAutoLock lock;
+
+ 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 bf6147d..0ed0f47 100644
--- a/ppapi/proxy/ppb_var_unittest.cc
+++ b/ppapi/proxy/ppb_var_unittest.cc
@@ -164,11 +164,7 @@ 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 df1af0c..317f4d4 100644
--- a/ppapi/proxy/ppp_instance_private_proxy_unittest.cc
+++ b/ppapi/proxy/ppp_instance_private_proxy_unittest.cc
@@ -147,15 +147,7 @@ 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 91775b0..e2df26e 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,11 +163,10 @@ TEST_F(PPP_Instance_ProxyTest, PPPInstance1_0) {
data.clip_rect = expected_clip;
data.device_scale = 1.0f;
ResetReceived();
- ScopedPPResource view_resource(
- ScopedPPResource::PassRef(),
+ LockingResourceReleaser view_resource(
(new PPB_View_Shared(OBJECT_IS_IMPL,
expected_instance, data))->GetReference());
- ppp_instance->DidChangeView(expected_instance, view_resource);
+ ppp_instance->DidChangeView(expected_instance, view_resource.get());
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 980147f..ed96364 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,13 +47,12 @@ TEST_F(PrintingResourceTest, GetDefaultPrintSettings) {
const PPB_Printing_Dev_0_7* printing_iface =
thunk::GetPPB_Printing_Dev_0_7_Thunk();
- ScopedPPResource res(ScopedPPResource::PassRef(),
- printing_iface->Create(pp_instance()));
+ LockingResourceReleaser res(printing_iface->Create(pp_instance()));
PP_PrintSettings_Dev output_settings;
int32_t result = printing_iface->GetDefaultPrintSettings(
- res, &output_settings, PP_MakeCompletionCallback(&Callback, NULL));
+ res.get(), &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 56012b3..89ea831 100644
--- a/ppapi/proxy/websocket_resource_unittest.cc
+++ b/ppapi/proxy/websocket_resource_unittest.cc
@@ -7,10 +7,14 @@
#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"
@@ -59,11 +63,10 @@ TEST_F(WebSocketResourceTest, Connect) {
PP_Var url_var = MakeStringVar(url);
PP_Var protocols[] = { MakeStringVar(protocol0), MakeStringVar(protocol1) };
- ScopedPPResource res(ScopedPPResource::PassRef(),
- websocket_iface->Create(pp_instance()));
+ LockingResourceReleaser res(websocket_iface->Create(pp_instance()));
- int32_t result =
- websocket_iface->Connect(res, url_var, protocols, 2, MakeCallback());
+ int32_t result = websocket_iface->Connect(res.get(), url_var, protocols, 2,
+ MakeCallback());
ASSERT_EQ(PP_OK_COMPLETIONPENDING, result);
// Should be sent a "Connect" message.
@@ -94,18 +97,17 @@ TEST_F(WebSocketResourceTest, UnsolicitedReplies) {
const PPB_WebSocket_1_0* websocket_iface =
thunk::GetPPB_WebSocket_1_0_Thunk();
- ScopedPPResource res(ScopedPPResource::PassRef(),
- websocket_iface->Create(pp_instance()));
+ LockingResourceReleaser res(websocket_iface->Create(pp_instance()));
// Check if BufferedAmountReply is handled.
- ResourceMessageReplyParams reply_params(res, 0);
+ ResourceMessageReplyParams reply_params(res.get(), 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);
+ uint64_t amount = websocket_iface->GetBufferedAmount(res.get());
EXPECT_EQ(19760227u, amount);
// Check if StateReply is handled.
@@ -115,7 +117,7 @@ TEST_F(WebSocketResourceTest, UnsolicitedReplies) {
PpapiPluginMsg_WebSocket_StateReply(
static_cast<int32_t>(PP_WEBSOCKETREADYSTATE_CLOSING)))));
- PP_WebSocketReadyState state = websocket_iface->GetReadyState(res);
+ PP_WebSocketReadyState state = websocket_iface->GetReadyState(res.get());
EXPECT_EQ(PP_WEBSOCKETREADYSTATE_CLOSING, state);
}
@@ -126,12 +128,11 @@ TEST_F(WebSocketResourceTest, MessageError) {
std::string url("ws://ws.google.com");
PP_Var url_var = MakeStringVar(url);
- ScopedPPResource res(ScopedPPResource::PassRef(),
- websocket_iface->Create(pp_instance()));
+ LockingResourceReleaser res(websocket_iface->Create(pp_instance()));
// Establish the connection virtually.
int32_t result =
- websocket_iface->Connect(res, url_var, NULL, 0, MakeCallback());
+ websocket_iface->Connect(res.get(), url_var, NULL, 0, MakeCallback());
ASSERT_EQ(PP_OK_COMPLETIONPENDING, result);
ResourceMessageCallParams params;
@@ -150,11 +151,11 @@ TEST_F(WebSocketResourceTest, MessageError) {
EXPECT_TRUE(g_callback_called);
PP_Var message;
- result = websocket_iface->ReceiveMessage(res, &message, MakeCallback());
+ result = websocket_iface->ReceiveMessage(res.get(), &message, MakeCallback());
EXPECT_FALSE(g_callback_called);
// Synthesize a WebSocket_ErrorReply message.
- ResourceMessageReplyParams error_reply_params(res, 0);
+ ResourceMessageReplyParams error_reply_params(res.get(), 0);
error_reply_params.set_result(PP_OK);
ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived(
PpapiPluginMsg_ResourceReply(error_reply_params,
diff --git a/ppapi/shared_impl/resource_tracker.cc b/ppapi/shared_impl/resource_tracker.cc
index a448ce4..a46c823 100644
--- a/ppapi/shared_impl/resource_tracker.cc
+++ b/ppapi/shared_impl/resource_tracker.cc
@@ -15,17 +15,23 @@
namespace ppapi {
-ResourceTracker::ResourceTracker()
+ResourceTracker::ResourceTracker(ThreadMode thread_mode)
: last_resource_value_(0),
ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
+ if (thread_mode == SINGLE_THREADED)
+ thread_checker_.reset(new base::ThreadChecker);
}
ResourceTracker::~ResourceTracker() {
}
-Resource* ResourceTracker::GetResource(PP_Resource res) const {
- CHECK(thread_checker_.CalledOnValidThread());
+void ResourceTracker::CheckThreadingPreconditions() const {
+ DCHECK(!thread_checker_ || thread_checker_->CalledOnValidThread());
ProxyLock::AssertAcquired();
+}
+
+Resource* ResourceTracker::GetResource(PP_Resource res) const {
+ CheckThreadingPreconditions();
ResourceMap::const_iterator i = live_resources_.find(res);
if (i == live_resources_.end())
return NULL;
@@ -33,7 +39,7 @@ Resource* ResourceTracker::GetResource(PP_Resource res) const {
}
void ResourceTracker::AddRefResource(PP_Resource res) {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
DLOG_IF(ERROR, !CheckIdType(res, PP_ID_TYPE_RESOURCE))
<< res << " is not a PP_Resource.";
ResourceMap::iterator i = live_resources_.find(res);
@@ -55,7 +61,7 @@ void ResourceTracker::AddRefResource(PP_Resource res) {
}
void ResourceTracker::ReleaseResource(PP_Resource res) {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
DLOG_IF(ERROR, !CheckIdType(res, PP_ID_TYPE_RESOURCE))
<< res << " is not a PP_Resource.";
ResourceMap::iterator i = live_resources_.find(res);
@@ -86,7 +92,7 @@ void ResourceTracker::ReleaseResourceSoon(PP_Resource res) {
}
void ResourceTracker::DidCreateInstance(PP_Instance instance) {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
// Due to the infrastructure of some tests, the instance is registered
// twice in a few cases. It would be nice not to do that and assert here
// instead.
@@ -96,7 +102,7 @@ void ResourceTracker::DidCreateInstance(PP_Instance instance) {
}
void ResourceTracker::DidDeleteInstance(PP_Instance instance) {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
InstanceMap::iterator found_instance = instance_map_.find(instance);
// Due to the infrastructure of some tests, the instance is unregistered
@@ -151,7 +157,7 @@ void ResourceTracker::DidDeleteInstance(PP_Instance instance) {
}
int ResourceTracker::GetLiveObjectsForInstance(PP_Instance instance) const {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
InstanceMap::const_iterator found = instance_map_.find(instance);
if (found == instance_map_.end())
return 0;
@@ -159,7 +165,7 @@ int ResourceTracker::GetLiveObjectsForInstance(PP_Instance instance) const {
}
PP_Resource ResourceTracker::AddResource(Resource* object) {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
// If the plugin manages to create too many resources, don't do crazy stuff.
if (last_resource_value_ == kMaxPPId)
return 0;
@@ -191,7 +197,7 @@ PP_Resource ResourceTracker::AddResource(Resource* object) {
}
void ResourceTracker::RemoveResource(Resource* object) {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
PP_Resource pp_resource = object->pp_resource();
InstanceMap::iterator found = instance_map_.find(object->pp_instance());
if (found != instance_map_.end())
diff --git a/ppapi/shared_impl/resource_tracker.h b/ppapi/shared_impl/resource_tracker.h
index f5f790c..54e6daf 100644
--- a/ppapi/shared_impl/resource_tracker.h
+++ b/ppapi/shared_impl/resource_tracker.h
@@ -10,6 +10,7 @@
#include "base/basictypes.h"
#include "base/hash_tables.h"
#include "base/memory/linked_ptr.h"
+#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/threading/thread_checker.h"
#include "base/threading/thread_checker_impl.h"
@@ -23,7 +24,12 @@ class Resource;
class PPAPI_SHARED_EXPORT ResourceTracker {
public:
- ResourceTracker();
+ // A SINGLE_THREADED ResourceTracker will use a thread-checker to make sure
+ // it's always invoked on the same thread on which it was constructed. A
+ // THREAD_SAFE ResourceTracker will check that the ProxyLock is held. See
+ // CheckThreadingPreconditions() for more details.
+ enum ThreadMode { SINGLE_THREADED, THREAD_SAFE };
+ explicit ResourceTracker(ThreadMode thread_mode);
virtual ~ResourceTracker();
// The returned pointer will be NULL if there is no resource. The reference
@@ -53,6 +59,10 @@ class PPAPI_SHARED_EXPORT ResourceTracker {
// This calls AddResource and RemoveResource.
friend class Resource;
+ // On the host-side, make sure we are called on the right thread. On the
+ // plugin side, make sure we have the proxy lock.
+ void CheckThreadingPreconditions() const;
+
// Adds the given resource to the tracker, associating it with the instance
// stored in the resource object. The new resource ID is returned, and the
// resource will have 0 plugin refcount. This is called by the resource
@@ -98,19 +108,11 @@ class PPAPI_SHARED_EXPORT ResourceTracker {
base::WeakPtrFactory<ResourceTracker> weak_ptr_factory_;
- // TODO(raymes): We won't need to do thread checks once pepper calls are
- // allowed off of the main thread.
- // See http://code.google.com/p/chromium/issues/detail?id=92909.
-#ifdef ENABLE_PEPPER_THREADING
- base::ThreadCheckerDoNothing thread_checker_;
-#else
- // TODO(raymes): We've seen plugins (Flash) creating resources from random
- // threads. Let's always crash for now in this case. Once we have a handle
- // over how common this is, we can change ThreadCheckerImpl->ThreadChecker
- // so that we only crash in debug mode. See
- // https://code.google.com/p/chromium/issues/detail?id=146415.
- base::ThreadCheckerImpl thread_checker_;
-#endif
+ // On the host side, we want to check that we are only called on the main
+ // thread. This is to protect us from accidentally using the tracker from
+ // other threads (especially the IO thread). On the plugin side, the tracker
+ // is protected by the proxy lock and is thread-safe, so this will be NULL.
+ scoped_ptr<base::ThreadChecker> thread_checker_;
DISALLOW_COPY_AND_ASSIGN(ResourceTracker);
};
diff --git a/ppapi/shared_impl/test_globals.cc b/ppapi/shared_impl/test_globals.cc
index 913d53c..6c6af5b 100644
--- a/ppapi/shared_impl/test_globals.cc
+++ b/ppapi/shared_impl/test_globals.cc
@@ -8,11 +8,13 @@ namespace ppapi {
TestGlobals::TestGlobals()
: ppapi::PpapiGlobals(),
+ resource_tracker_(ResourceTracker::THREAD_SAFE),
callback_tracker_(new CallbackTracker) {
}
TestGlobals::TestGlobals(PpapiGlobals::PerThreadForTest per_thread_for_test)
: ppapi::PpapiGlobals(per_thread_for_test),
+ resource_tracker_(ResourceTracker::THREAD_SAFE),
callback_tracker_(new CallbackTracker) {
}
diff --git a/ppapi/shared_impl/test_globals.h b/ppapi/shared_impl/test_globals.h
index 5579b09..c4d4aa1 100644
--- a/ppapi/shared_impl/test_globals.h
+++ b/ppapi/shared_impl/test_globals.h
@@ -16,7 +16,7 @@ namespace ppapi {
class TestVarTracker : public VarTracker {
public:
- TestVarTracker() {}
+ TestVarTracker() : VarTracker(THREAD_SAFE) {}
virtual ~TestVarTracker() {}
virtual ArrayBufferVar* CreateArrayBuffer(uint32 size_in_bytes) OVERRIDE {
return NULL;
diff --git a/ppapi/shared_impl/var_tracker.cc b/ppapi/shared_impl/var_tracker.cc
index 434121e..26160f4 100644
--- a/ppapi/shared_impl/var_tracker.cc
+++ b/ppapi/shared_impl/var_tracker.cc
@@ -29,22 +29,27 @@ VarTracker::VarInfo::VarInfo(Var* v, int input_ref_count)
track_with_no_reference_count(0) {
}
-VarTracker::VarTracker() : last_var_id_(0) {
+VarTracker::VarTracker(ThreadMode thread_mode) : last_var_id_(0) {
+ if (thread_mode == SINGLE_THREADED)
+ thread_checker_.reset(new base::ThreadChecker);
}
VarTracker::~VarTracker() {
}
-int32 VarTracker::AddVar(Var* var) {
- DCHECK(CalledOnValidThread());
+void VarTracker::CheckThreadingPreconditions() const {
+ DCHECK(!thread_checker_ || thread_checker_->CalledOnValidThread());
ProxyLock::AssertAcquired();
+}
+
+int32 VarTracker::AddVar(Var* var) {
+ CheckThreadingPreconditions();
return AddVarInternal(var, ADD_VAR_TAKE_ONE_REFERENCE);
}
Var* VarTracker::GetVar(int32 var_id) const {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
VarMap::const_iterator result = live_vars_.find(var_id);
if (result == live_vars_.end())
@@ -53,8 +58,7 @@ Var* VarTracker::GetVar(int32 var_id) const {
}
Var* VarTracker::GetVar(const PP_Var& var) const {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
if (!IsVarTypeRefcounted(var.type))
return NULL;
@@ -62,8 +66,7 @@ Var* VarTracker::GetVar(const PP_Var& var) const {
}
bool VarTracker::AddRefVar(int32 var_id) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR))
<< var_id << " is not a PP_Var ID.";
@@ -88,8 +91,7 @@ bool VarTracker::AddRefVar(int32 var_id) {
}
bool VarTracker::AddRefVar(const PP_Var& var) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
if (!IsVarTypeRefcounted(var.type))
return true;
@@ -97,8 +99,7 @@ bool VarTracker::AddRefVar(const PP_Var& var) {
}
bool VarTracker::ReleaseVar(int32 var_id) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR))
<< var_id << " is not a PP_Var ID.";
@@ -129,8 +130,7 @@ bool VarTracker::ReleaseVar(int32 var_id) {
}
bool VarTracker::ReleaseVar(const PP_Var& var) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
if (!IsVarTypeRefcounted(var.type))
return false;
@@ -154,8 +154,7 @@ VarTracker::VarMap::iterator VarTracker::GetLiveVar(int32 id) {
}
int VarTracker::GetRefCountForObject(const PP_Var& plugin_object) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
VarMap::iterator found = GetLiveVar(plugin_object);
if (found == live_vars_.end())
@@ -165,8 +164,7 @@ int VarTracker::GetRefCountForObject(const PP_Var& plugin_object) {
int VarTracker::GetTrackedWithNoReferenceCountForObject(
const PP_Var& plugin_object) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
VarMap::iterator found = GetLiveVar(plugin_object);
if (found == live_vars_.end())
@@ -188,8 +186,7 @@ bool VarTracker::IsVarTypeRefcounted(PP_VarType type) const {
}
PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
scoped_refptr<ArrayBufferVar> array_buffer(CreateArrayBuffer(size_in_bytes));
if (!array_buffer)
@@ -199,8 +196,7 @@ PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes) {
PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes,
const void* data) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
ArrayBufferVar* array_buffer = MakeArrayBufferVar(size_in_bytes, data);
return array_buffer ? array_buffer->GetPPVar() : PP_MakeNull();
@@ -208,8 +204,7 @@ PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes,
ArrayBufferVar* VarTracker::MakeArrayBufferVar(uint32 size_in_bytes,
const void* data) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
ArrayBufferVar* array_buffer(CreateArrayBuffer(size_in_bytes));
if (!array_buffer)
@@ -220,7 +215,7 @@ ArrayBufferVar* VarTracker::MakeArrayBufferVar(uint32 size_in_bytes,
PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes,
base::SharedMemoryHandle handle) {
- DCHECK(CalledOnValidThread());
+ CheckThreadingPreconditions();
scoped_refptr<ArrayBufferVar> array_buffer(
CreateShmArrayBuffer(size_in_bytes, handle));
@@ -230,8 +225,7 @@ PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes,
}
std::vector<PP_Var> VarTracker::GetLiveVars() {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
std::vector<PP_Var> var_vector;
var_vector.reserve(live_vars_.size());
diff --git a/ppapi/shared_impl/var_tracker.h b/ppapi/shared_impl/var_tracker.h
index 32acdeb..a9d92ea 100644
--- a/ppapi/shared_impl/var_tracker.h
+++ b/ppapi/shared_impl/var_tracker.h
@@ -10,8 +10,9 @@
#include "base/basictypes.h"
#include "base/hash_tables.h"
#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
#include "base/shared_memory.h"
-#include "base/threading/non_thread_safe.h"
+#include "base/threading/thread_checker.h"
#include "ppapi/c/pp_instance.h"
#include "ppapi/c/pp_module.h"
#include "ppapi/c/pp_resource.h"
@@ -36,16 +37,14 @@ class Var;
// This class maintains the "track_with_no_reference_count" but doesn't do
// anything with it other than call virtual functions. The interesting parts
// are added by the PluginObjectVar derived from this class.
-class PPAPI_SHARED_EXPORT VarTracker
-#ifdef ENABLE_PEPPER_THREADING
- : NON_EXPORTED_BASE(public base::NonThreadSafeDoNothing) {
-#else
- // TODO(dmichael): Remove the thread checking when calls are allowed off the
- // main thread (crbug.com/92909).
- : NON_EXPORTED_BASE(public base::NonThreadSafe) {
-#endif
+class PPAPI_SHARED_EXPORT VarTracker {
public:
- VarTracker();
+ // A SINGLE_THREADED VarTracker will use a thread-checker to make sure it's
+ // always invoked on the same thread on which it was constructed. A
+ // THREAD_SAFE VarTracker will check that the ProxyLock is held. See
+ // CheckThreadingPreconditions() for more details.
+ enum ThreadMode { SINGLE_THREADED, THREAD_SAFE };
+ explicit VarTracker(ThreadMode thread_mode);
virtual ~VarTracker();
// Called by the Var object to add a new var to the tracker.
@@ -148,6 +147,10 @@ class PPAPI_SHARED_EXPORT VarTracker
ADD_VAR_CREATE_WITH_NO_REFERENCE
};
+ // On the host-side, make sure we are called on the right thread. On the
+ // plugin side, make sure we have the proxy lock.
+ void CheckThreadingPreconditions() const;
+
// Implementation of AddVar that allows the caller to specify whether the
// initial refcount of the added object will be 0 or 1.
//
@@ -198,6 +201,12 @@ class PPAPI_SHARED_EXPORT VarTracker
uint32 size_in_bytes,
base::SharedMemoryHandle handle) = 0;
+ // On the host side, we want to check that we are only called on the main
+ // thread. This is to protect us from accidentally using the tracker from
+ // other threads (especially the IO thread). On the plugin side, the tracker
+ // is protected by the proxy lock and is thread-safe, so this will be NULL.
+ scoped_ptr<base::ThreadChecker> thread_checker_;
+
DISALLOW_COPY_AND_ASSIGN(VarTracker);
};
diff --git a/ppapi/tests/test_case.h b/ppapi/tests/test_case.h
index c2379d1..9ffc8f6 100644
--- a/ppapi/tests/test_case.h
+++ b/ppapi/tests/test_case.h
@@ -137,7 +137,6 @@ class TestCase {
// Run the given test method on a background thread and return the result.
template <class T>
std::string RunOnThread(std::string(T::*test_to_run)()) {
-#ifdef ENABLE_PEPPER_THREADING
if (!testing_interface_) {
return "Testing blocking callbacks requires the testing interface. In "
"Chrome, use the --enable-pepper-testing flag.";
@@ -152,10 +151,6 @@ class TestCase {
RunOnThreadInternal(&ThreadedTestRunner<T>::ThreadFunction, &runner,
testing_interface_);
return runner.result();
-#else
- // If threading's not enabled, just treat it as success.
- return std::string();
-#endif
}
// Pointer to the instance that owns us.
diff --git a/webkit/plugins/plugin_switches.cc b/webkit/plugins/plugin_switches.cc
index dda1acc..bb1b3b4 100644
--- a/webkit/plugins/plugin_switches.cc
+++ b/webkit/plugins/plugin_switches.cc
@@ -15,9 +15,6 @@ const char kDisablePepper3d[] = "disable-pepper-3d";
// "Command-line" arguments for the PPAPI Flash; used for debugging options.
const char kPpapiFlashArgs[] = "ppapi-flash-args";
-// Set to true to not allow threadsafety support in native Pepper plugins.
-const char kDisablePepperThreading[] = "disable-pepper-threading";
-
#if defined(OS_WIN)
// Used by the plugins_test when testing the older WMP plugin to force the new
// plugin to not get loaded.
diff --git a/webkit/plugins/plugin_switches.h b/webkit/plugins/plugin_switches.h
index a195094..08ed5ae 100644
--- a/webkit/plugins/plugin_switches.h
+++ b/webkit/plugins/plugin_switches.h
@@ -13,7 +13,6 @@ namespace switches {
WEBKIT_PLUGINS_EXPORT extern const char kDebugPluginLoading[];
WEBKIT_PLUGINS_EXPORT extern const char kDisablePepper3d[];
WEBKIT_PLUGINS_EXPORT extern const char kPpapiFlashArgs[];
-WEBKIT_PLUGINS_EXPORT extern const char kDisablePepperThreading[];
#if defined(OS_WIN)
WEBKIT_PLUGINS_EXPORT extern const char kUseOldWMPPlugin[];
diff --git a/webkit/plugins/ppapi/host_globals.cc b/webkit/plugins/ppapi/host_globals.cc
index 6370269..15dedf2 100644
--- a/webkit/plugins/ppapi/host_globals.cc
+++ b/webkit/plugins/ppapi/host_globals.cc
@@ -25,6 +25,7 @@
using ppapi::CheckIdType;
using ppapi::MakeTypedId;
using ppapi::PPIdType;
+using ppapi::ResourceTracker;
using WebKit::WebConsoleMessage;
using WebKit::WebString;
@@ -74,14 +75,17 @@ WebConsoleMessage MakeLogMessage(PP_LogLevel level,
HostGlobals* HostGlobals::host_globals_ = NULL;
-HostGlobals::HostGlobals() : ::ppapi::PpapiGlobals() {
+HostGlobals::HostGlobals()
+ : ::ppapi::PpapiGlobals(),
+ resource_tracker_(ResourceTracker::SINGLE_THREADED) {
DCHECK(!host_globals_);
host_globals_ = this;
}
HostGlobals::HostGlobals(
::ppapi::PpapiGlobals::PerThreadForTest per_thread_for_test)
- : ::ppapi::PpapiGlobals(per_thread_for_test) {
+ : ::ppapi::PpapiGlobals(per_thread_for_test),
+ resource_tracker_(ResourceTracker::SINGLE_THREADED) {
DCHECK(!host_globals_);
}
diff --git a/webkit/plugins/ppapi/host_var_tracker.cc b/webkit/plugins/ppapi/host_var_tracker.cc
index 5d53499..99518fd 100644
--- a/webkit/plugins/ppapi/host_var_tracker.cc
+++ b/webkit/plugins/ppapi/host_var_tracker.cc
@@ -16,7 +16,9 @@ using ppapi::NPObjectVar;
namespace webkit {
namespace ppapi {
-HostVarTracker::HostVarTracker() : last_shared_memory_map_id_(0) {
+HostVarTracker::HostVarTracker()
+ : VarTracker(SINGLE_THREADED),
+ last_shared_memory_map_id_(0) {
}
HostVarTracker::~HostVarTracker() {
@@ -33,7 +35,7 @@ ArrayBufferVar* HostVarTracker::CreateShmArrayBuffer(
}
void HostVarTracker::AddNPObjectVar(NPObjectVar* object_var) {
- DCHECK(CalledOnValidThread());
+ CheckThreadingPreconditions();
InstanceMap::iterator found_instance = instance_map_.find(
object_var->pp_instance());
@@ -53,7 +55,7 @@ void HostVarTracker::AddNPObjectVar(NPObjectVar* object_var) {
}
void HostVarTracker::RemoveNPObjectVar(NPObjectVar* object_var) {
- DCHECK(CalledOnValidThread());
+ CheckThreadingPreconditions();
InstanceMap::iterator found_instance = instance_map_.find(
object_var->pp_instance());
@@ -78,7 +80,7 @@ void HostVarTracker::RemoveNPObjectVar(NPObjectVar* object_var) {
NPObjectVar* HostVarTracker::NPObjectVarForNPObject(PP_Instance instance,
NPObject* np_object) {
- DCHECK(CalledOnValidThread());
+ CheckThreadingPreconditions();
InstanceMap::iterator found_instance = instance_map_.find(instance);
if (found_instance == instance_map_.end())
@@ -93,7 +95,7 @@ NPObjectVar* HostVarTracker::NPObjectVarForNPObject(PP_Instance instance,
}
int HostVarTracker::GetLiveNPObjectVarsForInstance(PP_Instance instance) const {
- DCHECK(CalledOnValidThread());
+ CheckThreadingPreconditions();
InstanceMap::const_iterator found = instance_map_.find(instance);
if (found == instance_map_.end())
@@ -102,7 +104,7 @@ int HostVarTracker::GetLiveNPObjectVarsForInstance(PP_Instance instance) const {
}
void HostVarTracker::DidDeleteInstance(PP_Instance instance) {
- DCHECK(CalledOnValidThread());
+ CheckThreadingPreconditions();
InstanceMap::iterator found_instance = instance_map_.find(instance);
if (found_instance == instance_map_.end())