Closed Bug 1455492 Opened 3 years ago Closed 3 years ago

Most of the ParseSheet arguments seem redundant.

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: emilio, Assigned: emilio)

Details

Attachments

(2 files)

Unless I'm missing something, we always call it with URIs / principals / etc, that have been set into the sheet a bit earlier, so those can go.
Comment on attachment 8969521 [details]
Bug 1455492: Remove redundant ParseSheet arguments.

https://reviewboard.mozilla.org/r/238284/#review244232

::: layout/style/nsLayoutStylesheetCache.cpp:891
(Diff revision 1)
> -  // NB: The pref sheet never has @import rules.
> -  servoSheet->ParseSheetSync(nullptr, sheetText, uri, uri, nullptr,
> -                             /* aLoadData = */ nullptr, 0,
> -                             eCompatibility_FullStandards);
> +  // NB: The pref sheet never has @import rules, thus no loader.
> +  servoSheet->ParseSheetSync(nullptr,
> +                             sheetText,
> +                             /* aLoadData = */ nullptr,

Hm, so this will change from passing nullptr as the principal to having the callee infer it from the sheet, which in this case will be the singleton NullPrincipal from URLExtraData::Dummy AFAICT.

I _think_ this is ok, but please check if any consumer behavior will change.
Attachment #8969521 - Flags: review?(bobbyholley) → review+
Comment on attachment 8969522 [details]
Bug 1455492: While here, fix refcount churn.

https://reviewboard.mozilla.org/r/238286/#review244240
Attachment #8969522 - Flags: review?(bobbyholley) → review+
Comment on attachment 8969521 [details]
Bug 1455492: Remove redundant ParseSheet arguments.

https://reviewboard.mozilla.org/r/238284/#review244232

> Hm, so this will change from passing nullptr as the principal to having the callee infer it from the sheet, which in this case will be the singleton NullPrincipal from URLExtraData::Dummy AFAICT.
> 
> I _think_ this is ok, but please check if any consumer behavior will change.

Yeah, this is fine, this sheet is not exposed in any way, and we don't use the principal for parsing at all (we only pass it around to images and such). Given this doesn't have images or what not, this shouldn't change anything.
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c007f07170a5
Remove redundant ParseSheet arguments. r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/046d0e259116
While here, fix refcount churn. r=bholley
https://hg.mozilla.org/mozilla-central/rev/c007f07170a5
https://hg.mozilla.org/mozilla-central/rev/046d0e259116
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.