1728398 - Opening a new tab causes the current browser to resize

Status: RESOLVED DUPLICATE of bug 1705215

(Firefox :: Tabbed Browser, defect)

Description

[Florian Quèze [:florian]]

See this profile: https://share.firefox.dev/2WJc5Rq

I had the Firefox Profiler displaying a profile, I had the profiler recording, and I pressed Cmd+t to open a new tab. The about:newtab page opened with the bookmarks toolbar visible.

I then captured the profile, and was surprised to see that we triggered a resize event in the content process of the previous tab (the profiler in this case). I noticed because resizing the profiler display takes a while (more than 500ms here).

Comment 1

[Bas Schouten (:bas.schouten)]

We believe there's no real excuse for us to suffer through this resize. It's costly and can make for a pretty bad experience.

Mike, do you have an idea of who could look at this?

Comment 2

[Mike Conley (:mconley) (:⚙️) (PTO June 8 - 12)]

Interesting... it looks like the logic for showing / hiding the bookmarks toolbar is kicked off from location change state updates - but those state updates occur synchronously when switching tabs (and don't wait for the actual tab switch to complete), which I guess causes the content to get pushed down in the previous tab when switching to about:newtab.

So a few things come to mind:

We don't currently have a great way of waiting until a tab becomes visible. We could fire an Event TabVisible event for that in AsyncTabSwitcher and wait on that instead. Gijs, what do you think of that?

Comment 3

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #2)

Interesting... it looks like the logic for showing / hiding the bookmarks toolbar is kicked off from location change state updates - but those state updates occur synchronously when switching tabs (and don't wait for the actual tab switch to complete), which I guess causes the content to get pushed down in the previous tab when switching to about:newtab.

So a few things come to mind:

We don't currently have a great way of waiting until a tab becomes visible. We could fire an Event TabVisible event for that in AsyncTabSwitcher and wait on that instead. Gijs, what do you think of that?

SGTM, might also help with bug 1698221?

That said, we'lll need to keep the onLocationChange handler, but should except the "simulated" changes from it, if we use the TabVisible event for that -- it's possible to navigate an existing tab to the new tab page (e.g. using the "home" button) and it should keep showing/hiding the toolbar for navigations like that.

Comment 4

[:Gijs (he/him)]

So I'm a bit confused... if we wait for the new tab to be visible before we show the toolbar, don't we guarantee asking for a resize in the new tab? Does that make it better?

Comment 5

[Florian Quèze [:florian]]

(In reply to :Gijs (he/him) from comment #4)

So I'm a bit confused... if we wait for the new tab to be visible before we show the toolbar, don't we guarantee asking for a resize in the new tab? Does that make it better?

It would avoid resizing a (potentially) massive document from the previous tab... but it's not great to resize about:newtab all the time either. Ideally we would have the pre-loaded about:newtab at the correct size, and avoid all resizing when opening a new tab. I don't know the details of what should be done when to achieve this.

Comment 6

[Frank Doty [:fdoty]]

The Performance Priority Calculator has determined this bug's performance priority to be P1.

Platforms: [x] Windows [x] macOS [x] Linux
Impact on browser UI: Causes noticeable jank
Impact on site: Causes noticeable jank
[x] Able to reproduce locally

Comment 7

[:Gijs (he/him)]

Is this a dupe of bug 1705215 or is there a difference I'm missing? There's some discussion in that bug as to why it's difficult to address. Note that this bug is marked performance impact high, whereas the other is perf impact medium. This bug is currently the only open high-impact perf bug in the Firefox product.

Comment 8

[Florian Quèze [:florian]]

Yes, looks like a duplicate. And I think performance impact high is correct.