Revert 187427 "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 > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=187340 > > Review URL: https://chromiumcodereview.appspot.com/12378050 TBR=dmichael@chromium.org Review URL: https://codereview.chromium.org/12457021 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@187716 0039d316-1c4b-4281-b951-d872f2087c98
SaschaDeWaal · Mar 13, 2013 · b465d46 · b465d46
1 parent 4e8ee2c
commit b465d46
Show file tree

Hide file tree

Showing 33 changed files with 226 additions and 259 deletions.
diff --git a/build/common.gypi b/build/common.gypi
@@ -321,6 +321,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,
@@ -668,6 +673,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)',
@@ -1764,6 +1770,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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