Closed Bug 1448944 Opened 2 years ago Closed 2 years ago

Avoid ContentPrefService roundtrip when loading initial about:blank

Categories

(Firefox :: Tabbed Browser, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: dthayer, Assigned: dthayer)

References

(Blocks 1 open bug)

Details

(Keywords: perf, Whiteboard: [qf:61][qf:p1][fxperf:p1])

Attachments

(2 files)

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.
Keywords: perf
Priority: -- → P3
See Also: → 1397365
Whiteboard: [qf]
Flags: needinfo?(mconley)
Whiteboard: [qf] → [qf:61][qf:p1]
Hey dthayer, after bug 887889 lands, I imagine this bug is pretty simple to fix, right? If so, can you take it?
Flags: needinfo?(mconley) → needinfo?(dothayer)
Whiteboard: [qf:61][qf:p1] → [qf:61][qf:p1][fxperf]
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.
Assignee: nobody → dothayer
Status: NEW → ASSIGNED
Flags: needinfo?(dothayer)
Whiteboard: [qf:61][qf:p1][fxperf] → [qf:61][qf:p1][fxperf:p1]
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.
Attachment #8971708 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8971709 [details]
Bug 1448944 - Move Sqlite.jsm back to stricter startup check

https://reviewboard.mozilla.org/r/240444/#review246224
Attachment #8971709 - Flags: review?(gijskruitbosch+bugs) → review+
Blocks: 1457597
Pushed by dothayer@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/748824210490
https://hg.mozilla.org/mozilla-central/rev/c5751928dac6
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
You need to log in before you can comment on or make changes to this bug.