1448944 - Avoid ContentPrefService roundtrip when loading initial about:blank

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, enhancement, P3)

Description

[Alex Thayer [:alexical] (she/her)]

Since the intial about:blank page is completely empty, zooming it makes no sense, and going through the content prefs causes us to (once bug 887889 lands) load Sqlite.jsm. It would be ideal if we could just avoid this call entirely.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Hey dthayer, after bug 887889 lands, I imagine this bug is pretty simple to fix, right? If so, can you take it?

Comment 2

[Alex Thayer [:alexical] (she/her)]

Somewhat, yeah. There's a decent chunk of tests that rely on this weird behavior, but I *think* they should be simple to fix. But yeah I'll grab this for now at least.

Comment 3

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1448944 - Avoid cps2 roundtrip for null principal about:blank

Review commit: https://reviewboard.mozilla.org/r/240442/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/240442/

Comment 4

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1448944 - Move Sqlite.jsm back to stricter startup check

Review commit: https://reviewboard.mozilla.org/r/240444/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/240444/

Comment 5

[:Gijs (out for now; he/him)]

Comment on attachment 8971708 [details]
Bug 1448944 - Avoid cps2 roundtrip for null principal about:blank

https://reviewboard.mozilla.org/r/240442/#review246220

::: browser/base/content/browser-fullZoom.js:221
(Diff revision 1)
> -      this._applyPrefToZoom(undefined, browser,
> +        this._applyPrefToZoom(undefined, browser,
> -                            this._notifyOnLocationChange.bind(this, browser));
> +                              this._notifyOnLocationChange.bind(this, browser));

Can you file YAF (yet another follow-up) to make us use the principal's origin for this lookup if there's an http/https codebase principal for the about:blank document? This is a pre-existing bug, and probably doesn't affect a lot of places, but purely in principle this is a bug. :-)

Then again, really these APIs should probably be using a principal instead of a URI anyway... 

Anyway, adding a comment about this here, referencing the bug, may make the code slightly easier to understand in future.

Comment 6

[:Gijs (out for now; he/him)]

Comment on attachment 8971709 [details]
Bug 1448944 - Move Sqlite.jsm back to stricter startup check

https://reviewboard.mozilla.org/r/240444/#review246224

Comment 7

[Alex Thayer [:alexical] (she/her)]

Comment on attachment 8971708 [details]
Bug 1448944 - Avoid cps2 roundtrip for null principal about:blank

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/240442/diff/1-2/

Comment 8

[Alex Thayer [:alexical] (she/her)]

Comment on attachment 8971709 [details]
Bug 1448944 - Move Sqlite.jsm back to stricter startup check

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/240444/diff/1-2/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/748824210490
Avoid cps2 roundtrip for null principal about:blank r=Gijs
https://hg.mozilla.org/integration/autoland/rev/c5751928dac6
Move Sqlite.jsm back to stricter startup check r=Gijs

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/748824210490
https://hg.mozilla.org/mozilla-central/rev/c5751928dac6