Bug 957883: Handle channel errors during process launch such that we don't sit on a dead channel for the full time-out (e.g. 45 secs for NPAPI), and allow us to detect when an error happens during child process init. Also, now that it's possible, actually check for an error during NPAPI child process init. r=bsmedberg
authorJosh Aas <joshmoz@gmail.com>
Sat, 11 Jan 2014 21:51:00 -0600
changeset 163035 2b1118ec0cc1268b41338c6e6ce9fab4b55f72df
parent 163034 9539fbbb66ec40667aa9e541e582c390c93f0a5e
child 163036 3b1616997a89d23a2cf7fc8e359055225f74ed41
push id25979
push usercbook@mozilla.com
push dateMon, 13 Jan 2014 11:46:02 +0000
treeherdermozilla-central@ea6657f1d682 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg
bugs957883
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 957883: Handle channel errors during process launch such that we don't sit on a dead channel for the full time-out (e.g. 45 secs for NPAPI), and allow us to detect when an error happens during child process init. Also, now that it's possible, actually check for an error during NPAPI child process init. r=bsmedberg
dom/plugins/ipc/PluginProcessChild.cpp
dom/plugins/ipc/PluginProcessParent.h
ipc/glue/GeckoChildProcessHost.cpp
--- a/dom/plugins/ipc/PluginProcessChild.cpp
+++ b/dom/plugins/ipc/PluginProcessChild.cpp
@@ -114,21 +114,19 @@ PluginProcessChild::Init()
 #  error Sorry
 #endif
 
     if (NS_FAILED(nsRegion::InitStatic())) {
       NS_ERROR("Could not initialize nsRegion");
       return false;
     }
 
-    mPlugin.Init(pluginFilename, ParentHandle(),
-                 IOThreadChild::message_loop(),
-                 IOThreadChild::channel());
-
-    return true;
+    return mPlugin.Init(pluginFilename, ParentHandle(),
+                        IOThreadChild::message_loop(),
+                        IOThreadChild::channel());
 }
 
 void
 PluginProcessChild::CleanUp()
 {
 #ifdef XP_WIN
     ::OleUninitialize();
 #endif
--- a/dom/plugins/ipc/PluginProcessParent.h
+++ b/dom/plugins/ipc/PluginProcessParent.h
@@ -40,17 +40,16 @@ public:
     {
         return true;
     }
 
     const std::string& GetPluginFilePath() { return mPluginFilePath; }
 
     using mozilla::ipc::GeckoChildProcessHost::GetShutDownEvent;
     using mozilla::ipc::GeckoChildProcessHost::GetChannel;
-    using mozilla::ipc::GeckoChildProcessHost::GetChildProcessHandle;
 
 private:
     std::string mPluginFilePath;
 
     DISALLOW_EVIL_CONSTRUCTORS(PluginProcessParent);
 };
 
 
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -296,17 +296,22 @@ GeckoChildProcessHost::SyncLaunch(std::v
   // NB: this uses a different mechanism than the chromium parent
   // class.
   MonitorAutoLock lock(mMonitor);
   PRIntervalTime waitStart = PR_IntervalNow();
   PRIntervalTime current;
 
   // We'll receive several notifications, we need to exit when we
   // have either successfully launched or have timed out.
-  while (mProcessState < PROCESS_CONNECTED) {
+  while (mProcessState != PROCESS_CONNECTED) {
+    // If there was an error then return it, don't wait out the timeout.
+    if (mProcessState == PROCESS_ERROR) {
+      break;
+    }
+
     lock.Wait(timeoutTicks);
 
     if (timeoutTicks != PR_INTERVAL_NO_TIMEOUT) {
       current = PR_IntervalNow();
       PRIntervalTime elapsed = current - waitStart;
       if (elapsed > timeoutTicks) {
         break;
       }
@@ -838,16 +843,25 @@ GeckoChildProcessHost::OnMessageReceived
   // We never process messages ourself, just save them up for the next
   // listener.
   mQueue.push(aMsg);
 }
 
 void
 GeckoChildProcessHost::OnChannelError()
 {
+  // Update the process state to an error state if we have a channel
+  // error before we're connected. This fixes certain failures,
+  // but does not address the full range of possible issues described
+  // in the FIXME comment below.
+  MonitorAutoLock lock(mMonitor);
+  if (mProcessState < PROCESS_CONNECTED) {
+    mProcessState = PROCESS_ERROR;
+    lock.Notify();
+  }
   // FIXME/bug 773925: save up this error for the next listener.
 }
 
 void
 GeckoChildProcessHost::GetQueuedMessages(std::queue<IPC::Message>& queue)
 {
   // If this is called off the IO thread, bad things will happen.
   DCHECK(MessageLoopForIO::current());