Revert of Minor changes to allow Pepper InstancePrivate tests to pass with gin...

Revert of Minor changes to allow Pepper InstancePrivate tests to pass with gin (patchset #3 of https://codereview.chromium.org/472693002/)

Reason for revert:
Caused failures in DrMemory:

UNADDRESSABLE ACCESS of freed memory: writing 0x031c0278-0x031c0279 1 byte(s)
# 0 ppapi_tests.dll!`anonymous namespace'::InstanceSO::~InstanceSO             [ppapi\tests\test_instance_deprecated.cc:48]
# 1 ppapi_tests.dll!`anonymous namespace'::InstanceSO::`scalar deleting destructor'
# 2 ppapi_tests.dll!pp::deprecated::`anonymous namespace'::Deallocate          [ppapi\cpp\dev\scriptable_object_deprecated.cc:126]
# 3 ppapi_proxy.dll!ppapi::CallWhileUnlocked<>                                 [ppapi\shared_impl\proxy_lock.h:123]
# 4 ppapi_proxy.dll!ppapi::proxy::PluginVarTracker::DidDeleteInstance          [ppapi\proxy\plugin_var_tracker.cc:281]
# 5 ppapi_proxy.dll!ppapi::proxy::PPP_Instance_Proxy::OnPluginMsgDidDestroy    [ppapi\proxy\ppp_instance_proxy.cc:194]
# 6 ppapi_proxy.dll!PpapiMsg_PPPInstance_DidDestroy::Dispatch<>                [ppapi\proxy\ppapi_messages.h:670]
# 7 ppapi_proxy.dll!ppapi::proxy::PPP_Instance_Proxy::OnMessageReceived        [ppapi\proxy\ppp_instance_proxy.cc:143]
# 8 ppapi_proxy.dll!ppapi::proxy::Dispatcher::OnMessageReceived                [ppapi\proxy\dispatcher.cc:69]
# 9 ppapi_proxy.dll!ppapi::proxy::PluginDispatcher::OnMessageReceived          [ppapi\proxy\plugin_dispatcher.cc:238]
#10 ipc.dll!IPC::ChannelProxy::Context::OnDispatchMessage                      [ipc\ipc_channel_proxy.cc:273]
#11 ipc.dll!base::internal::Invoker<>::Run                                     [base\bind_internal.h:1253]
#12 base.dll!base::debug::TaskAnnotator::RunTask                               [base\debug\task_annotator.cc:62]
#13 base.dll!base::MessageLoop::RunTask                                        [base\message_loop\message_loop.cc:436]
#14 base.dll!base::MessageLoop::DeferOrRunPendingTask                          [base\message_loop\message_loop.cc:445]
#15 base.dll!base::MessageLoop::DoWork                                         [base\message_loop\message_loop.cc:552]
#16 base.dll!base::MessagePumpDefault::Run                                     [base\message_loop\message_pump_default.cc:32]
#17 base.dll!base::MessageLoop::RunHandler                                     [base\message_loop\message_loop.cc:408]
#18 content.dll!content::PpapiPluginMain                                       [content\ppapi_plugin\ppapi_plugin_main.cc:141]
#19 content.dll!content::RunNamedProcessTypeMain                               [content\app\content_main_runner.cc:415]
#20 content.dll!content::ContentMainRunnerImpl::Run                            [content\app\content_main_runner.cc:764]
#21 content.dll!content::ContentMain                                           [content\app\content_main.cc:19]
#22 content::LaunchTests                                                       [content\public\test\test_launcher.cc:475]
#23 main                                                                       [content\test\content_test_launcher.cc:123]

First appeared in http://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Content%20Browser%20%28DrMemory%29/builds/631/steps/memory%20test%3A%20content_browsertests/logs/stdio.

Original issue's description:
> 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

TBR=dmichael@chromium.org,raymes@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=351636,407372

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

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

content/content_tests.gypi

@@ -740,7 +740,6 @@
             '../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,7 +6,6 @@
 #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"
@@ -16,7 +15,7 @@ namespace content {
 class PepperPluginInstanceImpl;
 
 // Base class for scripting TryCatch helpers.
-class CONTENT_EXPORT PepperTryCatch {
+class 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 PPAPI_SHARED_EXPORT ScopedPPVarArray {
+class ScopedPPVarArray {
  public:
   struct PassPPBMemoryAllocatedArray {};
 

ppapi/tests/test_instance_deprecated.cc

@@ -40,12 +40,40 @@ class InstanceSO : public pp::deprecated::ScriptableObject {
 InstanceSO::InstanceSO(TestInstance* i)
     : test_instance_(i),
       testing_interface_(i->testing_interface()) {
-  if (testing_interface_->IsOutOfProcess() == PP_FALSE)
-    test_instance_->set_instance_object_destroyed(false);
+  // 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() {
-  test_instance_->set_instance_object_destroyed(true);
+  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.
+  }
 }
 
 bool InstanceSO::HasMethod(const pp::Var& name, pp::Var* exception) {
@@ -89,9 +117,8 @@ pp::Var InstanceSO::Call(const pp::Var& method_name,
 
 REGISTER_TEST_CASE(Instance);
 
-TestInstance::TestInstance(TestingInstance* instance)
-    : TestCase(instance),
-      instance_object_destroyed_(false) {}
+TestInstance::TestInstance(TestingInstance* instance) : TestCase(instance) {
+}
 
 bool TestInstance::Init() {
   return true;
@@ -99,32 +126,11 @@ bool TestInstance::Init() {
 
 TestInstance::~TestInstance() {
   ResetTestObject();
-  if (testing_interface_->IsOutOfProcess() == PP_FALSE) {
-    // This should cause the instance objects descructor to be called.
+  // 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)
     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

ppapi/tests/test_instance_deprecated.h

@@ -26,10 +26,6 @@ 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();
@@ -44,9 +40,6 @@ 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_