Bug 1432409 part 1 - Prevent nsDeviceContextSpecProxy using RemotePrintJobChild if initialization fails. r=dholbert, a=lizzard
authorJonathan Watt <jwatt@jwatt.org>
Fri, 16 Feb 2018 14:53:26 +0000
changeset 452479 9002f06257c6ca8f58ec761f65a6ea0d90a35e25
parent 452478 04ec73c33ff12871343c7fe5dad3cc58c428a30a
child 452480 0788981ad3266054b86bd77ef23ad8c8ec58825b
push id8740
push userjwatt@jwatt.org
push dateTue, 20 Feb 2018 23:40:37 +0000
treeherdermozilla-beta@0788981ad326 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, lizzard
bugs1432409
milestone59.0
Bug 1432409 part 1 - Prevent nsDeviceContextSpecProxy using RemotePrintJobChild if initialization fails. r=dholbert, a=lizzard When RemotePrintJobChild::InitializePrint sends a message to the parent process to ask it to initialize printing it spins the event loop and waits for a reply. If the parent fails to initialize printing it will send back an error message followed immediately by a second message telling the child process to delete its RemotePrintJobChild. The error message causes the nested event loop to terminate and blocks RemotePrintJobChild::InitializePrint. We then do various async things to clean up, some of which can try to post messages to the parent process's RemotePrintJobParent. This is a problem since the delete message is pending in the child process's event loop resulting in a race between the code that wants to use the RemotePrintJobChild to send a message to the parent process, and the delete event that will make us crash if anyone tries to use the RemotePrintJobChild. This patch makes sure that if nsDeviceContextSpecProxy's BeginDocument returns failure (remote print job initialization failed) that its EndDocument and AbortDocument methods are then no-ops and will not try to use its RemotePrintJobChild. (BeginPage and EndPage are not changed since they are not called if BeginDocument returns an error result.) MozReview-Commit-ID: 2H6GHjngX7R
widget/nsDeviceContextSpecProxy.cpp
--- a/widget/nsDeviceContextSpecProxy.cpp
+++ b/widget/nsDeviceContextSpecProxy.cpp
@@ -134,32 +134,44 @@ nsDeviceContextSpecProxy::GetPrintingSca
 }
 
 NS_IMETHODIMP
 nsDeviceContextSpecProxy::BeginDocument(const nsAString& aTitle,
                                         const nsAString& aPrintToFileName,
                                         int32_t aStartPage, int32_t aEndPage)
 {
   mRecorder = new mozilla::layout::DrawEventRecorderPRFileDesc();
-  return mRemotePrintJob->InitializePrint(nsString(aTitle),
-                                          nsString(aPrintToFileName),
-                                          aStartPage, aEndPage);
+  nsresult rv = mRemotePrintJob->InitializePrint(nsString(aTitle),
+                                                 nsString(aPrintToFileName),
+                                                 aStartPage, aEndPage);
+  if (NS_FAILED(rv)) {
+    // The parent process will send a 'delete' message to tell this process to
+    // delete our RemotePrintJobChild.  As soon as we return to the event loop
+    // and evaluate that message we will crash if we try to access
+    // mRemotePrintJob.  We must not try to use it again.
+    mRemotePrintJob = nullptr;
+  }
+  return rv;
 }
 
 NS_IMETHODIMP
 nsDeviceContextSpecProxy::EndDocument()
 {
-  Unused << mRemotePrintJob->SendFinalizePrint();
+  if (mRemotePrintJob) {
+    Unused << mRemotePrintJob->SendFinalizePrint();
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDeviceContextSpecProxy::AbortDocument()
 {
-  Unused << mRemotePrintJob->SendAbortPrint(NS_OK);
+  if (mRemotePrintJob) {
+    Unused << mRemotePrintJob->SendAbortPrint(NS_OK);
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDeviceContextSpecProxy::BeginPage()
 {
   mRecorder->OpenFD(mRemotePrintJob->GetNextPageFD());