Bug 1366808: Properly detect buildID mismatches between parent and child processes and display about:restartrequired to prompt the user to restart Firefox before proceeding. r=jimm,felipe,bz

browser/base/content/tabbrowser.js

@@ -4111,7 +4111,7 @@ window._gBrowser = {
       let tab = this.getTabForBrowser(browser);
 
       if (this.selectedBrowser == browser) {
-        TabCrashHandler.onSelectedBrowserCrash(browser);
+        TabCrashHandler.onSelectedBrowserCrash(browser, false);
       } else {
         this.updateBrowserRemoteness(browser, false);
         SessionStore.reviveCrashedTab(tab);
@@ -4121,6 +4121,17 @@ window._gBrowser = {
       this.setIcon(tab, icon, browser.contentPrincipal, browser.contentRequestContextID);
     });
 
+    this.addEventListener("oop-browser-buildid-mismatch", (event) => {
+      if (!event.isTrusted)
+        return;
+
+      let browser = event.originalTarget;
+
+      if (this.selectedBrowser == browser) {
+        TabCrashHandler.onSelectedBrowserCrash(browser, true);
+      }
+    });
+
     this.addEventListener("DOMAudioPlaybackStarted", (event) => {
       var tab = this.getTabFromAudioEvent(event);
       if (!tab) {

browser/modules/ContentCrashHandlers.jsm

@@ -223,9 +223,13 @@ var TabCrashHandler = {
 
     let sentBrowser = false;
     for (let weakBrowser of browserQueue) {
-      let browser = weakBrowser.get();
+      let browser = weakBrowser.browser.get();
       if (browser) {
-        this.sendToTabCrashedPage(browser);
+        if (weakBrowser.restartRequired) {
+          this.sendToRestartRequiredPage(browser);
+        } else {
+          this.sendToTabCrashedPage(browser);
+        }
         sentBrowser = true;
       }
     }
@@ -240,8 +244,10 @@ var TabCrashHandler = {
    *
    * @param browser (<xul:browser>)
    *        The selected browser that just crashed.
+   * @param restartRequired (bool)
+   *        Whether or not a browser restart is required to recover.
    */
-  onSelectedBrowserCrash(browser) {
+  onSelectedBrowserCrash(browser, restartRequired) {
     if (!browser.isRemoteBrowser) {
       Cu.reportError("Selected crashed browser is not remote.");
       return;
@@ -263,7 +269,8 @@ var TabCrashHandler = {
     // this queue will be flushed. The weak reference is to avoid
     // leaking browsers in case anything goes wrong during this
     // teardown process.
-    browserQueue.push(Cu.getWeakReference(browser));
+    browserQueue.push({browser: Cu.getWeakReference(browser),
+                       restartRequired});
   },
 
   /**
@@ -306,6 +313,17 @@ var TabCrashHandler = {
     return false;
   },
 
+  sendToRestartRequiredPage(browser) {
+    let uri = browser.currentURI;
+    let gBrowser = browser.ownerGlobal.gBrowser;
+    let tab = gBrowser.getTabForBrowser(browser);
+    // The restart required page is non-remote by default.
+    gBrowser.updateBrowserRemoteness(browser, false);
+
+    browser.docShell.displayLoadError(Cr.NS_ERROR_BUILDID_MISMATCH, uri, null);
+    tab.setAttribute("crashed", true);
+  },
+
   /**
    * We show a special page to users when a normal browser tab has crashed.
    * This method should be called to send a browser to that page once the

docshell/base/nsDocShell.cpp

@@ -4706,6 +4706,16 @@ nsDocShell::DisplayLoadError(nsresult aError, nsIURI* aURI,
       element->GetAttribute(NS_LITERAL_STRING("crashedPageTitle"), messageStr);
     }
 
+    // DisplayLoadError requires a non-empty messageStr to proceed and call
+    // LoadErrorPage. If the page doesn't have a title, we will use a blank
+    // space which will be trimmed and thus treated as empty by the front-end.
+    if (messageStr.IsEmpty()) {
+      messageStr.AssignLiteral(u" ");
+    }
+  } else if (NS_ERROR_BUILDID_MISMATCH == aError) {
+    errorPage.AssignLiteral("restartrequired");
+    error = "restartrequired";
+
     // DisplayLoadError requires a non-empty messageStr to proceed and call
     // LoadErrorPage. If the page doesn't have a title, we will use a blank
     // space which will be trimmed and thus treated as empty by the front-end.

dom/ipc/ContentChild.cpp

@@ -605,6 +605,7 @@ ContentChild::RecvSetXPCOMProcessAttributes(const XPCOMInitData& aXPCOMInit,
 bool
 ContentChild::Init(MessageLoop* aIOLoop,
                    base::ProcessId aParentPid,
+                   const char* aParentBuildID,
                    IPC::Channel* aChannel,
                    uint64_t aChildID,
                    bool aIsForBrowser)
@@ -673,10 +674,15 @@ ContentChild::Init(MessageLoop* aIOLoop,
   GetIPCChannel()->SetChannelFlags(MessageChannel::REQUIRE_A11Y_REENTRY);
 #endif
 
-  // This must be sent before any IPDL message, which may hit sentinel
+  // This must be checked before any IPDL message, which may hit sentinel
   // errors due to parent and content processes having different
   // versions.
-  GetIPCChannel()->SendBuildID();
+  MessageChannel* channel = GetIPCChannel();
+  if (channel && !channel->SendBuildIDsMatchMessage(aParentBuildID)) {
+    // We need to quit this process if the buildID doesn't match the parent's.
+    // This can occur when an update occurred in the background.
+    ProcessChild::QuickExit();
+  }
 
 #ifdef MOZ_X11
   if (!gfxPlatform::IsHeadless()) {

dom/ipc/ContentChild.h

@@ -120,6 +120,7 @@ public:
 
   bool Init(MessageLoop* aIOLoop,
             base::ProcessId aParentPid,
+            const char* aParentBuildID,
             IPC::Channel* aChannel,
             uint64_t aChildID,
             bool aIsForBrowser);

dom/ipc/ContentParent.cpp

@@ -2054,6 +2054,10 @@ ContentParent::LaunchSubprocess(ProcessPriority aInitialPriority /* = PROCESS_PR
     extraArgs.push_back("-safeMode");
   }
 
+  nsCString parentBuildID(mozilla::PlatformBuildID());
+  extraArgs.push_back("-parentBuildID");
+  extraArgs.push_back(parentBuildID.get());
+
   SetOtherProcessId(kInvalidProcessId, ProcessIdState::ePending);
   if (!mSubprocess->Launch(extraArgs)) {
     NS_ERROR("failed to launch child in the parent");

dom/ipc/ContentProcess.cpp

@@ -99,6 +99,7 @@ ContentProcess::Init(int aArgc, char* aArgv[])
   Maybe<base::SharedMemoryHandle> prefsHandle;
   Maybe<size_t> prefsLen;
   Maybe<const char*> schedulerPrefs;
+  Maybe<const char*> parentBuildID;
 #if defined(XP_MACOSX) && defined(MOZ_CONTENT_SANDBOX)
   nsCOMPtr<nsIFile> profileDir;
 #endif
@@ -168,6 +169,12 @@ ContentProcess::Init(int aArgc, char* aArgv[])
     } else if (strcmp(aArgv[i], "-safeMode") == 0) {
       gSafeMode = true;
 
+    } else if (strcmp(aArgv[i], "-parentBuildID") == 0) {
+      if (++i == aArgc) {
+        return false;
+      }
+      parentBuildID = Some(aArgv[i]);
+
 #if defined(XP_MACOSX) && defined(MOZ_CONTENT_SANDBOX)
     } else if (strcmp(aArgv[i], "-profile") == 0) {
       if (++i == aArgc) {
@@ -197,7 +204,8 @@ ContentProcess::Init(int aArgc, char* aArgv[])
       isForBrowser.isNothing() ||
       prefsHandle.isNothing() ||
       prefsLen.isNothing() ||
-      schedulerPrefs.isNothing()) {
+      schedulerPrefs.isNothing() ||
+      parentBuildID.isNothing()) {
     return false;
   }
 
@@ -217,6 +225,7 @@ ContentProcess::Init(int aArgc, char* aArgv[])
   Scheduler::SetPrefs(*schedulerPrefs);
   mContent.Init(IOThreadChild::message_loop(),
                 ParentPid(),
+                *parentBuildID,
                 IOThreadChild::channel(),
                 *childID,
                 *isForBrowser);

dom/ipc/TabParent.cpp

@@ -466,10 +466,16 @@ TabParent::ActorDestroy(ActorDestroyReason why)
         // and created a new frameloader. If so, we don't fire the event,
         // since the frameloader owner has clearly moved on.
         if (currentFrameLoader == frameLoader) {
-          nsContentUtils::DispatchTrustedEvent(frameElement->OwnerDoc(), frameElement,
-                                               NS_LITERAL_STRING("oop-browser-crashed"),
-                                               true, true);
-
+          MessageChannel* channel = GetIPCChannel();
+          if (channel && !channel->DoBuildIDsMatch()) {
+            nsContentUtils::DispatchTrustedEvent(
+              frameElement->OwnerDoc(), frameElement,
+              NS_LITERAL_STRING("oop-browser-buildid-mismatch"), true, true);
+          } else {
+            nsContentUtils::DispatchTrustedEvent(
+              frameElement->OwnerDoc(), frameElement,
+              NS_LITERAL_STRING("oop-browser-crashed"), true, true);
+          }
         }
       }
     }

gfx/ipc/GPUParent.cpp

@@ -83,6 +83,7 @@ GPUParent::GetSingleton()
 
 bool
 GPUParent::Init(base::ProcessId aParentPid,
+                const char* aParentBuildID,
                 MessageLoop* aIOLoop,
                 IPC::Channel* aChannel)
 {
@@ -99,10 +100,15 @@ GPUParent::Init(base::ProcessId aParentPid,
 
   nsDebugImpl::SetMultiprocessMode("GPU");
 
-  // This must be sent before any IPDL message, which may hit sentinel
+  // This must be checked before any IPDL message, which may hit sentinel
   // errors due to parent and content processes having different
   // versions.
-  GetIPCChannel()->SendBuildID();
+  MessageChannel* channel = GetIPCChannel();
+  if (channel && !channel->SendBuildIDsMatchMessage(aParentBuildID)) {
+    // We need to quit this process if the buildID doesn't match the parent's.
+    // This can occur when an update occurred in the background.
+    ProcessChild::QuickExit();
+  }
 
   // Init crash reporter support.
   CrashReporterClient::InitSingleton(this);

gfx/ipc/GPUParent.h

@@ -27,6 +27,7 @@ public:
   static GPUParent* GetSingleton();
 
   bool Init(base::ProcessId aParentPid,
+            const char* aParentBuildID,
             MessageLoop* aIOLoop,
             IPC::Channel* aChannel);
   void NotifyDeviceReset();

gfx/ipc/GPUProcessImpl.cpp

@@ -31,8 +31,15 @@ GPUProcessImpl::Init(int aArgc, char* aArgv[])
 #if defined(MOZ_SANDBOX) && defined(OS_WIN)
   mozilla::SandboxTarget::Instance()->StartSandbox();
 #endif
+  char* parentBuildID = nullptr;
+  for (int idx = aArgc; idx > 0; idx--) {
+    if (!strcmp(aArgv[idx], "-parentBuildID")) {
+      parentBuildID = aArgv[idx + 1];
+    }
+  }
 
   return mGPU.Init(ParentPid(),
+                   parentBuildID,
                    IOThreadChild::message_loop(),
                    IOThreadChild::channel());
 }

ipc/glue/MessageChannel.cpp

@@ -534,7 +534,8 @@ MessageChannel::MessageChannel(const char* aName,
     mPeerPidSet(false),
     mPeerPid(-1),
     mIsPostponingSends(false),
-    mInKillHardShutdown(false)
+    mInKillHardShutdown(false),
+    mBuildIDsConfirmedMatch(false)
 {
     MOZ_COUNT_CTOR(ipc::MessageChannel);
 
@@ -1003,29 +1004,38 @@ MessageChannel::RejectPendingResponsesForActor(ActorIdType aActorId)
   }
 }
 
-class BuildIDMessage : public IPC::Message
+class BuildIDsMatchMessage : public IPC::Message
 {
 public:
-    BuildIDMessage()
-        : IPC::Message(MSG_ROUTING_NONE, BUILD_ID_MESSAGE_TYPE)
+    BuildIDsMatchMessage()
+        : IPC::Message(MSG_ROUTING_NONE, BUILD_IDS_MATCH_MESSAGE_TYPE)
     {
     }
     void Log(const std::string& aPrefix, FILE* aOutf) const
     {
-        fputs("(special `Build ID' message)", aOutf);
+        fputs("(special `Build IDs match' message)", aOutf);
     }
 };
 
-// Send the parent a special async message to allow it to detect if
-// this process is running a different build. This is a minor
-// variation on MessageChannel::Send(Message* aMsg).
-void
-MessageChannel::SendBuildID()
+// Send the parent a special async message to confirm when the parent and child
+// are of the same buildID. Skips sending the message and returns false if the
+// buildIDs don't match. This is a minor variation on
+// MessageChannel::Send(Message* aMsg).
+bool
+MessageChannel::SendBuildIDsMatchMessage(const char* aParentBuildID)
 {
     MOZ_ASSERT(!XRE_IsParentProcess());
-    nsAutoPtr<BuildIDMessage> msg(new BuildIDMessage());
-    nsCString buildID(mozilla::PlatformBuildID());
-    IPC::WriteParam(msg, buildID);
+
+    nsCString parentBuildID(aParentBuildID);
+    nsCString childBuildID(mozilla::PlatformBuildID());
+
+    if (parentBuildID != childBuildID) {
+        // The build IDs didn't match, usually because an update occurred in the
+        // background.
+        return false;
+    }
+
+    nsAutoPtr<BuildIDsMatchMessage> msg(new BuildIDsMatchMessage());
 
     MOZ_RELEASE_ASSERT(!msg->is_sync());
     MOZ_RELEASE_ASSERT(msg->nested_level() != IPC::Message::NESTED_INSIDE_SYNC);
@@ -1037,9 +1047,10 @@ MessageChannel::SendBuildID()
     MonitorAutoLock lock(*mMonitor);
     if (!Connected()) {
         ReportConnectionError("MessageChannel", msg);
-        return;
+        return false;
     }
     mLink->SendMessage(msg.forget());
+    return true;
 }
 
 class CancelMessage : public IPC::Message
@@ -1058,22 +1069,6 @@ public:
     }
 };
 
-MOZ_NEVER_INLINE static void
-CheckChildProcessBuildID(const IPC::Message& aMsg)
-{
-    MOZ_ASSERT(XRE_IsParentProcess());
-    nsCString childBuildID;
-    PickleIterator msgIter(aMsg);
-    MOZ_ALWAYS_TRUE(IPC::ReadParam(&aMsg, &msgIter, &childBuildID));
-    aMsg.EndRead(msgIter);
-
-    nsCString parentBuildID(mozilla::PlatformBuildID());
-
-    // This assert can fail if the child process has been updated
-    // to a newer version while the parent process was running.
-    MOZ_RELEASE_ASSERT(parentBuildID == childBuildID);
-}
-
 bool
 MessageChannel::MaybeInterceptSpecialIOMessage(const Message& aMsg)
 {
@@ -1095,9 +1090,9 @@ MessageChannel::MaybeInterceptSpecialIOMessage(const Message& aMsg)
             CancelTransaction(aMsg.transaction_id());
             NotifyWorkerThread();
             return true;
-        } else if (BUILD_ID_MESSAGE_TYPE == aMsg.type()) {
-            IPC_LOG("Build ID message");
-            CheckChildProcessBuildID(aMsg);
+        } else if (BUILD_IDS_MATCH_MESSAGE_TYPE == aMsg.type()) {
+            IPC_LOG("Build IDs match message");
+            mBuildIDsConfirmedMatch = true;
             return true;
         }
     }

ipc/glue/MessageChannel.h

@@ -238,7 +238,8 @@ private:
         gUnresolvedResponses++;
     }
 
-    void SendBuildID();
+    bool SendBuildIDsMatchMessage(const char* aParentBuildI);
+    bool DoBuildIDsMatch() { return mBuildIDsConfirmedMatch; }
 
     // Asynchronously deliver a message back to this side of the
     // channel
@@ -860,6 +861,8 @@ private:
     std::vector<UniquePtr<Message>> mPostponedSends;
 
     bool mInKillHardShutdown;
+
+    bool mBuildIDsConfirmedMatch;
 };
 
 void

ipc/glue/ProtocolUtils.h

@@ -50,12 +50,13 @@ namespace {
 // protocol 0.  Oops!  We can get away with this until protocol 0
 // starts approaching its 65,536th message.
 enum {
-    BUILD_ID_MESSAGE_TYPE = kuint16max - 7,
-    CHANNEL_OPENED_MESSAGE_TYPE = kuint16max - 6,
-    SHMEM_DESTROYED_MESSAGE_TYPE = kuint16max - 5,
-    SHMEM_CREATED_MESSAGE_TYPE = kuint16max - 4,
-    GOODBYE_MESSAGE_TYPE       = kuint16max - 3,
-    CANCEL_MESSAGE_TYPE        = kuint16max - 2,
+    BUILD_IDS_MATCH_MESSAGE_TYPE   = kuint16max - 8,
+    BUILD_ID_MESSAGE_TYPE          = kuint16max - 7, // unused
+    CHANNEL_OPENED_MESSAGE_TYPE    = kuint16max - 6,
+    SHMEM_DESTROYED_MESSAGE_TYPE   = kuint16max - 5,
+    SHMEM_CREATED_MESSAGE_TYPE     = kuint16max - 4,
+    GOODBYE_MESSAGE_TYPE           = kuint16max - 3,
+    CANCEL_MESSAGE_TYPE            = kuint16max - 2,
 
     // kuint16max - 1 is used by ipc_channel.h.
 };

js/xpconnect/src/xpc.msg

@@ -220,6 +220,7 @@ XPC_MSG_DEF(NS_ERROR_GFX_PRINTER_DOC_IS_BUSY          , "Cannot print this docum
 
 /* Codes related to content */
 XPC_MSG_DEF(NS_ERROR_CONTENT_CRASHED                  , "The process that hosted this content has crashed.")
+XPC_MSG_DEF(NS_ERROR_BUILDID_MISMATCH                 , "The process that hosted this content did not have the same buildID as the parent.")
 
 /* Codes for the JS-implemented Push DOM API. These can be removed as part of bug 1252660. */
 XPC_MSG_DEF(NS_ERROR_DOM_PUSH_INVALID_KEY_ERR         , "Invalid raw ECDSA P-256 public key.")

xpcom/base/ErrorList.py

@@ -863,6 +863,9 @@ with modules["CONTENT"]:
     errors["NS_ERROR_XBL_BLOCKED"] = FAILURE(15)
     # Error code for when the content process crashed
     errors["NS_ERROR_CONTENT_CRASHED"] = FAILURE(16)
+    # Error code for when the content process had a different buildID than the
+    # parent
+    errors["NS_ERROR_BUILDID_MISMATCH"] = FAILURE(17)
 
     # XXX this is not really used
     errors["NS_HTML_STYLE_PROPERTY_NOT_THERE"] = SUCCESS(2)