889611 - Switch to using a vbox instead of a deck for the content-deck

Status: RESOLVED WONTFIX

(Firefox :: Theme, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

The content-deck is currently using a deck, but it may be more performant to use a vbox. Here's a try push, https://tbpl.mozilla.org/?tree=Try&rev=fbec691d9fff

Not sure if we should go through with s/content-deck/content-box/.

Comment 1

[:Gijs (he/him)]

Comment on attachment 770451 [details] [diff] [review]
Patch

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

So yeah, in principle this seems fine to me if it improves things and works. Let's wait for the try push results before committing this, but tentatively, r=me. Might also be worth asking Blair for his opinion? Don't know who originally came up with using a <deck> here.

::: browser/components/customizableui/src/CustomizeMode.jsm
@@ +105,5 @@
>  
>      this._transitioning = true;
>  
>      let customizer = document.getElementById("customization-container");
> +    let browser = document.getElementById("browser");

Do we not have a reference to this lying around from browser.js, on window/window.gBrowser? If not, this is fine, but it'd be nice if we don't need to look this up every time as this transition is already not very snappy.

@@ +106,5 @@
>      this._transitioning = true;
>  
>      let customizer = document.getElementById("customization-container");
> +    let browser = document.getElementById("browser");
> +    browser.hidden = true;

This scares me a lot. AFAIK this will set display: none which will detach the tabbrowser.xml binding, amongst others. Waiting for those try push results, I guess.

Comment 2

[Justin Dolske [:Dolske]]

Is this still relevant to current perf work?

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

This doesn't look like it will get us any perf wins, but I do wonder if there is an optimization that can be made for nsDeckFrame::DoLayout since it calls HideBox for each non-selected pane each time we do layout, which shouldn't be necessary. I'll file a separate bug for that.

Also there is a chance that we could move this customization code to an external XUL file, like extensions.xul is done, and that might get us some wins. I'll file a separate bug for that as well.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Jared Wein [:jaws] (Vacation 8/1 to 8/4) from comment #3)
> This doesn't look like it will get us any perf wins, but I do wonder if
> there is an optimization that can be made for nsDeckFrame::DoLayout since it
> calls HideBox for each non-selected pane each time we do layout, which
> shouldn't be necessary. I'll file a separate bug for that.

Filed as bug 900118.

> Also there is a chance that we could move this customization code to an
> external XUL file, like extensions.xul is done, and that might get us some
> wins. I'll file a separate bug for that as well.

Filed as bug 900122.