Skip to content

Commit

Permalink
Bug 1432651 p1 - Pass RemotePrintJobChild through to the places where…
Browse files Browse the repository at this point in the history
… it's needed. r=emilio

Given how nsIPrintSettings is passed around, stored and copied all over the
place, it's very hard to reason about where and when a RemotePrintJobChild is
needed or valid. This patch avoids all that by explicitly passing a
RemotePrintJobChild when it's needed.

Another reason to make this change is because RemotePrintJobChild really does
not belong on nsIPrintSettings. That interface is supposed to represent a
collection of settings for laying out the document that is to be printed.

Differential Revision: https://phabricator.services.mozilla.com/D146380
  • Loading branch information
jwatt committed May 16, 2022
1 parent 41cb3fd commit 41b151c
Show file tree
Hide file tree
Showing 13 changed files with 78 additions and 49 deletions.
3 changes: 2 additions & 1 deletion docshell/base/CanonicalBrowsingContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,8 @@ already_AddRefed<Promise> CanonicalBrowsingContext::Print(
}

ErrorResult rv;
outerWindow->Print(aPrintSettings, listener,
outerWindow->Print(aPrintSettings,
/* aRemotePrintJob = */ nullptr, listener,
/* aDocShellToCloneInto = */ nullptr,
nsGlobalWindowOuter::IsPreview::No,
nsGlobalWindowOuter::IsForWindowDotPrint::No,
Expand Down
7 changes: 6 additions & 1 deletion docshell/base/nsIContentViewer.idl
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ class PresShell;
namespace dom {
class WindowGlobalChild;
} // namespace dom
namespace layout {
class RemotePrintJobChild;
} // namespace layout
} // namespace mozilla
%}

Expand All @@ -34,6 +37,7 @@ class WindowGlobalChild;
[ptr] native nsDOMNavigationTimingPtr(nsDOMNavigationTiming);
[ptr] native Encoding(const mozilla::Encoding);
[ptr] native PresShellPtr(mozilla::PresShell);
[ptr] native RemotePrintJobChildPtr(mozilla::layout::RemotePrintJobChild);
[ptr] native WindowGlobalChildPtr(mozilla::dom::WindowGlobalChild);

[scriptable, builtinclass, uuid(2da17016-7851-4a45-a7a8-00b360e01595)]
Expand Down Expand Up @@ -205,7 +209,8 @@ interface nsIContentViewer : nsISupports
/**
* Sets the print settings for print / print-previewing a subdocument.
*/
[can_run_script] void setPrintSettingsForSubdocument(in nsIPrintSettings aPrintSettings);
[can_run_script] void setPrintSettingsForSubdocument(in nsIPrintSettings aPrintSettings,
in RemotePrintJobChildPtr aRemotePrintJob);

/**
* Get the history entry that this viewer will save itself into when
Expand Down
1 change: 1 addition & 0 deletions dom/base/nsFrameLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3449,6 +3449,7 @@ already_AddRefed<Promise> nsFrameLoader::PrintPreview(
ErrorResult rv;
sourceWindow->Print(
aPrintSettings,
/* aRemotePrintJob = */ nullptr,
/* aListener = */ nullptr, docShellToCloneInto,
nsGlobalWindowOuter::IsPreview::Yes,
nsGlobalWindowOuter::IsForWindowDotPrint::No,
Expand Down
14 changes: 8 additions & 6 deletions dom/base/nsGlobalWindowInner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3866,12 +3866,14 @@ void nsGlobalWindowInner::Print(ErrorResult& aError) {
Nullable<WindowProxyHolder> nsGlobalWindowInner::PrintPreview(
nsIPrintSettings* aSettings, nsIWebProgressListener* aListener,
nsIDocShell* aDocShellToCloneInto, ErrorResult& aError) {
FORWARD_TO_OUTER_OR_THROW(Print,
(aSettings, aListener, aDocShellToCloneInto,
nsGlobalWindowOuter::IsPreview::Yes,
nsGlobalWindowOuter::IsForWindowDotPrint::No,
/* aPrintPreviewCallback = */ nullptr, aError),
aError, nullptr);
FORWARD_TO_OUTER_OR_THROW(
Print,
(aSettings,
/* aRemotePrintJob = */ nullptr, aListener, aDocShellToCloneInto,
nsGlobalWindowOuter::IsPreview::Yes,
nsGlobalWindowOuter::IsForWindowDotPrint::No,
/* aPrintPreviewCallback = */ nullptr, aError),
aError, nullptr);
}

void nsGlobalWindowInner::MoveTo(int32_t aXPos, int32_t aYPos,
Expand Down
11 changes: 6 additions & 5 deletions dom/base/nsGlobalWindowOuter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ using namespace mozilla::dom::ipc;
using mozilla::BasePrincipal;
using mozilla::OriginAttributes;
using mozilla::TimeStamp;
using mozilla::layout::RemotePrintJobChild;

#define FORWARD_TO_INNER(method, args, err_rval) \
PR_BEGIN_MACRO \
Expand Down Expand Up @@ -5139,7 +5140,7 @@ void nsGlobalWindowOuter::PrintOuter(ErrorResult& aError) {
});

const bool forPreview = !StaticPrefs::print_always_print_silent();
Print(nullptr, nullptr, nullptr, IsPreview(forPreview),
Print(nullptr, nullptr, nullptr, nullptr, IsPreview(forPreview),
IsForWindowDotPrint::Yes, nullptr, aError);
#endif
}
Expand All @@ -5159,9 +5160,9 @@ class MOZ_RAII AutoModalState {
};

Nullable<WindowProxyHolder> nsGlobalWindowOuter::Print(
nsIPrintSettings* aPrintSettings, nsIWebProgressListener* aListener,
nsIDocShell* aDocShellToCloneInto, IsPreview aIsPreview,
IsForWindowDotPrint aForWindowDotPrint,
nsIPrintSettings* aPrintSettings, RemotePrintJobChild* aRemotePrintJob,
nsIWebProgressListener* aListener, nsIDocShell* aDocShellToCloneInto,
IsPreview aIsPreview, IsForWindowDotPrint aForWindowDotPrint,
PrintPreviewResolver&& aPrintPreviewCallback, ErrorResult& aError) {
#ifdef NS_PRINTING
nsCOMPtr<nsIPrintSettingsService> printSettingsService =
Expand Down Expand Up @@ -5318,7 +5319,7 @@ Nullable<WindowProxyHolder> nsGlobalWindowOuter::Print(
}
} else {
// Historically we've eaten this error.
webBrowserPrint->Print(ps, aListener);
webBrowserPrint->Print(ps, aRemotePrintJob, aListener);
}
}

Expand Down
8 changes: 6 additions & 2 deletions dom/base/nsGlobalWindowOuter.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ class CacheStorage;
} // namespace cache
class IDBFactory;
} // namespace dom
namespace layout {
class RemotePrintJobChild;
} // namespace layout
} // namespace mozilla

extern already_AddRefed<nsIScriptTimeoutHandler> NS_CreateJSTimeoutHandler(
Expand Down Expand Up @@ -588,8 +591,9 @@ class nsGlobalWindowOuter final : public mozilla::dom::EventTarget,
enum class IsPreview : bool { No, Yes };
enum class IsForWindowDotPrint : bool { No, Yes };
mozilla::dom::Nullable<mozilla::dom::WindowProxyHolder> Print(
nsIPrintSettings*, nsIWebProgressListener*, nsIDocShell*, IsPreview,
IsForWindowDotPrint, PrintPreviewResolver&&, mozilla::ErrorResult&);
nsIPrintSettings*, mozilla::layout::RemotePrintJobChild* aRemotePrintJob,
nsIWebProgressListener*, nsIDocShell*, IsPreview, IsForWindowDotPrint,
PrintPreviewResolver&&, mozilla::ErrorResult&);
mozilla::dom::Selection* GetSelectionOuter();
already_AddRefed<mozilla::dom::Selection> GetSelection() override;
nsScreen* GetScreen();
Expand Down
19 changes: 12 additions & 7 deletions dom/ipc/BrowserChild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "mozilla/EventListenerManager.h"
#include "mozilla/HoldDropJSObjects.h"
#include "mozilla/IMEStateManager.h"
#include "mozilla/layout/RemotePrintJobChild.h"
#include "mozilla/LookAndFeel.h"
#include "mozilla/MouseEvents.h"
#include "mozilla/NativeKeyBindingsType.h"
Expand Down Expand Up @@ -1116,7 +1117,9 @@ nsresult BrowserChild::UpdateRemotePrintSettings(
// BC tree, and our code above is simple enough and keeps strong refs to
// everything.
([&]() MOZ_CAN_RUN_SCRIPT_BOUNDARY {
cv->SetPrintSettingsForSubdocument(printSettings);
cv->SetPrintSettingsForSubdocument(
printSettings, static_cast<RemotePrintJobChild*>(
aPrintData.remotePrintJobChild()));
}());
} else if (RefPtr<BrowserBridgeChild> remoteChild =
BrowserBridgeChild::GetFrom(aBc->GetEmbedderElement())) {
Expand Down Expand Up @@ -2468,6 +2471,7 @@ mozilla::ipc::IPCResult BrowserChild::RecvPrintPreview(
}

sourceWindow->Print(printSettings,
/* aRemotePrintJob = */ nullptr,
/* aListener = */ nullptr, docShellToCloneInto,
nsGlobalWindowOuter::IsPreview::Yes,
nsGlobalWindowOuter::IsForWindowDotPrint::No,
Expand Down Expand Up @@ -2523,12 +2527,13 @@ mozilla::ipc::IPCResult BrowserChild::RecvPrint(
printSettingsSvc->DeserializeToPrintSettings(aPrintData, printSettings);
{
IgnoredErrorResult rv;
outerWindow->Print(printSettings,
/* aListener = */ nullptr,
/* aWindowToCloneInto = */ nullptr,
nsGlobalWindowOuter::IsPreview::No,
nsGlobalWindowOuter::IsForWindowDotPrint::No,
/* aPrintPreviewCallback = */ nullptr, rv);
outerWindow->Print(
printSettings,
static_cast<RemotePrintJobChild*>(aPrintData.remotePrintJobChild()),
/* aListener = */ nullptr,
/* aWindowToCloneInto = */ nullptr, nsGlobalWindowOuter::IsPreview::No,
nsGlobalWindowOuter::IsForWindowDotPrint::No,
/* aPrintPreviewCallback = */ nullptr, rv);
if (NS_WARN_IF(rv.Failed())) {
return IPC_OK();
}
Expand Down
10 changes: 7 additions & 3 deletions layout/base/nsDocumentViewer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ class PrintPreviewResultInfo;
using namespace mozilla;
using namespace mozilla::dom;

using mozilla::layout::RemotePrintJobChild;
using PrintPreviewResolver =
std::function<void(const mozilla::dom::PrintPreviewResultInfo&)>;

Expand Down Expand Up @@ -2898,6 +2899,7 @@ nsresult nsDocViewerFocusListener::HandleEvent(Event* aEvent) {

NS_IMETHODIMP
nsDocumentViewer::Print(nsIPrintSettings* aPrintSettings,
RemotePrintJobChild* aRemotePrintJob,
nsIWebProgressListener* aWebProgressListener) {
if (NS_WARN_IF(!mContainer)) {
PR_PL(("Container was destroyed yet we are still trying to use it!"));
Expand Down Expand Up @@ -2932,7 +2934,8 @@ nsDocumentViewer::Print(nsIPrintSettings* aPrintSettings,
}

mPrintJob = printJob;
rv = printJob->Print(mDocument, aPrintSettings, aWebProgressListener);
rv = printJob->Print(mDocument, aPrintSettings, aRemotePrintJob,
aWebProgressListener);
if (NS_WARN_IF(NS_FAILED(rv))) {
OnDonePrinting();
}
Expand Down Expand Up @@ -3394,7 +3397,7 @@ void nsDocumentViewer::OnDonePrinting() {
}

NS_IMETHODIMP nsDocumentViewer::SetPrintSettingsForSubdocument(
nsIPrintSettings* aPrintSettings) {
nsIPrintSettings* aPrintSettings, RemotePrintJobChild* aRemotePrintJob) {
#ifdef NS_PRINTING
{
nsAutoScriptBlocker scriptBlocker;
Expand All @@ -3414,7 +3417,8 @@ NS_IMETHODIMP nsDocumentViewer::SetPrintSettingsForSubdocument(
return NS_ERROR_NOT_AVAILABLE;
}

RefPtr<nsIDeviceContextSpec> devspec = new nsDeviceContextSpecProxy();
RefPtr<nsDeviceContextSpecProxy> devspec =
new nsDeviceContextSpecProxy(aRemotePrintJob);
nsresult rv =
devspec->Init(nullptr, aPrintSettings, /* aIsPrintPreview = */ true);
NS_ENSURE_SUCCESS(rv, rv);
Expand Down
30 changes: 11 additions & 19 deletions layout/printing/nsPrintJob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -551,22 +551,18 @@ nsresult nsPrintJob::DoCommonPrint(bool aIsPrintPreview,
printSession = do_CreateInstance("@mozilla.org/gfx/printsession;1", &rv);
NS_ENSURE_SUCCESS(rv, rv);
printData->mPrintSettings->SetPrintSession(printSession);
} else {
RefPtr<layout::RemotePrintJobChild> remotePrintJob =
printSession->GetRemotePrintJob();
if (remotePrintJob) {
// If we have a RemotePrintJob add it to the print progress listeners,
// so it can forward to the parent.
printData->mPrintProgressListeners.AppendElement(remotePrintJob);
}
} else if (mRemotePrintJob) {
// If we have a RemotePrintJob add it to the print progress listeners,
// so it can forward to the parent.
printData->mPrintProgressListeners.AppendElement(mRemotePrintJob);
}
}

bool printingViaParent =
XRE_IsContentProcess() && StaticPrefs::print_print_via_parent();
nsCOMPtr<nsIDeviceContextSpec> devspec;
if (printingViaParent) {
devspec = new nsDeviceContextSpecProxy();
devspec = new nsDeviceContextSpecProxy(mRemotePrintJob);
} else {
devspec = do_CreateInstance("@mozilla.org/gfx/devicecontextspec;1", &rv);
NS_ENSURE_SUCCESS(rv, rv);
Expand Down Expand Up @@ -601,7 +597,10 @@ nsresult nsPrintJob::DoCommonPrint(bool aIsPrintPreview,
//---------------------------------------------------------------------------------
nsresult nsPrintJob::Print(Document* aSourceDoc,
nsIPrintSettings* aPrintSettings,
RemotePrintJobChild* aRemotePrintJob,
nsIWebProgressListener* aWebProgressListener) {
mRemotePrintJob = aRemotePrintJob;

// If we have a print preview document, use that instead of the original
// mDocument. That way animated images etc. get printed using the same state
// as in print preview.
Expand Down Expand Up @@ -2272,16 +2271,9 @@ nsresult nsPrintJob::StartPagePrintTimer(const UniquePtr<nsPrintObject>& aPO) {
mPagePrintTimer =
new nsPagePrintTimer(this, mDocViewerPrint, doc, printPageDelay);

nsCOMPtr<nsIPrintSession> printSession;
nsresult rv =
mPrt->mPrintSettings->GetPrintSession(getter_AddRefs(printSession));
if (NS_SUCCEEDED(rv) && printSession) {
RefPtr<layout::RemotePrintJobChild> remotePrintJob =
printSession->GetRemotePrintJob();
if (remotePrintJob) {
remotePrintJob->SetPagePrintTimer(mPagePrintTimer);
remotePrintJob->SetPrintJob(this);
}
if (mRemotePrintJob) {
mRemotePrintJob->SetPagePrintTimer(mPagePrintTimer);
mRemotePrintJob->SetPrintJob(this);
}
}

Expand Down
6 changes: 6 additions & 0 deletions layout/printing/nsPrintJob.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#define nsPrintJob_h

#include "mozilla/Attributes.h"
#include "mozilla/layout/RemotePrintJobChild.h"
#include "mozilla/UniquePtr.h"

#include "nsCOMPtr.h"
Expand Down Expand Up @@ -52,6 +53,7 @@ class nsPrintJob final : public nsIWebProgressListener,
using Document = mozilla::dom::Document;
using PrintPreviewResolver =
std::function<void(const mozilla::dom::PrintPreviewResultInfo&)>;
using RemotePrintJobChild = mozilla::layout::RemotePrintJobChild;

public:
nsPrintJob();
Expand Down Expand Up @@ -97,6 +99,7 @@ class nsPrintJob final : public nsIWebProgressListener,
*/
MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult
Print(Document* aSourceDoc, nsIPrintSettings* aPrintSettings,
RemotePrintJobChild* aRemotePrintJob,
nsIWebProgressListener* aWebProgressListener);

/**
Expand Down Expand Up @@ -254,6 +257,9 @@ class nsPrintJob final : public nsIWebProgressListener,

RefPtr<nsPagePrintTimer> mPagePrintTimer;

// Only set if this nsPrintJob was created for a real print.
RefPtr<RemotePrintJobChild> mRemotePrintJob;

// If the code that initiates a print preview passes a PrintPreviewResolver
// (a std::function) to be notified of the final sheet/page counts (once
// we've sufficiently laid out the document to know what those are), that
Expand Down
5 changes: 5 additions & 0 deletions toolkit/components/browser/nsIWebBrowserPrint.idl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ namespace mozilla {
namespace dom {
class PrintPreviewResultInfo;
} // namespace dom
namespace layout {
class RemotePrintJobChild;
} // namespace layout
} // namespace mozilla
%}

Expand All @@ -21,6 +24,7 @@ interface nsIPrintSettings;
interface nsIWebProgressListener;

native PrintPreviewResolver(std::function<void(const mozilla::dom::PrintPreviewResultInfo&)>&&);
[ptr] native RemotePrintJobChildPtr(mozilla::layout::RemotePrintJobChild);

/**
* nsIWebBrowserPrint corresponds to the main interface
Expand Down Expand Up @@ -101,6 +105,7 @@ interface nsIWebBrowserPrint : nsISupports
* @note To cancel, close the window of the document that is being printed.
*/
[noscript] void print(in nsIPrintSettings aThePrintSettings,
in RemotePrintJobChildPtr aRemotePrintJob,
in nsIWebProgressListener aWPListener);

/**
Expand Down
5 changes: 3 additions & 2 deletions widget/nsDeviceContextSpecProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ using namespace mozilla::gfx;

NS_IMPL_ISUPPORTS(nsDeviceContextSpecProxy, nsIDeviceContextSpec)

nsDeviceContextSpecProxy::nsDeviceContextSpecProxy() = default;
nsDeviceContextSpecProxy::nsDeviceContextSpecProxy(
RemotePrintJobChild* aRemotePrintJob)
: mRemotePrintJob(aRemotePrintJob) {}
nsDeviceContextSpecProxy::~nsDeviceContextSpecProxy() = default;

NS_IMETHODIMP
Expand Down Expand Up @@ -61,7 +63,6 @@ nsDeviceContextSpecProxy::Init(nsIWidget* aWidget,
return NS_ERROR_FAILURE;
}

mRemotePrintJob = mPrintSession->GetRemotePrintJob();
if (!mRemotePrintJob) {
NS_WARNING("We can't print via the parent without a RemotePrintJobChild.");
return NS_ERROR_FAILURE;
Expand Down
8 changes: 5 additions & 3 deletions widget/nsDeviceContextSpecProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ class RemotePrintJobChild;

class nsDeviceContextSpecProxy final : public nsIDeviceContextSpec {
public:
using RemotePrintJobChild = mozilla::layout::RemotePrintJobChild;

explicit nsDeviceContextSpecProxy(RemotePrintJobChild* aRemotePrintJob);

NS_DECL_ISUPPORTS

NS_IMETHOD Init(nsIWidget* aWidget, nsIPrintSettings* aPrintSettings,
Expand Down Expand Up @@ -52,15 +56,13 @@ class nsDeviceContextSpecProxy final : public nsIDeviceContextSpec {

NS_IMETHOD EndPage() final;

nsDeviceContextSpecProxy();

private:
~nsDeviceContextSpecProxy();

nsCOMPtr<nsIPrintSettings> mPrintSettings;
nsCOMPtr<nsIPrintSession> mPrintSession;
nsCOMPtr<nsIDeviceContextSpec> mRealDeviceContextSpec;
RefPtr<mozilla::layout::RemotePrintJobChild> mRemotePrintJob;
RefPtr<RemotePrintJobChild> mRemotePrintJob;
RefPtr<mozilla::layout::DrawEventRecorderPRFileDesc> mRecorder;
};

Expand Down

0 comments on commit 41b151c

Please sign in to comment.