From 7d8410d3a51b4b853a07c38dc4e1d69bf2179067 Mon Sep 17 00:00:00 2001 From: Michael Layzell Date: Tue, 14 Mar 2017 17:01:38 -0400 Subject: [PATCH] Bug 1348401 - Make the PPrinting::ShowProgress IPC message async, r=mconley, r=billm MozReview-Commit-ID: 5pK08I3itYa --- ipc/ipdl/sync-messages.ini | 2 - .../components/printingui/ipc/PPrinting.ipdl | 10 ++- .../printingui/ipc/PrintingParent.cpp | 63 ++++++++++--------- .../printingui/ipc/PrintingParent.h | 4 +- .../printingui/ipc/nsPrintingProxy.cpp | 10 +-- 5 files changed, 45 insertions(+), 44 deletions(-) diff --git a/ipc/ipdl/sync-messages.ini b/ipc/ipdl/sync-messages.ini index 62ff87aa8619..a04d06b191ea 100644 --- a/ipc/ipdl/sync-messages.ini +++ b/ipc/ipdl/sync-messages.ini @@ -1078,8 +1078,6 @@ description = description = [PCookieService::GetCookieString] description = -[PPrinting::ShowProgress] -description = [PPrinting::SavePrintSettings] description = [PHandlerService::FillHandlerInfo] diff --git a/toolkit/components/printingui/ipc/PPrinting.ipdl b/toolkit/components/printingui/ipc/PPrinting.ipdl index 0b4a1cf64205..5adf0e930b79 100644 --- a/toolkit/components/printingui/ipc/PPrinting.ipdl +++ b/toolkit/components/printingui/ipc/PPrinting.ipdl @@ -21,12 +21,10 @@ sync protocol PPrinting manages PRemotePrintJob; parent: - sync ShowProgress(PBrowser browser, - PPrintProgressDialog printProgressDialog, - nullable PRemotePrintJob remotePrintJob, - bool isForPrinting) - returns(bool notifyOnOpen, - nsresult rv); + async ShowProgress(PBrowser browser, + PPrintProgressDialog printProgressDialog, + nullable PRemotePrintJob remotePrintJob, + bool isForPrinting); async ShowPrintDialog(PPrintSettingsDialog dialog, nullable PBrowser browser, diff --git a/toolkit/components/printingui/ipc/PrintingParent.cpp b/toolkit/components/printingui/ipc/PrintingParent.cpp index 12ca280037f5..9695d9836e22 100644 --- a/toolkit/components/printingui/ipc/PrintingParent.cpp +++ b/toolkit/components/printingui/ipc/PrintingParent.cpp @@ -33,24 +33,13 @@ mozilla::ipc::IPCResult PrintingParent::RecvShowProgress(PBrowserParent* parent, PPrintProgressDialogParent* printProgressDialog, PRemotePrintJobParent* remotePrintJob, - const bool& isForPrinting, - bool* notifyOnOpen, - nsresult* result) + const bool& isForPrinting) { - *result = NS_ERROR_FAILURE; - *notifyOnOpen = false; + bool notifyOnOpen = false; nsCOMPtr parentWin = DOMWindowFromBrowserParent(parent); - if (!parentWin) { - return IPC_OK(); - } - nsCOMPtr pps(do_GetService("@mozilla.org/embedcomp/printingprompt-service;1")); - if (!pps) { - return IPC_OK(); - } - PrintProgressDialogParent* dialogParent = static_cast(printProgressDialog); nsCOMPtr observer = do_QueryInterface(dialogParent); @@ -58,24 +47,42 @@ PrintingParent::RecvShowProgress(PBrowserParent* parent, nsCOMPtr printProgressListener; nsCOMPtr printProgressParams; - *result = pps->ShowProgress(parentWin, nullptr, nullptr, observer, - isForPrinting, - getter_AddRefs(printProgressListener), - getter_AddRefs(printProgressParams), - notifyOnOpen); - NS_ENSURE_SUCCESS(*result, IPC_OK()); - - if (remotePrintJob) { - // If we have a RemotePrintJob use that as a more general forwarder for - // print progress listeners. - static_cast(remotePrintJob) - ->RegisterListener(printProgressListener); - } else { - dialogParent->SetWebProgressListener(printProgressListener); + nsresult rv = NS_ERROR_INVALID_ARG; + if (parentWin && pps) { + rv = pps->ShowProgress(parentWin, nullptr, nullptr, observer, + isForPrinting, + getter_AddRefs(printProgressListener), + getter_AddRefs(printProgressParams), + ¬ifyOnOpen); } - dialogParent->SetPrintProgressParams(printProgressParams); + if (NS_SUCCEEDED(rv)) { + if (remotePrintJob) { + // If we have a RemotePrintJob use that as a more general forwarder for + // print progress listeners. + static_cast(remotePrintJob) + ->RegisterListener(printProgressListener); + } else { + dialogParent->SetWebProgressListener(printProgressListener); + } + dialogParent->SetPrintProgressParams(printProgressParams); + } + + // NOTE: If we aren't going to observe an event on our observer, we need to + // fake one. This takes the form of sending the SendDialogOpened message. This + // is safe because the child process proxy will always return `true` for + // notifyOnOpen, as the request will always be async when performed across + // process boundaries. + // + // We can pass nullptr for all of the arguments, as all consumers of this + // observer don't care about the subject, topic, or data. + // + // If notifyOnOpen is true, then the ShowProgress call will handle notifying + // our observer for us. + if (!notifyOnOpen) { + observer->Observe(nullptr, nullptr, nullptr); + } return IPC_OK(); } diff --git a/toolkit/components/printingui/ipc/PrintingParent.h b/toolkit/components/printingui/ipc/PrintingParent.h index a2ac0de67b60..18a6663e8d1f 100644 --- a/toolkit/components/printingui/ipc/PrintingParent.h +++ b/toolkit/components/printingui/ipc/PrintingParent.h @@ -33,9 +33,7 @@ public: RecvShowProgress(PBrowserParent* parent, PPrintProgressDialogParent* printProgressDialog, PRemotePrintJobParent* remotePrintJob, - const bool& isForPrinting, - bool* notifyOnOpen, - nsresult* result); + const bool& isForPrinting); virtual mozilla::ipc::IPCResult RecvShowPrintDialog(PPrintSettingsDialogParent* aDialog, PBrowserParent* aParent, diff --git a/toolkit/components/printingui/ipc/nsPrintingProxy.cpp b/toolkit/components/printingui/ipc/nsPrintingProxy.cpp index b693a1cb9d90..ea420d693f07 100644 --- a/toolkit/components/printingui/ipc/nsPrintingProxy.cpp +++ b/toolkit/components/printingui/ipc/nsPrintingProxy.cpp @@ -166,12 +166,12 @@ nsPrintingProxy::ShowProgress(mozIDOMWindowProxy* parent, } } - nsresult rv = NS_OK; + // NOTE: We set notifyOnOpen to true unconditionally. If the parent process + // would get `false` for notifyOnOpen, then it will synthesize a notification + // which will be sent asynchronously down to the child. + *notifyOnOpen = true; mozilla::Unused << SendShowProgress(pBrowser, dialogChild, remotePrintJob, - isForPrinting, notifyOnOpen, &rv); - if (NS_FAILED(rv)) { - return rv; - } + isForPrinting); // If we have a RemotePrintJob that will be being used as a more general // forwarder for print progress listeners. Once we always have one we can