PNaCl on-demand installs: Make a separate async IPC to check if PNaCl is installed

chrome/renderer/pepper/pnacl_translation_resource_host.cc

Index: chrome/renderer/pepper/pnacl_translation_resource_host.cc
diff --git a/chrome/renderer/pepper/pnacl_translation_resource_host.cc b/chrome/renderer/pepper/pnacl_translation_resource_host.cc
index 463d186703a00dc68202f796942718555ba71160..980db66b8f3c65be1a3d807a6e101022e0c39f4b 100644
--- a/chrome/renderer/pepper/pnacl_translation_resource_host.cc
+++ b/chrome/renderer/pepper/pnacl_translation_resource_host.cc
@@ -26,6 +26,7 @@ PnaclTranslationResourceHost::PnaclTranslationResourceHost(
 
 PnaclTranslationResourceHost::~PnaclTranslationResourceHost() {
   CleanupCacheRequests();
+  CleanupEnsurePnaclRequests();
 }
 
 void PnaclTranslationResourceHost::OnFilterAdded(IPC::Channel* channel) {
@@ -49,6 +50,10 @@ bool PnaclTranslationResourceHost::OnMessageReceived(
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(PnaclTranslationResourceHost, message)
     IPC_MESSAGE_HANDLER(NaClViewMsg_NexeTempFileReply, OnNexeTempFileReply)
+    IPC_MESSAGE_HANDLER(NaClViewMsg_EnsurePnaclInstalledReply,
+                        OnEnsurePnaclInstalledReply)
+    IPC_MESSAGE_HANDLER(NaClViewMsg_EnsurePnaclInstalledProgress,
+                        OnEnsurePnaclInstalledProgress)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -144,4 +149,66 @@ void PnaclTranslationResourceHost::CleanupCacheRequests() {
   pending_cache_requests_.clear();
 }
 
+
+
+void PnaclTranslationResourceHost::EnsurePnaclInstalled(
+    PP_Instance instance,
+    scoped_refptr<TrackedCallback> callback) {
+  if (!io_message_loop_->BelongsToCurrentThread()) {

[dmichael (off chromium), 2013/08/01 17:33:49]

EnsurePnaclInstalled should always originate on the main renderer thread. I'd
rather you separate this in to two methods, the outer one that runs only on the
main thread and the internal one that only runs on the IO thread. Then you can
DCHECK your thread. That will make it more obvious what's going on and what your
thread expectations are.

[jvoung (off chromium), 2013/08/01 23:14:07]

Separated the two halfs of the function and put in DCHECKS.

+    io_message_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&PnaclTranslationResourceHost::EnsurePnaclInstalled,
+                   this, instance, callback));
+    return;
+  }
+  if (!channel_ || !channel_->Send(new NaClHostMsg_EnsurePnaclInstalled(
+          instance))) {
+    PpapiGlobals::Get()->GetMainThreadMessageLoop()->PostTask(
+        FROM_HERE,
+        base::Bind(&TrackedCallback::Run,
+                   callback,
+                   static_cast<int32_t>(PP_ERROR_FAILED)));
+    return;
+  }
+  pending_ensure_pnacl_requests_.insert(
+      std::make_pair(instance, callback));
+}
+
+void PnaclTranslationResourceHost::OnEnsurePnaclInstalledReply(
+    PP_Instance instance,
+    bool success) {
+  EnsurePnaclInstalledMap::iterator it =
+      pending_ensure_pnacl_requests_.find(instance);
+  if (it == pending_ensure_pnacl_requests_.end()) {
+    DLOG(ERROR) << "Could not find pending request for reply";
+  } else {
+    if (TrackedCallback::IsPending(it->second)) {
+      PpapiGlobals::Get()->GetMainThreadMessageLoop()->PostTask(
+          FROM_HERE,
+          base::Bind(&TrackedCallback::Run,
+                     it->second,
+                     static_cast<int32_t>(success ? PP_OK : PP_ERROR_FAILED)));
+    }
+    pending_ensure_pnacl_requests_.erase(it);
+  }
+}
+
+void PnaclTranslationResourceHost::OnEnsurePnaclInstalledProgress(
+    PP_Instance instance,
+    int64 current_progress,
+    int64 total) {
+  // TODO(jvoung): Issue a progress event w/ dmichael's new DispatchEvent
+  // interface: https://codereview.chromium.org/14588009/
+}
+
+void PnaclTranslationResourceHost::CleanupEnsurePnaclRequests() {
+  for (EnsurePnaclInstalledMap::iterator it =
+           pending_ensure_pnacl_requests_.begin();
+       it != pending_ensure_pnacl_requests_.end();
+       ++it) {
+    it->second->PostAbort();
+  }
+  pending_ensure_pnacl_requests_.clear();

[dmichael (off chromium), 2013/08/01 17:33:49]

So do CleanupEnsurePnaclRequests and the destructor always run on the IO thread?
Might be worth a DCHECK. (You might also have a look at ThreadChecker to make
some of the thread checking clear)

[jvoung (off chromium), 2013/08/01 23:14:07]

Yeah, it also has to run on the IO thread. Added DCHECK.

I tried ThreadChecker, but I think the constructor doesn't run on the IO thread,
so it didn't work?

[dmichael (off chromium), 2013/08/02 17:24:03]

FWIW, you can call DetachFromThread in that case.

[jvoung (off chromium), 2013/08/02 18:32:36]

Ah ok.  Any preference for using that or explicitly checking the IO thread?

+}
+
 #endif  // DISABLE_NACL