Fix race condition on ManifestService initialization.

ppapi/nacl_irt/manifest_service.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.
 
 #include "ppapi/nacl_irt/manifest_service.h"
 
 #include "base/message_loop/message_loop_proxy.h"
 #include "ipc/ipc_channel_handle.h"
 #include "ipc/ipc_channel_proxy.h"
 #include "ipc/ipc_sync_message_filter.h"
 #include "native_client/src/trusted/service_runtime/include/sys/errno.h"
 #include "ppapi/nacl_irt/irt_manifest.h"
 #include "ppapi/nacl_irt/plugin_startup.h"
 #include "ppapi/proxy/ppapi_messages.h"
 
 namespace ppapi {
 
 const char kFilePrefix[] = "files/";
 
+// IPC channel is asynchronously set up. So, the plugin may be try to send

[Mark Seaborn, 2014/06/13 00:34:35]

Nit: "plugin" is ambiguous.  Maybe say "NaCl process"?

[hidehiko, 2014/06/13 04:37:10]

Done.

+// a OpenResource message to the host before the connection is established.
+// In such a case, it is necessary to wait for the set up completion.
+class ManifestMessageFilter : public IPC::SyncMessageFilter {
+ public:
+  ManifestMessageFilter(base::WaitableEvent* shutdown_event)
+      : SyncMessageFilter(shutdown_event),
+        connected_event_(true, false) {  // Reset manually, and init by false.

[Mark Seaborn, 2014/06/13 00:34:35]

Nit: "init with" rather than "by"?

But if this is intended to comment the arguments, maybe it would be clearer to
write something like this:

connected_event_(/* manual_reset= */ true, /* initially_signaled= */ false);

[hidehiko, 2014/06/13 04:37:10]

Changed the style to one which seems to be used in chrome repository (by grep).

+  }
+
+  virtual bool Send(IPC::Message* message) OVERRIDE {
+    // Wait until set up is actually done.
+    connected_event_.Wait();
+    return SyncMessageFilter::Send(message);
+  }
+
+  // When set up is done, OnFilterAdded is called on IO thread. Unblocks the
+  // Send().
+  virtual void OnFilterAdded(IPC::Channel* channel) OVERRIDE {
+    SyncMessageFilter::OnFilterAdded(channel);
+    connected_event_.Signal();
+  }
+
+  // If an error is found, unblocks the Send(), too, to return an error.
+  virtual void OnChannelError() OVERRIDE {
+    SyncMessageFilter::OnChannelError();
+    connected_event_.Signal();
+  }
+
+  // Similar to OnChannelError, unblocks the Send() on the channel closing.
+  virtual void OnChannelClosing() OVERRIDE {
+    SyncMessageFilter::OnChannelClosing();
+    connected_event_.Signal();
+  }
+
+ private:
+  base::WaitableEvent connected_event_;
+
+  DISALLOW_COPY_AND_ASSIGN(ManifestMessageFilter);
+};
+
 ManifestService::ManifestService(
     const IPC::ChannelHandle& handle,
     scoped_refptr<base::MessageLoopProxy> io_message_loop,
     base::WaitableEvent* shutdown_event) {
-  filter_ = new IPC::SyncMessageFilter(shutdown_event);
+  filter_ = new ManifestMessageFilter(shutdown_event);
   channel_ = IPC::ChannelProxy::Create(handle,
                                        IPC::Channel::MODE_SERVER,
                                        NULL,  // Listener
                                        io_message_loop);
   channel_->AddFilter(filter_.get());
 }
 
 ManifestService::~ManifestService() {
 }
 
@@ skipping 25 matching lines @@
   ManifestService* manifest_service = GetManifestService();
   if (manifest_service == NULL ||
       !manifest_service->OpenResource(file, fd)) {
     return NACL_ABI_EIO;
   }
 
   return (*fd == -1) ? NACL_ABI_ENOENT : 0;
 }
 
 }  // namespace ppapi