Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(277)

Unified Diff: ppapi/tests/test_instance_deprecated.cc

Issue 472693002: Minor changes to allow Pepper InstancePrivate tests to pass with gin (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « ppapi/tests/test_instance_deprecated.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ppapi/tests/test_instance_deprecated.cc
diff --git a/ppapi/tests/test_instance_deprecated.cc b/ppapi/tests/test_instance_deprecated.cc
index 6746858e2a9edb6fbe8f2d1141e995c0584f5cb8..fe6e36f2980ae7537e0bfd2edbf4680560fbf697 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(
dmichael (off chromium) 2014/08/20 17:55:53 FWIW, I think this was the only usage of "AddPostC
- "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
« no previous file with comments | « ppapi/tests/test_instance_deprecated.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698