diff options
author | raymes <raymes@chromium.org> | 2014-08-24 20:00:03 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-08-25 03:01:07 +0000 |
commit | 41480ebd0095ac41c5b5f4e2fdbadedd8f69e041 (patch) | |
tree | 47b493ffad1f4b1c829208b7b44008977d10deee | |
parent | 1cc19e420c43d6ec628a0b62bdcc86625d366a65 (diff) | |
download | chromium_src-41480ebd0095ac41c5b5f4e2fdbadedd8f69e041.zip chromium_src-41480ebd0095ac41c5b5f4e2fdbadedd8f69e041.tar.gz chromium_src-41480ebd0095ac41c5b5f4e2fdbadedd8f69e041.tar.bz2 |
Minor changes to allow Pepper InstancePrivate tests to pass with gin
This includes some minor changes which allow tests to when NPObject is replaced
by gin in pepper:
-Add shared library exports for use of PepperTryCatch and ScopedPPVarArray in
tests.
-Add gin as a dependency to content_unittests for use in host_var_tracker_unittest
-Change TestInstanceDeprecated so that it doesn't execute a script from the
ScriptableObject upon destruction. The reason for this is that the garbage
collector is manually run from the destructor of the plugin Instance object.
This results in destruction of the ScriptableObject and then javascript is
re-entered via ExecuteScript. This should never happen outside tests because
the garbage collector won't be run synchronously. Instead of running
ExecuteScript, which just set a variable in the Instance object to verify
that the ScriptableObject was correctly destroyed.
BUG=351636
Review URL: https://codereview.chromium.org/472693002
Cr-Commit-Position: refs/heads/master@{#291636}
-rw-r--r-- | content/content_tests.gypi | 1 | ||||
-rw-r--r-- | content/renderer/pepper/pepper_try_catch.h | 3 | ||||
-rw-r--r-- | ppapi/shared_impl/scoped_pp_var.h | 2 | ||||
-rw-r--r-- | ppapi/tests/test_instance_deprecated.cc | 68 | ||||
-rw-r--r-- | ppapi/tests/test_instance_deprecated.h | 7 |
5 files changed, 42 insertions, 39 deletions
diff --git a/content/content_tests.gypi b/content/content_tests.gypi index 9f85ca5..105139c 100644 --- a/content/content_tests.gypi +++ b/content/content_tests.gypi @@ -739,6 +739,7 @@ '../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations', '../cc/cc.gyp:cc', '../cc/cc_tests.gyp:cc_test_support', + '../gin/gin.gyp:gin', '../gpu/gpu.gyp:gpu', '../gpu/gpu.gyp:gpu_unittest_utils', '../ipc/ipc.gyp:test_support_ipc', diff --git a/content/renderer/pepper/pepper_try_catch.h b/content/renderer/pepper/pepper_try_catch.h index f82c0c3..6b76717 100644 --- a/content/renderer/pepper/pepper_try_catch.h +++ b/content/renderer/pepper/pepper_try_catch.h @@ -6,6 +6,7 @@ #define CONTENT_RENDERER_PEPPER_PEPPER_TRY_CATCH_H_ #include "base/basictypes.h" +#include "content/common/content_export.h" #include "ppapi/c/pp_var.h" #include "ppapi/shared_impl/scoped_pp_var.h" #include "v8/include/v8.h" @@ -15,7 +16,7 @@ namespace content { class PepperPluginInstanceImpl; // Base class for scripting TryCatch helpers. -class PepperTryCatch { +class CONTENT_EXPORT PepperTryCatch { public: PepperTryCatch(PepperPluginInstanceImpl* instance, bool convert_objects); diff --git a/ppapi/shared_impl/scoped_pp_var.h b/ppapi/shared_impl/scoped_pp_var.h index 44d0ff1..f1e1347 100644 --- a/ppapi/shared_impl/scoped_pp_var.h +++ b/ppapi/shared_impl/scoped_pp_var.h @@ -47,7 +47,7 @@ class PPAPI_SHARED_EXPORT ScopedPPVar { // An array of PP_Vars which will be deallocated and have their references // decremented when they go out of scope. -class ScopedPPVarArray { +class PPAPI_SHARED_EXPORT ScopedPPVarArray { public: struct PassPPBMemoryAllocatedArray {}; diff --git a/ppapi/tests/test_instance_deprecated.cc b/ppapi/tests/test_instance_deprecated.cc index 6746858..fe6e36f 100644 --- a/ppapi/tests/test_instance_deprecated.cc +++ b/ppapi/tests/test_instance_deprecated.cc @@ -40,40 +40,12 @@ class InstanceSO : public pp::deprecated::ScriptableObject { InstanceSO::InstanceSO(TestInstance* i) : test_instance_(i), testing_interface_(i->testing_interface()) { - // Set up a post-condition for the test so that we can ensure our destructor - // is called. This only works reliably in-process. Out-of-process, it only - // can work when the renderer stays alive a short while after the plugin - // instance is destroyed. If the renderer is being shut down, too much happens - // asynchronously for the out-of-process case to work reliably. In - // particular: - // - The Var ReleaseObject message is asynchronous. - // - The PPB_Var_Deprecated host-side proxy posts a task to actually release - // the object when the ReleaseObject message is received. - // - The PPP_Class Deallocate message is asynchronous. - // At time of writing this comment, if you modify the code so that the above - // happens synchronously, and you remove the restriction that the plugin can't - // be unblocked by a sync message, then this check actually passes reliably - // for out-of-process. But we don't want to make any of those changes, so we - // just skip the check. - if (testing_interface_->IsOutOfProcess() == PP_FALSE) { - i->instance()->AddPostCondition( - "window.document.getElementById('container').instance_object_destroyed" - ); - } + if (testing_interface_->IsOutOfProcess() == PP_FALSE) + test_instance_->set_instance_object_destroyed(false); } InstanceSO::~InstanceSO() { - if (testing_interface_->IsOutOfProcess() == PP_FALSE) { - // TODO(dmichael): It would probably be best to make in-process consistent - // with out-of-process. That would mean that the instance - // would already be destroyed at this point. - pp::Var ret = test_instance_->instance()->ExecuteScript( - "document.getElementById('container').instance_object_destroyed=true;"); - } else { - // Out-of-process, this destructor might not actually get invoked. See the - // comment in InstanceSO's constructor for an explanation. Also, instance() - // has already been destroyed :-(. So we can't really do anything here. - } + test_instance_->set_instance_object_destroyed(true); } bool InstanceSO::HasMethod(const pp::Var& name, pp::Var* exception) { @@ -117,8 +89,9 @@ pp::Var InstanceSO::Call(const pp::Var& method_name, REGISTER_TEST_CASE(Instance); -TestInstance::TestInstance(TestingInstance* instance) : TestCase(instance) { -} +TestInstance::TestInstance(TestingInstance* instance) + : TestCase(instance), + instance_object_destroyed_(false) {} bool TestInstance::Init() { return true; @@ -126,11 +99,32 @@ bool TestInstance::Init() { TestInstance::~TestInstance() { ResetTestObject(); - // When running tests in process, some post conditions check that teardown - // happened successfully. We need to run the garbage collector to ensure that - // vars get released. - if (testing_interface_->IsOutOfProcess() == PP_FALSE) + if (testing_interface_->IsOutOfProcess() == PP_FALSE) { + // This should cause the instance objects descructor to be called. testing_interface_->RunV8GC(instance_->pp_instance()); + + // Test a post-condition which ensures the instance objects destructor is + // called. This only works reliably in-process. Out-of-process, it only + // can work when the renderer stays alive a short while after the plugin + // instance is destroyed. If the renderer is being shut down, too much + // happens asynchronously for the out-of-process case to work reliably. In + // particular: + // - The Var ReleaseObject message is asynchronous. + // - The PPB_Var_Deprecated host-side proxy posts a task to actually + // release the object when the ReleaseObject message is received. + // - The PPP_Class Deallocate message is asynchronous. + // At time of writing this comment, if you modify the code so that the above + // happens synchronously, and you remove the restriction that the plugin + // can't be unblocked by a sync message, then this check actually passes + // reliably for out-of-process. But we don't want to make any of those + // changes so we just skip the check. + PP_DCHECK(instance_object_destroyed_); + } else { + // Out-of-process, this destructor might not actually get invoked. See the + // comment in InstanceSO's constructor for an explanation. Also, instance() + // has already been destroyed :-(. So we can't really do anything here. + } + // Save the fact that we were destroyed in sessionStorage. This tests that // we can ExecuteScript at instance destruction without crashing. It also // allows us to check that ExecuteScript will run and succeed in certain diff --git a/ppapi/tests/test_instance_deprecated.h b/ppapi/tests/test_instance_deprecated.h index dfccd13..58b3efe 100644 --- a/ppapi/tests/test_instance_deprecated.h +++ b/ppapi/tests/test_instance_deprecated.h @@ -26,6 +26,10 @@ class TestInstance : public TestCase { // happens on instance shutdown. void LeakReferenceAndIgnore(const pp::Var& leaked); + void set_instance_object_destroyed(bool destroyed) { + instance_object_destroyed_ = destroyed; + } + protected: // Test case protected overrides. virtual pp::deprecated::ScriptableObject* CreateTestObject(); @@ -40,6 +44,9 @@ class TestInstance : public TestCase { // Value written by set_string which is called by the ScriptableObject. This // allows us to keep track of what was called. std::string string_; + + // Whether the instance object for this instance has been destroyed. + bool instance_object_destroyed_; }; #endif // PPAPI_TESTS_TEST_INSTANCE_H_ |