Various fixes to make ppapi_unittests pass again.

ppapi/proxy/ppp_instance_private_proxy_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "base/test/test_timeouts.h"
 #include "base/time.h"
 #include "ppapi/c/dev/ppb_var_deprecated.h"
 #include "ppapi/c/dev/ppp_class_deprecated.h"
@@ skipping 48 matching lines @@
   // have a ref of our own.
   printf("GetInstanceObject called\n");
   plugin_var_deprecated_if()->AddRef(instance_obj);
   return instance_obj;
 }
 
 PPP_Instance_Private ppp_instance_private_mock = {
   &GetInstanceObject
 };
 
+// We need to pass in a |PPP_Class_Deprecated| to
+// |PPB_Var_Deprecated->CreateObject| for a mock |Deallocate| method.
+void Deallocate(void* object) {
+}
+
+const PPP_Class_Deprecated ppp_class_deprecated_mock = {
+    NULL, // HasProperty
+    NULL, // HasMethod
+    NULL, // GetProperty
+    NULL, // GetAllPropertyNames
+    NULL, // SetProperty
+    NULL, // RemoveProperty
+    NULL, // Call
+    NULL, // Construct
+    &Deallocate
+};
+
+
 // We need to mock PPP_Instance, so that we can create and destroy the pretend
 // instance that PPP_Instance_Private uses.
 PP_Bool DidCreate(PP_Instance /*instance*/, uint32_t /*argc*/,
                   const char* /*argn*/[], const char* /*argv*/[]) {
   // Create an object var. This should exercise the typical path for creating
   // instance objects.
   instance_obj =
-      plugin_var_deprecated_if()->CreateObject(kInstance, NULL, NULL);
+      plugin_var_deprecated_if()->CreateObject(kInstance,
+                                               &ppp_class_deprecated_mock,
+                                               NULL);
   return PP_TRUE;
 }
 
 void DidDestroy(PP_Instance /*instance*/) {
   // Decrement the reference count for our instance object. It should be
   // deleted.
   plugin_var_deprecated_if()->Release(instance_obj);
 }
 
 PPP_Instance_1_0 ppp_instance_mock = { &DidCreate, &DidDestroy };
@@ skipping 16 matching lines @@
   NULL, // GetProperty
   NULL, // EnumerateProperties
   NULL, // SetProperty
   NULL, // RemoveProperty
   NULL, // Call
   NULL, // Construct
   NULL, // IsInstanceOf
   &CreateObject
 };
 
-}  // namespace
-
 class PPP_Instance_Private_ProxyTest : public TwoWayTest {
  public:
    PPP_Instance_Private_ProxyTest()
        : TwoWayTest(TwoWayTest::TEST_PPP_INTERFACE) {
       plugin().RegisterTestInterface(PPP_INSTANCE_PRIVATE_INTERFACE,
                                      &ppp_instance_private_mock);
       plugin().RegisterTestInterface(PPP_INSTANCE_INTERFACE_1_0,
                                      &ppp_instance_mock);
       host().RegisterTestInterface(PPB_VAR_DEPRECATED_INTERFACE,
                                    &ppb_var_deprecated_mock);
   }
 };
 
+void GetRefCountForObject(PluginProxyTestHarness* plugin_harness,
+                          PP_Var instance_obj,
+                          int32_t* out_ref_count) {
+  *out_ref_count =
+      plugin_harness->var_tracker().GetRefCountForObject(instance_obj);
+}
+
+void GetHostObjectId(PluginProxyTestHarness* plugin_harness,
+                     PP_Var instance_obj,
+                     int32_t* out_id) {
+  *out_id =
+      plugin_harness->var_tracker().GetHostObject(instance_obj).value.as_id;
+}
+
+}  // namespace
+
 TEST_F(PPP_Instance_Private_ProxyTest, PPPInstancePrivate) {
   // This test controls its own instance; we can't use the one that
   // PluginProxyTestHarness provides.
   ASSERT_NE(kInstance, pp_instance());
   HostDispatcher::SetForInstance(kInstance, host().host_dispatcher());
 
   // This file-local global is used by the PPP_Instance mock above in order to
   // access PPB_Var_Deprecated.
   plugin_dispatcher = plugin().plugin_dispatcher();
 
   // Grab the host-side proxy for PPP_Instance and PPP_Instance_Private.
   const PPP_Instance_Private* ppp_instance_private =
       static_cast<const PPP_Instance_Private*>(
           host().host_dispatcher()->GetProxiedInterface(
               PPP_INSTANCE_PRIVATE_INTERFACE));
   const PPP_Instance_1_1* ppp_instance = static_cast<const PPP_Instance_1_1*>(
       host().host_dispatcher()->GetProxiedInterface(
           PPP_INSTANCE_INTERFACE_1_1));
 
   // Initialize an Instance, so that the plugin-side machinery will work
   // properly.
   EXPECT_EQ(PP_TRUE, ppp_instance->DidCreate(kInstance, 0, NULL, NULL));
 
   // Check the plugin-side reference count.
-  EXPECT_EQ(1, plugin().var_tracker().GetRefCountForObject(instance_obj));
+  int32_t ref_count = 0;
+  PostTaskOnRemoteHarness(base::Bind(GetRefCountForObject,
+                                     &plugin(), instance_obj, &ref_count));
+  EXPECT_EQ(1, ref_count);

[dmichael (off chromium), 2012/09/17 15:27:29]

This is fine, but I'd also be okay with just disabling this test entirely for
builds that don't have ENABLE_PEPPER_THREADING. It won't be too long, and then
we won't need the extra gyrations that are really only there to appease the
ThreadChecker; there's no real race condition even without the locking.

WDYT?

[raymes, 2012/09/18 21:29:15]

Done.

   // Check the host-side var exists with the expected id and has 1 refcount (the
   // refcount on behalf of the plugin).
-  int32 expected_host_id =
-      plugin().var_tracker().GetHostObject(instance_obj).value.as_id;
+  int32 expected_host_id = 0;
+  PostTaskOnRemoteHarness(base::Bind(GetHostObjectId,
+                                     &plugin(), instance_obj,
+                                     &expected_host_id));
   Var* host_var = host().var_tracker().GetVar(expected_host_id);
   ASSERT_TRUE(host_var);
   EXPECT_EQ(
       1,
       host().var_tracker().GetRefCountForObject(GetPPVarNoAddRef(host_var)));
 
   // Call from the browser side to get the instance object.
   PP_Var host_pp_var = ppp_instance_private->GetInstanceObject(kInstance);
   EXPECT_EQ(instance_obj.type, host_pp_var.type);
   EXPECT_EQ(host_pp_var.value.as_id, expected_host_id);
-  EXPECT_EQ(1, plugin().var_tracker().GetRefCountForObject(instance_obj));
+  ref_count = 0;
+  PostTaskOnRemoteHarness(base::Bind(GetRefCountForObject,
+                                     &plugin(), instance_obj, &ref_count));
+  EXPECT_EQ(1, ref_count);
   // A reference is passed to the browser, which we consume here.
   host().var_tracker().ReleaseVar(host_pp_var);
   EXPECT_EQ(1, host().var_tracker().GetRefCountForObject(host_pp_var));
 
   // The plugin is going away; generally, so will all references to its instance
   // object.
   host().var_tracker().ReleaseVar(host_pp_var);
   // Destroy the instance. DidDestroy above decrements the reference count for
   // instance_obj, so it should also be destroyed.
   ppp_instance->DidDestroy(kInstance);
-  EXPECT_EQ(-1, plugin().var_tracker().GetRefCountForObject(instance_obj));
+  ref_count = 0;
+  PostTaskOnRemoteHarness(base::Bind(GetRefCountForObject,
+                                     &plugin(), instance_obj, &ref_count));
+  EXPECT_EQ(-1, ref_count);
   EXPECT_EQ(-1, host().var_tracker().GetRefCountForObject(host_pp_var));
 }
 
 }  // namespace proxy
 }  // namespace ppapi