Closed Bug 1430682 Opened 2 years ago Closed 2 years ago

Remove mDocViewerPrint null check in nsPagePrintTimer

Categories

(Core :: Printing: Setup, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(1 file)

In nsPagePrintTimer we set mDocViewerPrint in the ctor and never change it/null it out. We've been dereferencing mDocViewerPrint without a null check in nsPagePrintTimer's ctor for some time (and recently started dereferencing it in the dtor as of bug 1426087 too). So we know this null check is useless and just confusing.
Attached patch patchSplinter Review
Attachment #8942764 - Flags: review?(bobowencode)
(In reply to Jonathan Watt [:jwatt] (needinfo? me) from comment #0)
> We've been dereferencing mDocViewerPrint without a null
> check in nsPagePrintTimer's ctor for some time.

By which I mean in excess of five years.
How about making mDocViewerPrint NotNull?
Comment on attachment 8942764 [details] [diff] [review]
patch

Review of attachment 8942764 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Masatoshi Kimura [:emk] from comment #3)
> How about making mDocViewerPrint NotNull?

Sounds like a good plan, I suppose it could be |const NotNull|.
Attachment #8942764 - Flags: review?(bobowencode) → review+
(In reply to Masatoshi Kimura [:emk] from comment #3)
> How about making mDocViewerPrint NotNull?

Good suggestion (although I'll actually use OwningNotNull, and const as suggested by Bob).
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/76ce0d94b9b7
Use OwningNonNull for nsPagePrintTimer::mDocViewerPrint and remove null checks. r=bobowen
https://hg.mozilla.org/mozilla-central/rev/76ce0d94b9b7
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.