Bug 1090439 - PPrinting calls from child to parent via ShowProgress and ShowPrintDialog should not be sync. r=smaug.

embedding/components/printingui/ipc/PPrintSettingsDialog.ipdl

@@ -0,0 +1,21 @@
+/* -*- Mode: C++; c-basic-offset: 2; indent-tabs-mode: nil; tab-width: 8 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+include PPrintingTypes;
+include protocol PPrinting;
+
+namespace mozilla {
+namespace embedding {
+
+protocol PPrintSettingsDialog
+{
+  manager PPrinting;
+
+child:
+  __delete__(nsresult rv, PrintData data);
+};
+
+} // namespace embedding
+} // namespace mozilla

embedding/components/printingui/ipc/PPrinting.ipdl

@@ -3,92 +3,20 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+include PPrintingTypes;
 include protocol PContent;
 include protocol PBrowser;
 include protocol PPrintProgressDialog;
+include protocol PPrintSettingsDialog;
 
 namespace mozilla {
 namespace embedding {
 
-struct PrintData {
-  int32_t startPageRange;
-  int32_t endPageRange;
-  double edgeTop;
-  double edgeLeft;
-  double edgeBottom;
-  double edgeRight;
-  double marginTop;
-  double marginLeft;
-  double marginBottom;
-  double marginRight;
-  double unwriteableMarginTop;
-  double unwriteableMarginLeft;
-  double unwriteableMarginBottom;
-  double unwriteableMarginRight;
-  double scaling;
-  bool printBGColors;
-  bool printBGImages;
-  short printRange;
-  nsString title;
-  nsString docURL;
-  nsString headerStrLeft;
-  nsString headerStrCenter;
-  nsString headerStrRight;
-  nsString footerStrLeft;
-  nsString footerStrCenter;
-  nsString footerStrRight;
-
-  short  howToEnableFrameUI;
-  bool isCancelled;
-  short printFrameTypeUsage;
-  short  printFrameType;
-  bool printSilent;
-  bool shrinkToFit;
-  bool showPrintProgress;
-
-  nsString paperName;
-  short paperSizeType;
-  short paperData;
-  double paperWidth;
-  double paperHeight;
-  short paperSizeUnit;
-  nsString plexName;
-  nsString colorspace;
-  nsString resolutionName;
-  bool downloadFonts;
-  bool printReversed;
-  bool printInColor;
-  int32_t orientation;
-  nsString printCommand;
-  int32_t numCopies;
-  nsString printerName;
-  bool printToFile;
-  nsString toFileName;
-  short outputFormat;
-  int32_t printPageDelay;
-  int32_t resolution;
-  int32_t duplex;
-  bool isInitializedFromPrinter;
-  bool isInitializedFromPrefs;
-  bool persistMarginBoxSettings;
-
-  /* Windows-specific things */
-  nsString driverName;
-  nsString deviceName;
-  bool isFramesetDocument;
-  bool isFramesetFrameSelected;
-  bool isIFrameSelected;
-  bool isRangeSelection;
-
-  /* TODO: OS X specific things - specifically, an array of names for the
-   * document to be supplied by nsIWebBrowserPrint::enumerateDocumentNames
-   */
-};
-
 sync protocol PPrinting
 {
   manager PContent;
   manages PPrintProgressDialog;
+  manages PPrintSettingsDialog;
 
 parent:
   sync ShowProgress(PBrowser browser,
@@ -97,10 +25,12 @@ parent:
     returns(bool notifyOnOpen,
             bool success);
 
-  sync ShowPrintDialog(PBrowser browser, PrintData settings)
-    returns(PrintData modifiedSettings, bool success);
+  async ShowPrintDialog(PPrintSettingsDialog dialog,
+                        PBrowser browser,
+                        PrintData settings);
 
   PPrintProgressDialog();
+  PPrintSettingsDialog();
 
   sync SavePrintSettings(PrintData settings, bool usePrinterNamePrefix,
                          uint32_t flags)

embedding/components/printingui/ipc/PPrintingTypes.ipdlh

@@ -0,0 +1,85 @@
+/* -*- Mode: C++; c-basic-offset: 2; indent-tabs-mode: nil; tab-width: 8 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+namespace mozilla {
+namespace embedding {
+
+struct PrintData {
+  int32_t startPageRange;
+  int32_t endPageRange;
+  double edgeTop;
+  double edgeLeft;
+  double edgeBottom;
+  double edgeRight;
+  double marginTop;
+  double marginLeft;
+  double marginBottom;
+  double marginRight;
+  double unwriteableMarginTop;
+  double unwriteableMarginLeft;
+  double unwriteableMarginBottom;
+  double unwriteableMarginRight;
+  double scaling;
+  bool printBGColors;
+  bool printBGImages;
+  short printRange;
+  nsString title;
+  nsString docURL;
+  nsString headerStrLeft;
+  nsString headerStrCenter;
+  nsString headerStrRight;
+  nsString footerStrLeft;
+  nsString footerStrCenter;
+  nsString footerStrRight;
+
+  short  howToEnableFrameUI;
+  bool isCancelled;
+  short printFrameTypeUsage;
+  short  printFrameType;
+  bool printSilent;
+  bool shrinkToFit;
+  bool showPrintProgress;
+
+  nsString paperName;
+  short paperSizeType;
+  short paperData;
+  double paperWidth;
+  double paperHeight;
+  short paperSizeUnit;
+  nsString plexName;
+  nsString colorspace;
+  nsString resolutionName;
+  bool downloadFonts;
+  bool printReversed;
+  bool printInColor;
+  int32_t orientation;
+  nsString printCommand;
+  int32_t numCopies;
+  nsString printerName;
+  bool printToFile;
+  nsString toFileName;
+  short outputFormat;
+  int32_t printPageDelay;
+  int32_t resolution;
+  int32_t duplex;
+  bool isInitializedFromPrinter;
+  bool isInitializedFromPrefs;
+  bool persistMarginBoxSettings;
+
+  /* Windows-specific things */
+  nsString driverName;
+  nsString deviceName;
+  bool isFramesetDocument;
+  bool isFramesetFrameSelected;
+  bool isIFrameSelected;
+  bool isRangeSelection;
+
+  /* TODO: OS X specific things - specifically, an array of names for the
+   * document to be supplied by nsIWebBrowserPrint::enumerateDocumentNames
+   */
+};
+
+} // namespace embedding
+} // namespace mozilla

embedding/components/printingui/ipc/PrintSettingsDialogChild.cpp

@@ -0,0 +1,34 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "PrintSettingsDialogChild.h"
+
+using mozilla::unused;
+
+namespace mozilla {
+namespace embedding {
+
+MOZ_IMPLICIT PrintSettingsDialogChild::PrintSettingsDialogChild()
+: mReturned(false)
+{
+  MOZ_COUNT_CTOR(PrintSettingsDialogChild);
+}
+
+MOZ_IMPLICIT PrintSettingsDialogChild::~PrintSettingsDialogChild()
+{
+  MOZ_COUNT_DTOR(PrintSettingsDialogChild);
+}
+
+bool
+PrintSettingsDialogChild::Recv__delete__(const nsresult& aResult,
+                                         const PrintData& aData)
+{
+  mResult = aResult;
+  mData = aData;
+  mReturned = true;
+  return true;
+}
+
+} // namespace embedding
+} // namespace mozilla

embedding/components/printingui/ipc/PrintSettingsDialogChild.h

@@ -0,0 +1,36 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_embedding_PrintSettingsDialogChild_h
+#define mozilla_embedding_PrintSettingsDialogChild_h
+
+#include "mozilla/embedding/PPrintSettingsDialogChild.h"
+namespace mozilla {
+namespace embedding {
+
+class PrintSettingsDialogChild MOZ_FINAL : public PPrintSettingsDialogChild
+{
+  NS_INLINE_DECL_REFCOUNTING(PrintSettingsDialogChild)
+
+public:
+  MOZ_IMPLICIT PrintSettingsDialogChild();
+
+  virtual bool Recv__delete__(const nsresult& aResult,
+                              const PrintData& aData) MOZ_OVERRIDE;
+
+  bool returned() { return mReturned; };
+  nsresult result() { return mResult; };
+  PrintData data() { return mData; };
+
+private:
+  virtual ~PrintSettingsDialogChild();
+  bool mReturned;
+  nsresult mResult;
+  PrintData mData;
+};
+
+} // namespace embedding
+} // namespace mozilla
+
+#endif

embedding/components/printingui/ipc/PrintSettingsDialogParent.cpp

@@ -0,0 +1,28 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "PrintSettingsDialogParent.h"
+
+// C++ file contents
+namespace mozilla {
+namespace embedding {
+
+MOZ_IMPLICIT PrintSettingsDialogParent::PrintSettingsDialogParent()
+{
+  MOZ_COUNT_CTOR(PrintSettingsDialogParent);
+}
+
+MOZ_IMPLICIT PrintSettingsDialogParent::~PrintSettingsDialogParent()
+{
+  MOZ_COUNT_DTOR(PrintSettingsDialogParent);
+}
+
+void
+PrintSettingsDialogParent::ActorDestroy(ActorDestroyReason aWhy)
+{
+}
+
+} // namespace embedding
+} // namespace mozilla
+

embedding/components/printingui/ipc/PrintSettingsDialogParent.h

@@ -0,0 +1,29 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_embedding_PrintSettingsDialogParent_h
+#define mozilla_embedding_PrintSettingsDialogParent_h
+
+#include "mozilla/embedding/PPrintSettingsDialogParent.h"
+
+// Header file contents
+namespace mozilla {
+namespace embedding {
+
+class PrintSettingsDialogParent MOZ_FINAL : public PPrintSettingsDialogParent
+{
+public:
+  virtual void
+  ActorDestroy(ActorDestroyReason aWhy) MOZ_OVERRIDE;
+
+  MOZ_IMPLICIT PrintSettingsDialogParent();
+
+private:
+  virtual ~PrintSettingsDialogParent();
+};
+
+} // namespace embedding
+} // namespace mozilla
+
+#endif

embedding/components/printingui/ipc/PrintingParent.cpp

@@ -18,6 +18,7 @@
 #include "PrintingParent.h"
 #include "PrintDataUtils.h"
 #include "PrintProgressDialogParent.h"
+#include "PrintSettingsDialogParent.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
@@ -65,50 +66,58 @@ PrintingParent::RecvShowProgress(PBrowserParent* parent,
   return true;
 }
 
-bool
-PrintingParent::RecvShowPrintDialog(PBrowserParent* parent,
-                                    const PrintData& data,
-                                    PrintData* retVal,
-                                    bool* success)
+nsresult
+PrintingParent::ShowPrintDialog(PBrowserParent* aParent,
+                                const PrintData& aData,
+                                PrintData* aResult)
 {
-  *success = false;
-
-  nsCOMPtr<nsIDOMWindow> parentWin = DOMWindowFromBrowserParent(parent);
+  nsCOMPtr<nsIDOMWindow> parentWin = DOMWindowFromBrowserParent(aParent);
   if (!parentWin) {
-    return true;
+    return NS_ERROR_FAILURE;
   }
 
   nsCOMPtr<nsIPrintingPromptService> pps(do_GetService("@mozilla.org/embedcomp/printingprompt-service;1"));
   if (!pps) {
-    return true;
+    return NS_ERROR_FAILURE;
   }
 
   // The initSettings we got can be wrapped using
   // PrintDataUtils' MockWebBrowserPrint, which implements enough of
   // nsIWebBrowserPrint to keep the dialogs happy.
-  nsCOMPtr<nsIWebBrowserPrint> wbp = new MockWebBrowserPrint(data);
+  nsCOMPtr<nsIWebBrowserPrint> wbp = new MockWebBrowserPrint(aData);
 
   nsresult rv;
   nsCOMPtr<nsIPrintOptions> po = do_GetService("@mozilla.org/gfx/printsettings-service;1", &rv);
-  NS_ENSURE_SUCCESS(rv, true);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIPrintSettings> settings;
   rv = po->CreatePrintSettings(getter_AddRefs(settings));
-  NS_ENSURE_SUCCESS(rv, true);
+  NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = po->DeserializeToPrintSettings(data, settings);
-  NS_ENSURE_SUCCESS(rv, true);
+  rv = po->DeserializeToPrintSettings(aData, settings);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   rv = pps->ShowPrintDialog(parentWin, wbp, settings);
-  NS_ENSURE_SUCCESS(rv, true);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   // And send it back.
-  PrintData result;
-  rv = po->SerializeToPrintData(settings, nullptr, &result);
-  NS_ENSURE_SUCCESS(rv, true);
+  rv = po->SerializeToPrintData(settings, nullptr, aResult);
+  return rv;
+}
 
-  *retVal = result;
-  *success = true;
+bool
+PrintingParent::RecvShowPrintDialog(PPrintSettingsDialogParent* aDialog,
+                                    PBrowserParent* aParent,
+                                    const PrintData& aData)
+{
+  PrintData resultData;
+  nsresult rv = ShowPrintDialog(aParent, aData, &resultData);
+
+  // The child has been spinning an event loop while waiting
+  // to hear about the print settings. We return the results
+  // with an async message which frees the child process from
+  // its nested event loop.
+  mozilla::unused << aDialog->Send__delete__(aDialog, rv, resultData);
   return true;
 }
 
@@ -157,6 +166,19 @@ PrintingParent::DeallocPPrintProgressDialogParent(PPrintProgressDialogParent* do
   return true;
 }
 
+PPrintSettingsDialogParent*
+PrintingParent::AllocPPrintSettingsDialogParent()
+{
+  return new PrintSettingsDialogParent();
+}
+
+bool
+PrintingParent::DeallocPPrintSettingsDialogParent(PPrintSettingsDialogParent* aDoomed)
+{
+  delete aDoomed;
+  return true;
+}
+
 void
 PrintingParent::ActorDestroy(ActorDestroyReason aWhy)
 {

embedding/components/printingui/ipc/PrintingParent.h

@@ -9,9 +9,10 @@
 
 #include "mozilla/dom/PBrowserParent.h"
 #include "mozilla/embedding/PPrintingParent.h"
-#include "mozilla/embedding/PPrintProgressDialogParent.h"
 
 class nsIDOMWindow;
+class PPrintProgressDialogParent;
+class PPrintSettingsDialogParent;
 
 namespace mozilla {
 namespace embedding {
@@ -26,16 +27,15 @@ public:
                      bool* notifyOnOpen,
                      bool* success);
     virtual bool
-    RecvShowPrintDialog(PBrowserParent* parent,
-                        const PrintData& initSettings,
-                        PrintData* retVal,
-                        bool* success);
+    RecvShowPrintDialog(PPrintSettingsDialogParent* aDialog,
+                        PBrowserParent* aParent,
+                        const PrintData& aData);
 
     virtual bool
-    RecvSavePrintSettings(const PrintData& aData,
-                          const bool& aUsePrinterNamePrefix,
-                          const uint32_t& aFlags,
-                          nsresult* aResult);
+    RecvSavePrintSettings(const PrintData& data,
+                          const bool& usePrinterNamePrefix,
+                          const uint32_t& flags,
+                          nsresult* rv);
 
     virtual PPrintProgressDialogParent*
     AllocPPrintProgressDialogParent();
@@ -43,6 +43,12 @@ public:
     virtual bool
     DeallocPPrintProgressDialogParent(PPrintProgressDialogParent* aActor);
 
+    virtual PPrintSettingsDialogParent*
+    AllocPPrintSettingsDialogParent();
+
+    virtual bool
+    DeallocPPrintSettingsDialogParent(PPrintSettingsDialogParent* aActor);
+
     virtual void
     ActorDestroy(ActorDestroyReason aWhy);
 
@@ -52,6 +58,11 @@ public:
 private:
     nsIDOMWindow*
     DOMWindowFromBrowserParent(PBrowserParent* parent);
+
+    nsresult
+    ShowPrintDialog(PBrowserParent* parent,
+                    const PrintData& data,
+                    PrintData* result);
 };
 } // namespace embedding
 } // namespace mozilla

embedding/components/printingui/ipc/moz.build

@@ -19,11 +19,15 @@ if CONFIG['NS_PRINTING']:
         'PrintingParent.cpp',
         'PrintProgressDialogChild.cpp',
         'PrintProgressDialogParent.cpp',
+        'PrintSettingsDialogChild.cpp',
+        'PrintSettingsDialogParent.cpp',
     ]
 
 IPDL_SOURCES += [
     'PPrinting.ipdl',
+    'PPrintingTypes.ipdlh',
     'PPrintProgressDialog.ipdl',
+    'PPrintSettingsDialog.ipdl',
 ]
 
 include('/ipc/chromium/chromium-config.mozbuild')

embedding/components/printingui/ipc/nsPrintingProxy.cpp

@@ -95,17 +95,24 @@ nsPrintingProxy::ShowPrintDialog(nsIDOMWindow *parent,
   rv = po->SerializeToPrintData(printSettings, webBrowserPrint, &inSettings);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  PrintData modifiedSettings;
-  bool success;
+  // Now, the waiting game. The parent process should be showing
+  // the printing dialog soon. In the meantime, we need to spin a
+  // nested event loop while we wait for the results of the dialog
+  // to be returned to us.
 
-  mozilla::unused << SendShowPrintDialog(pBrowser, inSettings, &modifiedSettings, &success);
+  nsRefPtr<PrintSettingsDialogChild> dialog = new PrintSettingsDialogChild();
+  SendPPrintSettingsDialogConstructor(dialog);
 
-  if (!success) {
-    // Something failed in the parent.
-    return NS_ERROR_FAILURE;
+  mozilla::unused << SendShowPrintDialog(dialog, pBrowser, inSettings);
+
+  while(!dialog->returned()) {
+    NS_ProcessNextEvent(nullptr, true);
   }
 
-  rv = po->DeserializeToPrintSettings(modifiedSettings, printSettings);
+  rv = dialog->result();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = po->DeserializeToPrintSettings(dialog->data(), printSettings);
   return NS_OK;
 }
 
@@ -206,3 +213,21 @@ nsPrintingProxy::DeallocPPrintProgressDialogChild(PPrintProgressDialogChild* aAc
                 "called on nsPrintingProxy.");
   return false;
 }
+
+PPrintSettingsDialogChild*
+nsPrintingProxy::AllocPPrintSettingsDialogChild()
+{
+  // The parent process will never initiate the PPrintSettingsDialog
+  // protocol connection, so no need to provide an allocator here.
+  NS_NOTREACHED("Allocator for PPrintSettingsDialogChild should not be "
+                "called on nsPrintingProxy.");
+  return nullptr;
+}
+
+bool
+nsPrintingProxy::DeallocPPrintSettingsDialogChild(PPrintSettingsDialogChild* aActor)
+{
+  // The PrintSettingsDialogChild implements refcounting, and
+  // will take itself out.
+  return true;
+}

embedding/components/printingui/ipc/nsPrintingProxy.h

@@ -33,6 +33,12 @@ public:
 
     virtual bool
     DeallocPPrintProgressDialogChild(PPrintProgressDialogChild* aActor) MOZ_OVERRIDE;
+
+    virtual PPrintSettingsDialogChild*
+    AllocPPrintSettingsDialogChild() MOZ_OVERRIDE;
+
+    virtual bool
+    DeallocPPrintSettingsDialogChild(PPrintSettingsDialogChild* aActor) MOZ_OVERRIDE;
 };
 
 #endif