NaCl: Remove SRPC-based manifest tests.

chrome/test/data/nacl/manifest_file/irt_manifest_file_test.cc

 /*
  * Copyright 2014 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.
  */
 
 //
 // Test for resource open before PPAPI initialization.
 //
 
 #include <stdio.h>
 #include <string.h>
 
 #include <sstream>
 #include <string>
 #include <vector>
 
 #include "native_client/src/untrusted/irt/irt.h"
 #include "native_client/src/untrusted/nacl/nacl_irt.h"
 
+#include "ppapi/cpp/completion_callback.h"
 #include "ppapi/cpp/instance.h"
 #include "ppapi/cpp/module.h"
 #include "ppapi/cpp/var.h"
 #include "ppapi/cpp/var_array.h"
 #include "ppapi/native_client/src/shared/ppapi_proxy/ppruntime.h"
 
 
 std::vector<std::string> result;
 
+pp::Instance* g_instance = NULL;
+
 std::string LoadManifestSuccess(TYPE_nacl_irt_query *query_func) {
   struct nacl_irt_resource_open nacl_irt_resource_open;
   if (sizeof(nacl_irt_resource_open) !=
       (*query_func)(
           NACL_IRT_RESOURCE_OPEN_v0_1,
           &nacl_irt_resource_open,
           sizeof(nacl_irt_resource_open))) {
     return "irt manifest api not found";
   }
   int desc;
@@ skipping 65 matching lines @@
   if (error != ENOENT) {
     printf("Unexpected error code: %d\n", error);
     char buf[80];
     snprintf(buf, sizeof(buf), "open_resource() result: %d", error);
     return std::string(buf);
   }
 
   return "Pass";
 }
 
+void RunTests() {
+  result.push_back(LoadManifestSuccess(&__nacl_irt_query));
+  result.push_back(LoadManifestNonExistentEntry(&__nacl_irt_query));
+  result.push_back(LoadManifestNonExistentFile(&__nacl_irt_query));
+}
+
+void PostReply(void* user_data, int32_t status) {
+  pp::VarArray reply = pp::VarArray();
+  for (size_t i = 0; i < result.size(); ++i)
+    reply.Set(i, pp::Var(result[i]));
+  g_instance->PostMessage(reply);
+}
+
+void* RunTestsOnBackgroundThread(void *thread_id) {
+  RunTests();
+  pp::Module::Get()->core()->CallOnMainThread(

[Mark Seaborn, 2014/09/04 17:41:36]

BTW, why does this require a CallOnMainThread()?  I thought
g_instance->PostMessage() would work on any thread.

[teravest, 2014/09/04 20:21:28]

pp::Instance::PostMessage() has to be called from the main thread. I could call
this from a background thread if I used PPB_Instance, it sounds like.

[Mark Seaborn, 2014/09/04 22:00:41]

Hmm, you mean the C++ wrappers for PPAPI add a thread restriction?

[teravest, 2014/09/04 22:12:39]

I believe that only happens for pp::Instance.
There's an explanatory comment here:
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/cpp/instance...

And the pp::Instance object is deleted here:
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/cpp/module.c...

Thanks to dmichael@ for pointing me at the details of this one.

[dmichael (off chromium), 2014/09/05 19:44:08]

In theory it doesn't matter for NaCl, since we just kill the process without
calling DidDestroy. But it's safer to not make assumptions about pp::Instance's
lifetime from other threads. pp::Instance's API doesn't really make it possible
for it to be thread-safe, unfortunately...  but it was designed before we had
thread-safe Pepper calls.

Most of the C++ wrappers work fine because the object is really just a handle
that holds a PP_Resource (and addrefs and removerefs the resource at appropriate
times). If the same PP_Resource is used from multiple threads, the best thing
would be for each thread to have its own pp::Foo (since it's just a handle
anyway).

+      0, pp::CompletionCallback(&PostReply, NULL));
+  return NULL;
+}
+
 class TestInstance : public pp::Instance {
  public:
-  explicit TestInstance(PP_Instance instance) : pp::Instance(instance) {}
+  explicit TestInstance(PP_Instance instance) : pp::Instance(instance) {
+    g_instance = this;
+  }
+
   virtual ~TestInstance() {}
   virtual void HandleMessage(const pp::Var& var_message) {
     if (!var_message.is_string()) {
       return;
     }
     if (var_message.AsString() != "hello") {
       return;
     }
-    pp::VarArray reply = pp::VarArray();
-    for (size_t i = 0; i < result.size(); ++i) {
-      reply.Set(i, pp::Var(result[i]));
-    }
-    PostMessage(reply);
+
+    // We test the manifest routines again after PPAPI has initialized to
+    // ensure that they still work.
+    //
+    // irt_open_resource() isn't allowed to be called on the main thread once
+    // pepper starts, so these tests must happen on a background thread.
+    pthread_create(&thread_, NULL, &RunTestsOnBackgroundThread, NULL);

[Mark Seaborn, 2014/09/04 17:41:36]

Nit: can you check the return value?

Nit: thread_ could be a local var instead -- doesn't need to be a member.

[teravest, 2014/09/04 20:21:28]

Sure, do you want me to write a change to fix up these nits?

[Mark Seaborn, 2014/09/04 22:00:41]

It's your call -- I'll leave it up to you.

   }
+ private:
+  pthread_t thread_;
 };
 
 class TestModule : public pp::Module {
  public:
   TestModule() : pp::Module() {}
   virtual ~TestModule() {}
 
   virtual pp::Instance* CreateInstance(PP_Instance instance) {
     return new TestInstance(instance);
   }
 };
 
 namespace pp {
 Module* CreateModule() {
   return new TestModule();
 }
 }
 
 int main() {
-  result.push_back(LoadManifestSuccess(&__nacl_irt_query));
-  result.push_back(LoadManifestNonExistentEntry(&__nacl_irt_query));
-  result.push_back(LoadManifestNonExistentFile(&__nacl_irt_query));
+  RunTests();
   return PpapiPluginMain();
 }