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

Committed: https://chromium.googlesource.com/chromium/src/+/92c3074f4b4417924115f880657f1ec15da0e0ba

Review URL: https://codereview.chromium.org/472693002

Cr-Commit-Position: refs/heads/master@{#292102}

content/content_tests.gypi

@@ -751,6 +751,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',

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);

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 {};
 

ppapi/tests/test_instance_deprecated.cc

@@ -8,72 +8,20 @@
 
 #include "ppapi/c/ppb_var.h"
 #include "ppapi/cpp/module.h"
-#include "ppapi/cpp/dev/scriptable_object_deprecated.h"
 #include "ppapi/tests/testing_instance.h"
 
-namespace {
-
 static const char kSetValueFunction[] = "SetValue";
 static const char kSetExceptionFunction[] = "SetException";
 static const char kReturnValueFunction[] = "ReturnValue";
 
-// ScriptableObject used by instance.
-class InstanceSO : public pp::deprecated::ScriptableObject {
- public:
-  explicit InstanceSO(TestInstance* i);
-  virtual ~InstanceSO();
-
-  // pp::deprecated::ScriptableObject overrides.
-  bool HasMethod(const pp::Var& name, pp::Var* exception);
-  pp::Var Call(const pp::Var& name,
-               const std::vector<pp::Var>& args,
-               pp::Var* exception);
-
- private:
-  TestInstance* test_instance_;
-  // For out-of-process, the InstanceSO might be deleted after the instance was
-  // already destroyed, so we can't rely on test_instance_->testing_interface()
-  // being valid. Therefore we store our own.
-  const PPB_Testing_Private* testing_interface_;
-};
-
 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"
-      );
-  }
 }
 
 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.
-  }
+  if (test_instance_)
+    test_instance_->clear_instance_so();
 }
 
 bool InstanceSO::HasMethod(const pp::Var& name, pp::Var* exception) {
@@ -94,7 +42,7 @@ pp::Var InstanceSO::Call(const pp::Var& method_name,
   if (name == kSetValueFunction) {
     if (args.size() != 1 || !args[0].is_string())
       *exception = pp::Var("Bad argument to SetValue(<string>)");
-    else
+    else if (test_instance_)
       test_instance_->set_string(args[0].AsString());
   } else if (name == kSetExceptionFunction) {
     if (args.size() != 1 || !args[0].is_string())
@@ -113,12 +61,11 @@ pp::Var InstanceSO::Call(const pp::Var& method_name,
   return pp::Var();
 }
 
-}  // namespace
-
 REGISTER_TEST_CASE(Instance);
 
-TestInstance::TestInstance(TestingInstance* instance) : TestCase(instance) {
-}
+TestInstance::TestInstance(TestingInstance* instance)
+    : TestCase(instance),
+      instance_so_(NULL) {}
 
 bool TestInstance::Init() {
   return true;
@@ -126,11 +73,33 @@ 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 object's 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_so_);
+  } else {
+    // Out-of-process, this destructor might not actually get invoked. Clear
+    // the InstanceSOs reference to the instance so there is no UAF.
+    if (instance_so_)
+      instance_so_->clear_test_instance();
+  }
+
   // 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
@@ -159,7 +128,9 @@ void TestInstance::LeakReferenceAndIgnore(const pp::Var& leaked) {
 }
 
 pp::deprecated::ScriptableObject* TestInstance::CreateTestObject() {
-  return new InstanceSO(this);
+  if (!instance_so_)
+    instance_so_ = new InstanceSO(this);
+  return instance_so_;
 }
 
 std::string TestInstance::TestExecuteScript() {

ppapi/tests/test_instance_deprecated.h

@@ -8,8 +8,32 @@
 #include <string>
 #include <vector>
 
+#include "ppapi/cpp/dev/scriptable_object_deprecated.h"
 #include "ppapi/tests/test_case.h"
 
+class TestInstance;
+
+// ScriptableObject used by TestInstance.
+class InstanceSO : public pp::deprecated::ScriptableObject {
+ public:
+  explicit InstanceSO(TestInstance* i);
+  virtual ~InstanceSO();
+
+  // pp::deprecated::ScriptableObject overrides.
+  bool HasMethod(const pp::Var& name, pp::Var* exception);
+  pp::Var Call(const pp::Var& name,
+               const std::vector<pp::Var>& args,
+               pp::Var* exception);
+
+  // For out-of-process, the InstanceSO might be deleted after the instance was
+  // already destroyed, so we can't rely on test_instance_ being valid.
+  void clear_test_instance() { test_instance_ = NULL; }
+
+ private:
+  TestInstance* test_instance_;
+  const PPB_Testing_Private* testing_interface_;
+};
+
 class TestInstance : public TestCase {
  public:
   TestInstance(TestingInstance* instance);
@@ -26,6 +50,10 @@ class TestInstance : public TestCase {
   // happens on instance shutdown.
   void LeakReferenceAndIgnore(const pp::Var& leaked);
 
+  // This resets the scriptable object if it gets destroyed before the instance
+  // which should be the case for in-process tests.
+  void clear_instance_so() { instance_so_ = NULL; }
+
  protected:
   // Test case protected overrides.
   virtual pp::deprecated::ScriptableObject* CreateTestObject();
@@ -40,6 +68,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_;
+
+  // The scriptable object for this test.
+  InstanceSO* instance_so_;
 };
 
 #endif  // PPAPI_TESTS_TEST_INSTANCE_H_