Closed Bug 1590573 Opened 3 years ago Closed 2 years ago

Unify how we cache fragments with parseXULToFragment in our Custom Elements

Categories

(Toolkit :: XUL Widgets, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: bgrins, Assigned: emmamalysz)

References

(Depends on 3 open bugs)

Details

(Whiteboard: [fxperf:p2])

Attachments

(2 files)

We currently have a few different patterns for caching fragments. At this point we should migrate our chrome custom elements to use the same one.

A bunch of examples can be found from callers https://searchfox.org/mozilla-central/search?q=parseXULToFragment&case=true&regexp=false, and usually actually happens with a call to importNode or cloneNode:

The latest elements have been doing something like: https://searchfox.org/mozilla-central/rev/8a63fc190b39ed6951abb4aef4a56487a43962bc/toolkit/content/widgets/arrowscrollbox.js#143-150

    get fragment() {
      if (!this.constructor.hasOwnProperty("_fragment")) {
        this.constructor._fragment = MozXULElement.parseXULToFragment(
          this.markup
        );
      }
      return document.importNode(this.constructor._fragment, true);
    }

which is good because it caches once per class and also supports subclasses (using hasOwnProperty instead of just checking this.constructor["_fragment"]).

There are also ideas around caching it per-process and not per window (in a JSM instead of in a Custom Element definition). That may or may not happen here but worth keeping in mind: https://phabricator.services.mozilla.com/D50137#1521553.

Depends on: 1590585
Whiteboard: [fxperf]
Depends on: 1513285
See Also: → 1585837

Given the improvements reported in bug 1590554 comment 7, I suspect this is worth doing and could have a positive impact on startup times.

Whiteboard: [fxperf] → [fxperf:p2]
Assignee: nobody → emalysz
Pushed by rmaries@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6b7c81561ecc
create a unified approach for caching fragments in our Custom Elements r=emilio
Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/70b0b35496a5
create a unified approach for caching fragments in our Custom Elements r=emilio
Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4ccafa875240
Followup: Revert changes on elements that don't use MozXULElement. CLOSED TREE
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
Depends on: 1628645
Depends on: 1628646
Flags: needinfo?(emalysz)
Regressions: 1629376
You need to log in before you can comment on or make changes to this bug.