From bd96c1e0f3c41f2254600c1285916f52ff29b92c Mon Sep 17 00:00:00 2001 From: dmichael Date: Wed, 11 Feb 2015 16:58:08 -0800 Subject: PPAPI: Make tests that disable the proxy lock re-enable it Rather than force all unit tests that rely on the ProxyLock to enable it, let's make sure that all tests that _disable_ it re-enable it on exit. BUG= R=teravest@chromium.org Committed: https://crrev.com/25ea5b2087b4c31fa42dea9b0de759d4895d0612 Cr-Commit-Position: refs/heads/master@{#315666} Review URL: https://codereview.chromium.org/908363002 Cr-Commit-Position: refs/heads/master@{#315895} --- content/renderer/pepper/pepper_file_chooser_host_unittest.cc | 3 ++- content/renderer/pepper/pepper_url_request_unittest.cc | 5 +++-- ppapi/proxy/ppapi_proxy_test.cc | 9 +-------- ppapi/proxy/ppapi_proxy_test.h | 4 ++++ ppapi/proxy/raw_var_data_unittest.cc | 1 - ppapi/shared_impl/proxy_lock.cc | 6 ++---- ppapi/shared_impl/proxy_lock.h | 11 +++++------ ppapi/shared_impl/tracked_callback_unittest.cc | 1 - ppapi/shared_impl/var_tracker_unittest.cc | 1 - 9 files changed, 17 insertions(+), 24 deletions(-) diff --git a/content/renderer/pepper/pepper_file_chooser_host_unittest.cc b/content/renderer/pepper/pepper_file_chooser_host_unittest.cc index bfc88e6..0585b04 100644 --- a/content/renderer/pepper/pepper_file_chooser_host_unittest.cc +++ b/content/renderer/pepper/pepper_file_chooser_host_unittest.cc @@ -36,7 +36,6 @@ class PepperFileChooserHostTest : public RenderViewTest { void SetUp() override { SetContentClient(&client_); RenderViewTest::SetUp(); - ppapi::ProxyLock::DisableLockingOnThreadForTest(); globals_.GetResourceTracker()->DidCreateInstance(pp_instance_); } @@ -51,6 +50,8 @@ class PepperFileChooserHostTest : public RenderViewTest { private: PP_Instance pp_instance_; + // Disables locking for the duration of the test. + ppapi::ProxyLock::LockingDisablerForTest disable_locking_; ppapi::TestGlobals globals_; TestContentClient client_; }; diff --git a/content/renderer/pepper/pepper_url_request_unittest.cc b/content/renderer/pepper/pepper_url_request_unittest.cc index e9008ec..bd5f3ae 100644 --- a/content/renderer/pepper/pepper_url_request_unittest.cc +++ b/content/renderer/pepper/pepper_url_request_unittest.cc @@ -59,8 +59,6 @@ class URLRequestInfoTest : public RenderViewTest { void SetUp() override { RenderViewTest::SetUp(); - ppapi::ProxyLock::DisableLockingOnThreadForTest(); - test_globals_.GetResourceTracker()->DidCreateInstance(pp_instance_); // This resource doesn't do IPC, so a null connection is fine. @@ -114,6 +112,9 @@ class URLRequestInfoTest : public RenderViewTest { PP_Instance pp_instance_; + // Disables locking for the duration of the test. + ppapi::ProxyLock::LockingDisablerForTest disable_locking_; + // Needs to be alive for resource tracking to work. ppapi::TestGlobals test_globals_; diff --git a/ppapi/proxy/ppapi_proxy_test.cc b/ppapi/proxy/ppapi_proxy_test.cc index acd533f..5a7867d 100644 --- a/ppapi/proxy/ppapi_proxy_test.cc +++ b/ppapi/proxy/ppapi_proxy_test.cc @@ -18,7 +18,6 @@ #include "ppapi/c/private/ppb_proxy_private.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/ppb_message_loop_proxy.h" -#include "ppapi/shared_impl/proxy_lock.h" namespace ppapi { namespace proxy { @@ -229,12 +228,8 @@ void PluginProxyTestHarness::CreatePluginGlobals() { if (globals_config_ == PER_THREAD_GLOBALS) { plugin_globals_.reset(new PluginGlobals(PpapiGlobals::PerThreadForTest())); PpapiGlobals::SetPpapiGlobalsOnThreadForTest(GetGlobals()); - // Enable locking in case some other unit test ran before us and disabled - // locking. - ProxyLock::EnableLockingOnThreadForTest(); } else { plugin_globals_.reset(new PluginGlobals()); - ProxyLock::EnableLockingOnThreadForTest(); } } @@ -464,13 +459,11 @@ void HostProxyTestHarness::TearDownHarness() { } void HostProxyTestHarness::CreateHostGlobals() { + disable_locking_.reset(new ProxyLock::LockingDisablerForTest); if (globals_config_ == PER_THREAD_GLOBALS) { host_globals_.reset(new TestGlobals(PpapiGlobals::PerThreadForTest())); PpapiGlobals::SetPpapiGlobalsOnThreadForTest(GetGlobals()); - // The host side of the proxy does not lock. - ProxyLock::DisableLockingOnThreadForTest(); } else { - ProxyLock::DisableLockingOnThreadForTest(); host_globals_.reset(new TestGlobals()); } } diff --git a/ppapi/proxy/ppapi_proxy_test.h b/ppapi/proxy/ppapi_proxy_test.h index ce9ec65..790c990 100644 --- a/ppapi/proxy/ppapi_proxy_test.h +++ b/ppapi/proxy/ppapi_proxy_test.h @@ -20,6 +20,7 @@ #include "ppapi/proxy/plugin_resource_tracker.h" #include "ppapi/proxy/plugin_var_tracker.h" #include "ppapi/proxy/resource_message_test_sink.h" +#include "ppapi/shared_impl/proxy_lock.h" #include "ppapi/shared_impl/test_globals.h" #include "testing/gtest/include/gtest/gtest.h" @@ -290,6 +291,9 @@ class HostProxyTestHarness : public ProxyTestHarnessBase { GlobalsConfiguration globals_config_; scoped_ptr host_globals_; scoped_ptr host_dispatcher_; + // The host side of the real proxy doesn't lock, so this disables locking for + // the thread the host side of the test runs on. + scoped_ptr disable_locking_; DelegateMock delegate_mock_; }; diff --git a/ppapi/proxy/raw_var_data_unittest.cc b/ppapi/proxy/raw_var_data_unittest.cc index e1c56a3..7dba345 100644 --- a/ppapi/proxy/raw_var_data_unittest.cc +++ b/ppapi/proxy/raw_var_data_unittest.cc @@ -38,7 +38,6 @@ class RawVarDataTest : public testing::Test { // testing::Test implementation. virtual void SetUp() { - ProxyLock::EnableLockingOnThreadForTest(); ProxyLock::Acquire(); } virtual void TearDown() { diff --git a/ppapi/shared_impl/proxy_lock.cc b/ppapi/shared_impl/proxy_lock.cc index 6598379..cd302f9 100644 --- a/ppapi/shared_impl/proxy_lock.cc +++ b/ppapi/shared_impl/proxy_lock.cc @@ -79,15 +79,13 @@ void ProxyLock::DisableLocking() { g_disable_locking = true; } -// static -void ProxyLock::DisableLockingOnThreadForTest() { +ProxyLock::LockingDisablerForTest::LockingDisablerForTest() { // Note, we don't DCHECK that this flag isn't already set, because multiple // unit tests may run in succession and all set it. g_disable_locking_for_thread.Get().Set(true); } -// static -void ProxyLock::EnableLockingOnThreadForTest() { +ProxyLock::LockingDisablerForTest::~LockingDisablerForTest() { g_disable_locking_for_thread.Get().Set(false); } diff --git a/ppapi/shared_impl/proxy_lock.h b/ppapi/shared_impl/proxy_lock.h index 23afc0e..39b653e 100644 --- a/ppapi/shared_impl/proxy_lock.h +++ b/ppapi/shared_impl/proxy_lock.h @@ -59,12 +59,11 @@ class PPAPI_SHARED_EXPORT ProxyLock { // We have some unit tests where one thread pretends to be the host and one // pretends to be the plugin. This allows the lock to do nothing on only one // thread to support these tests. See TwoWayTest for more information. - static void DisableLockingOnThreadForTest(); - - // Enables locking on the current thread. Although locking is enabled by - // default, unit tests that rely on the lock being enabled should *still* - // call this, since a previous test may have disabled locking. - static void EnableLockingOnThreadForTest(); + class PPAPI_SHARED_EXPORT LockingDisablerForTest { + public: + LockingDisablerForTest(); + ~LockingDisablerForTest(); + }; private: friend class content::HostGlobals; diff --git a/ppapi/shared_impl/tracked_callback_unittest.cc b/ppapi/shared_impl/tracked_callback_unittest.cc index 1819947..12ad423 100644 --- a/ppapi/shared_impl/tracked_callback_unittest.cc +++ b/ppapi/shared_impl/tracked_callback_unittest.cc @@ -26,7 +26,6 @@ class TrackedCallbackTest : public testing::Test { PP_Instance pp_instance() const { return pp_instance_; } virtual void SetUp() override { - ProxyLock::EnableLockingOnThreadForTest(); ProxyAutoLock lock; globals_.GetResourceTracker()->DidCreateInstance(pp_instance_); } diff --git a/ppapi/shared_impl/var_tracker_unittest.cc b/ppapi/shared_impl/var_tracker_unittest.cc index fc089ab..fb8fda3 100644 --- a/ppapi/shared_impl/var_tracker_unittest.cc +++ b/ppapi/shared_impl/var_tracker_unittest.cc @@ -42,7 +42,6 @@ class VarTrackerTest : public testing::Test { // Test implementation. virtual void SetUp() override { ASSERT_EQ(0, mock_var_alive_count); - ProxyLock::EnableLockingOnThreadForTest(); } virtual void TearDown() override {} -- cgit v1.1