summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-03-11 19:58:09 +0000
committerdmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-03-11 19:58:09 +0000
commita65fe6ed7d949e55e9fca008ec7805e4f1f19a23 (patch)
treec3f575a3c9911f48a0c145806272ab225ae6a0d3
parent5102497f1d1dc0cc29967f66ac103185e279b5c6 (diff)
downloadchromium_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
-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.cc48
-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.cc13
33 files changed, 226 insertions, 259 deletions
diff --git a/build/common.gypi b/build/common.gypi
index c71e889..f22c58c 100644
--- a/build/common.gypi
+++ b/build/common.gypi
@@ -320,6 +320,11 @@
# 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,
@@ -667,6 +672,7 @@
'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)',
@@ -1754,6 +1760,9 @@
['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 64975c5..a4dd416 100644
--- a/content/browser/ppapi_plugin_process_host.cc
+++ b/content/browser/ppapi_plugin_process_host.cc
@@ -248,9 +248,10 @@ bool PpapiPluginProcessHost::Init(const PepperPluginInfo& info) {
arraysize(kCommonForwardSwitches));
if (!is_broker_) {
- // TODO(vtl): Stop passing flash args in the command line, or windows is
+ // TODO(vtl): Stop passing flash args in the command line, on 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 d6971410..6aa7b80 100644
--- a/content/ppapi_plugin/ppapi_thread.cc
+++ b/content/ppapi_plugin/ppapi_thread.cc
@@ -91,6 +91,8 @@ 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 45ffd0f..fd55a58 100644
--- a/ppapi/ppapi_ipc_untrusted.gyp
+++ b/ppapi/ppapi_ipc_untrusted.gyp
@@ -23,6 +23,11 @@
'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 c3159bf..92e2238 100644
--- a/ppapi/ppapi_proxy.gypi
+++ b/ppapi/ppapi_proxy.gypi
@@ -69,7 +69,6 @@
'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 2b87d7b..49fae82 100644
--- a/ppapi/ppapi_proxy_untrusted.gyp
+++ b/ppapi/ppapi_proxy_untrusted.gyp
@@ -22,6 +22,11 @@
'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 2c0f8bd..e085f60 100644
--- a/ppapi/ppapi_shared_untrusted.gyp
+++ b/ppapi/ppapi_shared_untrusted.gyp
@@ -23,6 +23,11 @@
'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 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,
diff --git a/ppapi/shared_impl/resource_tracker.cc b/ppapi/shared_impl/resource_tracker.cc
index a46c823..a448ce4 100644
--- a/ppapi/shared_impl/resource_tracker.cc
+++ b/ppapi/shared_impl/resource_tracker.cc
@@ -15,23 +15,17 @@
namespace ppapi {
-ResourceTracker::ResourceTracker(ThreadMode thread_mode)
+ResourceTracker::ResourceTracker()
: 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() {
}
-void ResourceTracker::CheckThreadingPreconditions() const {
- DCHECK(!thread_checker_ || thread_checker_->CalledOnValidThread());
- ProxyLock::AssertAcquired();
-}
-
Resource* ResourceTracker::GetResource(PP_Resource res) const {
- CheckThreadingPreconditions();
+ CHECK(thread_checker_.CalledOnValidThread());
+ ProxyLock::AssertAcquired();
ResourceMap::const_iterator i = live_resources_.find(res);
if (i == live_resources_.end())
return NULL;
@@ -39,7 +33,7 @@ Resource* ResourceTracker::GetResource(PP_Resource res) const {
}
void ResourceTracker::AddRefResource(PP_Resource res) {
- CheckThreadingPreconditions();
+ CHECK(thread_checker_.CalledOnValidThread());
DLOG_IF(ERROR, !CheckIdType(res, PP_ID_TYPE_RESOURCE))
<< res << " is not a PP_Resource.";
ResourceMap::iterator i = live_resources_.find(res);
@@ -61,7 +55,7 @@ void ResourceTracker::AddRefResource(PP_Resource res) {
}
void ResourceTracker::ReleaseResource(PP_Resource res) {
- CheckThreadingPreconditions();
+ CHECK(thread_checker_.CalledOnValidThread());
DLOG_IF(ERROR, !CheckIdType(res, PP_ID_TYPE_RESOURCE))
<< res << " is not a PP_Resource.";
ResourceMap::iterator i = live_resources_.find(res);
@@ -92,7 +86,7 @@ void ResourceTracker::ReleaseResourceSoon(PP_Resource res) {
}
void ResourceTracker::DidCreateInstance(PP_Instance instance) {
- CheckThreadingPreconditions();
+ CHECK(thread_checker_.CalledOnValidThread());
// 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.
@@ -102,7 +96,7 @@ void ResourceTracker::DidCreateInstance(PP_Instance instance) {
}
void ResourceTracker::DidDeleteInstance(PP_Instance instance) {
- CheckThreadingPreconditions();
+ CHECK(thread_checker_.CalledOnValidThread());
InstanceMap::iterator found_instance = instance_map_.find(instance);
// Due to the infrastructure of some tests, the instance is unregistered
@@ -157,7 +151,7 @@ void ResourceTracker::DidDeleteInstance(PP_Instance instance) {
}
int ResourceTracker::GetLiveObjectsForInstance(PP_Instance instance) const {
- CheckThreadingPreconditions();
+ CHECK(thread_checker_.CalledOnValidThread());
InstanceMap::const_iterator found = instance_map_.find(instance);
if (found == instance_map_.end())
return 0;
@@ -165,7 +159,7 @@ int ResourceTracker::GetLiveObjectsForInstance(PP_Instance instance) const {
}
PP_Resource ResourceTracker::AddResource(Resource* object) {
- CheckThreadingPreconditions();
+ CHECK(thread_checker_.CalledOnValidThread());
// If the plugin manages to create too many resources, don't do crazy stuff.
if (last_resource_value_ == kMaxPPId)
return 0;
@@ -197,7 +191,7 @@ PP_Resource ResourceTracker::AddResource(Resource* object) {
}
void ResourceTracker::RemoveResource(Resource* object) {
- CheckThreadingPreconditions();
+ CHECK(thread_checker_.CalledOnValidThread());
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 8f0b8a6..f5f790c 100644
--- a/ppapi/shared_impl/resource_tracker.h
+++ b/ppapi/shared_impl/resource_tracker.h
@@ -10,7 +10,6 @@
#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"
@@ -24,12 +23,7 @@ class Resource;
class PPAPI_SHARED_EXPORT ResourceTracker {
public:
- // 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);
+ ResourceTracker();
virtual ~ResourceTracker();
// The returned pointer will be NULL if there is no resource. The reference
@@ -59,10 +53,6 @@ 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
@@ -108,11 +98,19 @@ class PPAPI_SHARED_EXPORT ResourceTracker {
base::WeakPtrFactory<ResourceTracker> weak_ptr_factory_;
- // 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::ThreadCheckerImpl> thread_checker_;
+ // 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
DISALLOW_COPY_AND_ASSIGN(ResourceTracker);
};
diff --git a/ppapi/shared_impl/test_globals.cc b/ppapi/shared_impl/test_globals.cc
index 6c6af5b..913d53c 100644
--- a/ppapi/shared_impl/test_globals.cc
+++ b/ppapi/shared_impl/test_globals.cc
@@ -8,13 +8,11 @@ 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 75e3a90..68a997a 100644
--- a/ppapi/shared_impl/test_globals.h
+++ b/ppapi/shared_impl/test_globals.h
@@ -15,7 +15,7 @@ namespace ppapi {
class TestVarTracker : public VarTracker {
public:
- TestVarTracker() : VarTracker(THREAD_SAFE) {}
+ TestVarTracker() {}
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 5e33a23..6bc0261 100644
--- a/ppapi/shared_impl/var_tracker.cc
+++ b/ppapi/shared_impl/var_tracker.cc
@@ -27,27 +27,22 @@ VarTracker::VarInfo::VarInfo(Var* v, int input_ref_count)
track_with_no_reference_count(0) {
}
-VarTracker::VarTracker(ThreadMode thread_mode) : last_var_id_(0) {
- if (thread_mode == SINGLE_THREADED)
- thread_checker_.reset(new base::ThreadChecker);
+VarTracker::VarTracker() : last_var_id_(0) {
}
VarTracker::~VarTracker() {
}
-void VarTracker::CheckThreadingPreconditions() const {
- DCHECK(!thread_checker_ || thread_checker_->CalledOnValidThread());
- ProxyLock::AssertAcquired();
-}
-
int32 VarTracker::AddVar(Var* var) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
return AddVarInternal(var, ADD_VAR_TAKE_ONE_REFERENCE);
}
Var* VarTracker::GetVar(int32 var_id) const {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
VarMap::const_iterator result = live_vars_.find(var_id);
if (result == live_vars_.end())
@@ -56,7 +51,8 @@ Var* VarTracker::GetVar(int32 var_id) const {
}
Var* VarTracker::GetVar(const PP_Var& var) const {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
if (!IsVarTypeRefcounted(var.type))
return NULL;
@@ -64,7 +60,8 @@ Var* VarTracker::GetVar(const PP_Var& var) const {
}
bool VarTracker::AddRefVar(int32 var_id) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR))
<< var_id << " is not a PP_Var ID.";
@@ -89,7 +86,8 @@ bool VarTracker::AddRefVar(int32 var_id) {
}
bool VarTracker::AddRefVar(const PP_Var& var) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
if (!IsVarTypeRefcounted(var.type))
return false;
@@ -97,7 +95,8 @@ bool VarTracker::AddRefVar(const PP_Var& var) {
}
bool VarTracker::ReleaseVar(int32 var_id) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR))
<< var_id << " is not a PP_Var ID.";
@@ -128,7 +127,8 @@ bool VarTracker::ReleaseVar(int32 var_id) {
}
bool VarTracker::ReleaseVar(const PP_Var& var) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
if (!IsVarTypeRefcounted(var.type))
return false;
@@ -152,7 +152,8 @@ VarTracker::VarMap::iterator VarTracker::GetLiveVar(int32 id) {
}
int VarTracker::GetRefCountForObject(const PP_Var& plugin_object) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
VarMap::iterator found = GetLiveVar(plugin_object);
if (found == live_vars_.end())
@@ -162,7 +163,8 @@ int VarTracker::GetRefCountForObject(const PP_Var& plugin_object) {
int VarTracker::GetTrackedWithNoReferenceCountForObject(
const PP_Var& plugin_object) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
VarMap::iterator found = GetLiveVar(plugin_object);
if (found == live_vars_.end())
@@ -184,7 +186,8 @@ bool VarTracker::IsVarTypeRefcounted(PP_VarType type) const {
}
PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
scoped_refptr<ArrayBufferVar> array_buffer(CreateArrayBuffer(size_in_bytes));
if (!array_buffer)
@@ -194,7 +197,8 @@ PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes) {
PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes,
const void* data) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
ArrayBufferVar* array_buffer = MakeArrayBufferVar(size_in_bytes, data);
return array_buffer ? array_buffer->GetPPVar() : PP_MakeNull();
@@ -202,7 +206,8 @@ PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes,
ArrayBufferVar* VarTracker::MakeArrayBufferVar(uint32 size_in_bytes,
const void* data) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
ArrayBufferVar* array_buffer(CreateArrayBuffer(size_in_bytes));
if (!array_buffer)
@@ -212,7 +217,8 @@ ArrayBufferVar* VarTracker::MakeArrayBufferVar(uint32 size_in_bytes,
}
std::vector<PP_Var> VarTracker::GetLiveVars() {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
+ ProxyLock::AssertAcquired();
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 7d544da..1d0c0d6 100644
--- a/ppapi/shared_impl/var_tracker.h
+++ b/ppapi/shared_impl/var_tracker.h
@@ -10,8 +10,7 @@
#include "base/basictypes.h"
#include "base/hash_tables.h"
#include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/threading/thread_checker.h"
+#include "base/threading/non_thread_safe.h"
#include "ppapi/c/pp_instance.h"
#include "ppapi/c/pp_module.h"
#include "ppapi/c/pp_var.h"
@@ -34,14 +33,16 @@ 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 {
+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
public:
- // 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);
+ VarTracker();
virtual ~VarTracker();
// Called by the Var object to add a new var to the tracker.
@@ -124,10 +125,6 @@ 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.
//
@@ -175,12 +172,6 @@ class PPAPI_SHARED_EXPORT VarTracker {
// a real WebKit ArrayBuffer on the host side.
virtual ArrayBufferVar* CreateArrayBuffer(uint32 size_in_bytes) = 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 9ffc8f6..c2379d1 100644
--- a/ppapi/tests/test_case.h
+++ b/ppapi/tests/test_case.h
@@ -137,6 +137,7 @@ 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.";
@@ -151,6 +152,10 @@ 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 bb1b3b4..dda1acc 100644
--- a/webkit/plugins/plugin_switches.cc
+++ b/webkit/plugins/plugin_switches.cc
@@ -15,6 +15,9 @@ 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 08ed5ae..a195094 100644
--- a/webkit/plugins/plugin_switches.h
+++ b/webkit/plugins/plugin_switches.h
@@ -13,6 +13,7 @@ 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 15dedf2..6370269 100644
--- a/webkit/plugins/ppapi/host_globals.cc
+++ b/webkit/plugins/ppapi/host_globals.cc
@@ -25,7 +25,6 @@
using ppapi::CheckIdType;
using ppapi::MakeTypedId;
using ppapi::PPIdType;
-using ppapi::ResourceTracker;
using WebKit::WebConsoleMessage;
using WebKit::WebString;
@@ -75,17 +74,14 @@ WebConsoleMessage MakeLogMessage(PP_LogLevel level,
HostGlobals* HostGlobals::host_globals_ = NULL;
-HostGlobals::HostGlobals()
- : ::ppapi::PpapiGlobals(),
- resource_tracker_(ResourceTracker::SINGLE_THREADED) {
+HostGlobals::HostGlobals() : ::ppapi::PpapiGlobals() {
DCHECK(!host_globals_);
host_globals_ = this;
}
HostGlobals::HostGlobals(
::ppapi::PpapiGlobals::PerThreadForTest per_thread_for_test)
- : ::ppapi::PpapiGlobals(per_thread_for_test),
- resource_tracker_(ResourceTracker::SINGLE_THREADED) {
+ : ::ppapi::PpapiGlobals(per_thread_for_test) {
DCHECK(!host_globals_);
}
diff --git a/webkit/plugins/ppapi/host_var_tracker.cc b/webkit/plugins/ppapi/host_var_tracker.cc
index 6ff896e..adeeaa1 100644
--- a/webkit/plugins/ppapi/host_var_tracker.cc
+++ b/webkit/plugins/ppapi/host_var_tracker.cc
@@ -16,8 +16,7 @@ using ppapi::NPObjectVar;
namespace webkit {
namespace ppapi {
-HostVarTracker::HostVarTracker()
- : VarTracker(SINGLE_THREADED) {
+HostVarTracker::HostVarTracker() {
}
HostVarTracker::~HostVarTracker() {
@@ -28,7 +27,7 @@ ArrayBufferVar* HostVarTracker::CreateArrayBuffer(uint32 size_in_bytes) {
}
void HostVarTracker::AddNPObjectVar(NPObjectVar* object_var) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
InstanceMap::iterator found_instance = instance_map_.find(
object_var->pp_instance());
@@ -48,7 +47,7 @@ void HostVarTracker::AddNPObjectVar(NPObjectVar* object_var) {
}
void HostVarTracker::RemoveNPObjectVar(NPObjectVar* object_var) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
InstanceMap::iterator found_instance = instance_map_.find(
object_var->pp_instance());
@@ -73,7 +72,7 @@ void HostVarTracker::RemoveNPObjectVar(NPObjectVar* object_var) {
NPObjectVar* HostVarTracker::NPObjectVarForNPObject(PP_Instance instance,
NPObject* np_object) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
InstanceMap::iterator found_instance = instance_map_.find(instance);
if (found_instance == instance_map_.end())
@@ -88,7 +87,7 @@ NPObjectVar* HostVarTracker::NPObjectVarForNPObject(PP_Instance instance,
}
int HostVarTracker::GetLiveNPObjectVarsForInstance(PP_Instance instance) const {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
InstanceMap::const_iterator found = instance_map_.find(instance);
if (found == instance_map_.end())
@@ -97,7 +96,7 @@ int HostVarTracker::GetLiveNPObjectVarsForInstance(PP_Instance instance) const {
}
void HostVarTracker::DidDeleteInstance(PP_Instance instance) {
- CheckThreadingPreconditions();
+ DCHECK(CalledOnValidThread());
InstanceMap::iterator found_instance = instance_map_.find(instance);
if (found_instance == instance_map_.end())