Various fixes to make ppapi_unittests pass again.

ppapi/proxy/ppp_instance_private_proxy_unittest.cc

Index: ppapi/proxy/ppp_instance_private_proxy_unittest.cc
diff --git a/ppapi/proxy/ppp_instance_private_proxy_unittest.cc b/ppapi/proxy/ppp_instance_private_proxy_unittest.cc
index 957a3a7ad87fb3c2e837d91aa1f789d5f59f3faf..898c776913d399fc9e9afc47b6077bcb0710a6da 100644
--- a/ppapi/proxy/ppp_instance_private_proxy_unittest.cc
+++ b/ppapi/proxy/ppp_instance_private_proxy_unittest.cc
@@ -66,6 +66,24 @@ 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*/,
@@ -73,7 +91,9 @@ PP_Bool DidCreate(PP_Instance /*instance*/, uint32_t /*argc*/,
   // 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;
 }
 
@@ -110,8 +130,6 @@ const PPB_Var_Deprecated ppb_var_deprecated_mock = {
   &CreateObject
 };
 
-}  // namespace
-
 class PPP_Instance_Private_ProxyTest : public TwoWayTest {
  public:
    PPP_Instance_Private_ProxyTest()
@@ -125,6 +143,22 @@ class PPP_Instance_Private_ProxyTest : public TwoWayTest {
   }
 };
 
+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.
@@ -149,11 +183,16 @@ TEST_F(PPP_Instance_Private_ProxyTest, PPPInstancePrivate) {
   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(
@@ -164,7 +203,10 @@ TEST_F(PPP_Instance_Private_ProxyTest, PPPInstancePrivate) {
   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));
@@ -175,7 +217,10 @@ TEST_F(PPP_Instance_Private_ProxyTest, PPPInstancePrivate) {
   // 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));
 }