This patch doesn't change behavior; it's just collapsing logic from
nsPrintJob's Initialize method (which is now de-facto infallible) into the
constructor.
I'm also removing the "Methods needed by the DocViewer" header-comment since
it's clearly innacurate at this point. It's only surrounding this constructor
and GetSeqFrameAndCountSheets, which is silly since nsDocumentViewer uses more
of our API than that.
(I also placed TODO(dholbert) comments for a few things that looked odd, to
follow up on later. In particular, I noticed that nsDocumentViewer holds a
dedicated stack-owned RefPtr reference to nsPrintJob after creating it, which
superficially looks unnecessary. I don't want to risk changing behvior or
introducing a crash by removing that reference in this refactoring patch, so
I'm leaving that as-is and simply flagging it as suspicious.)
Depends on D150194
Differential Revision: https://phabricator.services.mozilla.com/D150195
This patch does not change behavior (other than a minor correctness fix in some
off-by-default logging; see below).
As shown in the removed line of the init list, this mFrameType member-var's
semantics are equivalent to a null-check of the mParent member-var. So: rather
than encoding that same information twice, this patch simplifies to just
directly null-check the mParent pointer at all the usage sites.
Before this patch, nsPrintJob.cpp had two different patterns for logging
mFrameType; sometimes with a global `gFrameTypesStr` array, and other times
with a local `types` array (with shorter 2-character strings). I've converted
both of these to helper-functions.
In the case of the `types` array, the old code used a 4-value array, which was
interesting since the enum type only had 2 possible values. This discrepancy is
just due to an oversight in bug 1769508, where we recently condensed the enum
from 4 values to 2; that bug technically should've condensed these arrays as
well (but didn't do so). This left these arrays' enum-to-string mapping being
wrong (since eIFrame changed its numeric value from 2 to 1 in bug 1769508), but
probably nobody has used this logging code in a while, so nobody
noticed. Anyway: in this patch, I'm restoring the mappings that we had before
that change (so we'll log "DC" for root print objects and "IF" for non-root
i.e. iframe-flavored print objects).
Differential Revision: https://phabricator.services.mozilla.com/D150177
This patch doesn't change behavior; it's just refactoring.
The pre-existing Init code is now clearly infallible, now that earlier patches
have removed all of Init's NS_ENSURE_STATE arg-null-checks (which were Init's
only failure-returning codepaths).
So: now that it's infallible, we can just merge the Init() code directly into
the constructor.
This lets us promote some member-variables to be 'const' as well, since they
will now be initialized in the init list and are never modified after that.
Differential Revision: https://phabricator.services.mozilla.com/D150079
This patch doesn't change behavior. It just changes to c++ reference types and
code-comments to indicate where we know that pointers have been null-checked
(and removes some null-checks that now become trivially-unnecessary).
I've added code-comments to justify why we know these args are non-null.
Generally, `nsPrintObject::Init`'s args are null-checked by the caller, except
in one case (in `nsPrintJob::DoCommonPrint`) where the Document* pointer in
question was _not_ directly null-checked by the caller. But fortunately, it is
null-checked earlier, higher up in the call-stack. So, this patch simply
propagates the C++ reference type-conversion up to that point for additional
clarity.
Differential Revision: https://phabricator.services.mozilla.com/D150078
This patch doesn't change behavior.
The two init functions were already pretty similar. This patch merges them,
and uses the "aParent" arg as a signal for which variant we're handling (with
the "root" variant being signalled via a null value).
This usage of aParent (to distinguish between variants) is valid, as long as we
can assume that the old `InitAsNestedObject` variant was always guaranteed to
receive a non-null value for its aParent arg. (That would mean that we're OK to
use that arg's nullness as a way to distinguish between the two variants in
their new merged form.)
And indeed, we're safe to make this assumption, since there's only one callsite
for the `InitAsNestedObject` version (which is in
`nsPrintJob::BuildNestedPrintObjects`), and it dereferences the pointer before
calling into this init function. So, the pointer has to be non-null, or else
we would have crashed at the earlier dereference.
Therefore, this conversion (and usage of aParent to distinguish between
variants) is valid.
(Spoiler alert: a later patch in this series will merge this Init() method
into the constructor. I'm doing this as a multi-step process with this
intermediate state, in order to hopefully make it easier to reason about the
conversion and confirm that it's valid.)
Differential Revision: https://phabricator.services.mozilla.com/D150077
This patch doesn't change behavior.
The assignment here is working with a freshly-constructed `nsPrintObject`,
which was initialized via `InitAsRootObject`. Its `mFrameType` will have
already been assigned to `eDoc` in the constructor. The only place in our
codebase where we might hypothetically reassign `mFrameType` is in
`InitAsNestedObject`, but that's the other init method -- not the one we're
calling here.
So: `mFrameType` will already be `eDoc` when we reach this assignment, and
the assignment is unnecessary.
Differential Revision: https://phabricator.services.mozilla.com/D150076
We're seeing a lot of null crashes here. Maybe we're running script since
we last checked mPrintObject and the printer was disconnected or
something along those lines, so just add a null check.
Differential Revision: https://phabricator.services.mozilla.com/D149387
This avoids exposing printer names to potentially compromised content processes.
The changes in bug 1770211 mean that we no longer create platform specific
nsIDeviceContextSpec instances in the content process, so we no longer need
the printer name to instantiate an nsDeviceContextSpecWin there.
Differential Revision: https://phabricator.services.mozilla.com/D146868
Since bug 1659432 landed, we no longer keep the nsPrintJob around across print
settings changes, only replacing the nsPrintData. Now that we throw away
everything on a settings change it doesn't make sense to have code that was
written for juggling nsPrintData objects. That code was just making things
difficult to understand.
Differential Revision: https://phabricator.services.mozilla.com/D146718
The comments and structure of this code no longer made much sense after the
last 20 years of churn, including those made in bug 1659432 and bug 1552785.
Differential Revision: https://phabricator.services.mozilla.com/D146717
The comments and structure of this code no longer made much sense after the
last 20 years of churn, including those made in bug 1659432 and bug 1552785.
Differential Revision: https://phabricator.services.mozilla.com/D146717
We create a new nsPrintJob every time Print() or PrintPreview() is invoked
now ( https://phabricator.services.mozilla.com/D87417 ), so the mPrtPreview
member is never set this early on.
This also renames aSourceDoc to just aDoc since the document that is passed is
nowadays the actual document we're going to print from (the static clone,
which nowadays is created before these methods are invoked rather than being
created by them).
Differential Revision: https://phabricator.services.mozilla.com/D146563
Long ago we used to play switcheroo with PresShells in order to print a
document, but for over a decade we've been cloning the document that is
to be printed (bug 487667). Since then we've had no need for this method.
Differential Revision: https://phabricator.services.mozilla.com/D146545
Lately nsIPrintSession was only used to pass around RemotePrintJobChild objects.
Now that we pass those objects explicitly where needed (part 1), this class
serves no purpose.
Another reason to want to get rid of this class is that having it as a member
of nsIPrintSettings made no sense and was confusing.
Differential Revision: https://phabricator.services.mozilla.com/D146381
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
nsIPrintSettings is supposed to be a collection of settings passed to the
platform code to determine how the document prints. The
isPrintSelectionRBEnabled member doesn't belong here since it is a flag that
is passed to the OS native print settings dialog to tell it whether to
display a "Print selection only" radio button.
Depends on D146232
Differential Revision: https://phabricator.services.mozilla.com/D146251
nsIPrintSettings is supposed to be a collection of settings passed to the
platform code to determine how the document prints. The
isPrintSelectionRBEnabled member doesn't belong here since it is a flag that
is passed to the OS native print settings dialog to tell it whether to
display a "Print selection only" radio button.
Differential Revision: https://phabricator.services.mozilla.com/D146251
nsIPrintSettings.isCancelled was only being set to true by the Windows widget
code ShowNativePrintDialog nowadays. That seems pointless since that widget
code is only invoked under the _showPrintDialog call in print.js, and in the
case that the widget code throws, the print is never invoked and the
nsIPrintSettings object isn't used.
nsIPrintSettings.saveOnCancel was set in some places but never read.
Differential Revision: https://phabricator.services.mozilla.com/D144830
It seems the new tab-modal printing code doesn't actually use this. Instead it
relies on the window being closed (see cancelPrint() in
toolkit/components/printing/content/print.js ).
Differential Revision: https://phabricator.services.mozilla.com/D144821
There are some mediaqueries-5 features that we still don't support and
explain the remaining failures in at-container-{parsing,serialization}.
Differential Revision: https://phabricator.services.mozilla.com/D144446
The trickiest bits are the PrintTargetCG ones, the rest is just plumbing
and cleanups and tests, but let me know if you want those to be split
out, can do.
The GTK change to nsPrintSettingsGTK::GetResolution is a no-op (we only
read resolution on windows), but I did that because we assume that it
doesn't fail and GTK returns a sane default anyways.
Differential Revision: https://phabricator.services.mozilla.com/D142199
The trickiest bits are the PrintTargetCG ones, the rest is just plumbing
and cleanups and tests, but let me know if you want those to be split
out, can do.
The GTK change to nsPrintSettingsGTK::GetResolution is a no-op (we only
read resolution on windows), but I did that because we assume that it
doesn't fail and GTK returns a sane default anyways.
Differential Revision: https://phabricator.services.mozilla.com/D142199
IsSyncPagePrinting() only had one implementation which unconditionally
returned true.
So, any code that was conditioned on !IsSyncPagePrinting() is necessarily
dead/unreachable.
These are also crashing due to a null deref in mPrintTarget which might
happen if print is aborted.
Differential Revision: https://phabricator.services.mozilla.com/D140988
This removal fixes the following non-fatal warning which (like some categories of warnings) only appears when you build in non-unified mode:
layout/printing/nsPrintJob.cpp:61:19: warning: unused variable 'kPrintingPromptService' [-Wunused-const-variable]
(Indeed, this variable was entirely unused.)
This patch also removes a bunch of surrounding #includes that looked
likely-unused to me as well. I did some spot-checks to be pretty-sure the
removed headers are all unused at this point, and (most importantly) I made
sure we still successfully build in non-unified mode. This means that, if we
do actually depend on any of these removed includes, we're still getting them
indirectly via some other header, so these removals shouldn't really cause
trouble.
Depends on D138097
Differential Revision: https://phabricator.services.mozilla.com/D138098
The previous patch in this series fixed some sources of non-unified build errors for this directory, and this patch here fixes the only remaining one, which is:
layout/printing/nsPrintJob.cpp:584:22: error: use of undeclared identifier 'do_CreateInstance'
This API is provided by nsComponentManagerUtils.h, which is the include I'm adding here.
Depends on D138096
Differential Revision: https://phabricator.services.mozilla.com/D138097
