Fix race condition on ManifestService initialization.

SyncMessageFilter::Send() returns false immediately, if the IPC connection
is not yet established. As connecting is done asynchronously, there is
no guarantee that the connection is established on the first Send() invocation.
By this CL, Send() blocks the caller thread if the connection is not yet
established.
Note that currently the ratio should be probably low, because there are some
more initialization steps between the ManifestService creation and the first
Send() invocation. We're switching to changing the initialization procedure,
and then this race would be hit more easily.

TEST=Ran browser_tests --gtest_filter=*NonSfi*:*NonSFI* locally, and trybots. Also, locally modified the code to delay OnFilterAdded with and without this CL, and made sure this CL works well.
BUG=333950
CQ_EXTRA_TRYBOTS=tryserver.chromium:linux_rel_precise32

Review URL: https://codereview.chromium.org/334593004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@277840 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 44 insertions, 1 deletions

diff --git a/ppapi/nacl_irt/manifest_service.cc b/ppapi/nacl_irt/manifest_service.cc
index 85b7fae..441c401 100644
--- a/ppapi/nacl_irt/manifest_service.cc
+++ b/ppapi/nacl_irt/manifest_service.cc
@@ -17,11 +17,54 @@ namespace ppapi {
 
 const char kFilePrefix[] = "files/";
 
+// IPC channel is asynchronously set up. So, the NaCl process may try to
+// send 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 /* manual_reset */, false /* initially_signaled */) {
+  }
+
+  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::Sender* sender) OVERRIDE {
+    SyncMessageFilter::OnFilterAdded(sender);
+    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