1245978 - de-COM the nsDocumentViewer::CreateStyleSet method

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

Noticed while reviewing bug 1244068 that nsDocumentViewer::CreateStyleSet has an unnecessary (always-NS_OK) return value.

Filing this bug on dropping that return value, and just directly returning the thing it creates.

Wrote a patch for this locally (dropping the return value at least, in this function & its interface & its callers), and it compiles, so I think this doable.

I'll layer this on top of bug 1244068 so as not to bitrot heycam.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: part 1: return a StyleSetHandle directly, instead of using outparam

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: part 2: drop unnecessary (& style-guide-violating) 'virtual' keyword from NS_DECL_NSIDOCUMENTVIEWERPRINT macro

The first patch here makes a line in NS_DECL_NSIDOCUMENTVIEWERPRINT over 80 characters.

This second patch brings that line back underneath 80 characters by dropping the unnecessary "virtual" keyword on that line. (It's unnecessary because we already have "override", which requires that the method is virtual.)

This brings this macro into alignment with our style guide, which (since May of last year) says "Method declarations must use at most one of the following keywords: virtual, override, or final."
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Methods

(For background, this coding style guideline was introduced in this dev.platform thread:
https://groups.google.com/forum/#!msg/mozilla.dev.platform/sxYiu8JHwe4/pBkgfJL3g2gJ )

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: part 3: Drop unnecessary whitespace in NS_DECL_NSIDOCUMENTVIEWERPRINT macro. (whitespace only, no review)

This 3rd part does some whitespace cleanup in the NS_DECL_NSIDOCUMENTVIEWERPRINT macro. (The methods are all indented to align with this one method that used to have "nsresult" as its return value -- but now it no longer returns nsresult, so the indentation is insufficient & not really useful.)

Comment 4

[Cameron McCormack (:heycam)]

Comment on attachment 8723715 [details] [diff] [review]
part 2: drop unnecessary (& style-guide-violating) 'virtual' keyword from  NS_DECL_NSIDOCUMENTVIEWERPRINT macro

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

::: layout/base/nsIDocumentViewerPrint.h
@@ +80,5 @@
> +  bool     IsInitializedForPrintPreview() override; \
> +  void     InitializeForPrintPreview() override; \
> +  void     SetPrintPreviewPresentation(nsViewManager* aViewManager, \
> +                                       nsPresContext* aPresContext, \
> +                                       nsIPresShell* aPresShell) override;

Feel free, if you wish, to remove the extra space after those voids and bools.

Comment 5

[Cameron McCormack (:heycam)]

(In reply to Cameron McCormack (:heycam) (away Feb 27 – Mar 13) from comment #4)
> Feel free, if you wish, to remove the extra space after those voids and
> bools.

I see you did this in the next patch. :-)

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b2b51d8c60cf
https://hg.mozilla.org/integration/mozilla-inbound/rev/ceea6cf9be9d
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8e1ff0ab5a4

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b2b51d8c60cf
https://hg.mozilla.org/mozilla-central/rev/ceea6cf9be9d
https://hg.mozilla.org/mozilla-central/rev/c8e1ff0ab5a4