1795804 - Delay when maximizing browser window

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Blake]

Attachment: 2022-10-17 20-39-44.mkv

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:108.0) Gecko/20100101 Firefox/108.0

Steps to reproduce:

When double clicking the title bar, there is a slight delay before the tabs move to the far left

Comment 1

[Blake]

Mozregression points to 1789823

Comment 2

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Firefox::Tabbed Browser' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[BugBot [:suhaib / :marco]]

:emilio, since you are the author of the regressor, bug 1789823, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

That's a bit surprising, specially because only https://hg.mozilla.org/integration/autoland/rev/c9785cd100fc is potentially relevant to windows...

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1795804 - Don't use lock to set persistent attribute timer. r=smaug

Drive-by, but this timer is main-thread only, I have no idea why it'd
need a lock.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1795804 - Use atom version of attributes in persistence code when possible. r=smaug

Also drive-by, but it's faster.

Depends on D159576

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1795804 - Centralize persistent attribute update code. r=smaug

This patch doesn't change behavior by itself, but it makes the
persistent attribute code a bit clearer by making it use typed enums
rather than raw integers for flags. Also centralizes the update.

Depends on D159577

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1795804 - Persist sizemode synchronously. r=smaug

Much like everything else. This is important because the front-end uses
sizemode for styling.

The comment I'm removing doesn't really make much sense, since sizemode
and size are persisted separately (using different dirty flags) so it's
not like we're saving duplicate work or something.

The delay this fixes existed already on Linux since ~forever at least,
haven't tested macOS.

Depends on D159578

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9bf0d3022af
Don't use lock to set persistent attribute timer. r=smaug
https://hg.mozilla.org/integration/autoland/rev/9df50f3e64f5
Use atom version of attributes in persistence code when possible. r=smaug
https://hg.mozilla.org/integration/autoland/rev/643aaf8f2efd
Centralize persistent attribute update code. r=smaug
https://hg.mozilla.org/integration/autoland/rev/d2e0da08b38f
Persist sizemode synchronously. r=smaug

Comment 10

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1789823

Comment 11

[Sandor Molnar[:smolnar]]

Backed out for causing assertion failures in widget/cocoa/nsCocoaWindow.mm

Backout link: https://hg.mozilla.org/integration/autoland/rev/a84adce7fced223b632c6faebc84134472614a58

Push with failures

Failure log

INFO - GECKO(1871) | [Parent 1871, Main Thread] ###!!! ASSERTION: mBounds out of sync!: 'mWindow && mBounds == r', file /builds/worker/checkouts/gecko/widget/cocoa/nsCocoaWindow.mm:1851

Comment 12

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1795804 - Persist only what's requested sync. r=smaug

This fixes issues like the one that got me backed out, where we have a
dirty size request, and we want to synchronously persist sizemode.

With this patch, we'd persist sizemode sync, but size would still be
updated async, preserving the previous size behavior.

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/f9bf0d3022af
https://hg.mozilla.org/mozilla-central/rev/9df50f3e64f5
https://hg.mozilla.org/mozilla-central/rev/643aaf8f2efd

Comment 14

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox107 to wontfix.

For more information, please visit auto_nag documentation.

Comment 15

[Emilio Cobos Álvarez [:emilio]]

There's still more patches to land, the bug isn't closed.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e8f88bcb7a4
Persist only what's requested sync. r=smaug

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/491ac50c7721
Persist sizemode synchronously. r=smaug

Comment 18

[Marian-Vasile Laza]

Backed out changeset 491ac50c7721 (Bug 1795804) for causing mochitest failures on test_maximized_persist.xhtml.
Backout link
Push with failures <--> c3
Failure Log

Comment 19

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1795804 - Move code around a little bit to avoid calling GetRestoredBounds when not using them. r=smaug

This actually avoids the assertion that got me backed out, sigh.

Comment 20

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/6e8f88bcb7a4

Comment 21

[Emilio Cobos Álvarez [:emilio]]

sigh

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d5887b709cf4
Move code around a little bit to avoid calling GetRestoredBounds when not using them. r=smaug

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47e9c3d582b3
Persist sizemode synchronously. r=smaug

Comment 24

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/d5887b709cf4
https://hg.mozilla.org/mozilla-central/rev/47e9c3d582b3

Comment 25

[Bogdan Maris, Desktop Test Engineering]

Attachment: Screencast showing the still existing delay on mac for overflow tab arrows

Reproduced the initial issue here using old Nightly build 2022-10-17, I verified that using latest beta build Firefox 108.0b5 the tabs transition is now smooth on Windows and Ubuntu when having both a few tabs or loaded with bunch of tabs.

I noticed that on macOS when having enough tabs to trigger the overflow tabs arrows when having the browser not in full screen and switching to full screen there is still a small delay for the arrows to be dismissed from the tabs strip (the arrows must go away because there are not enough tabs to trigger them when in full screen but enough when out of full screen). I captured the delay in a video, it is a bit glitchy though, hope it captures the delay I see live.
Let me know if you want me to log a new bug on this or if this delay is acceptable.

Comment 26

[Emilio Cobos Álvarez [:emilio]]

I don't think it's a huge issue but can you file a new bug about that in any case? I'm curious to know if it's a regression. Thanks!

Comment 27

[Bogdan Maris, Desktop Test Engineering]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #26)

I don't think it's a huge issue but can you file a new bug about that in any case? I'm curious to know if it's a regression. Thanks!

I logged bug 1802165 for that issue (I've added you to CC on it), it is not a regression since even 72.0a1 had that particular behavior.

Will go ahead and close this bug as verified fixed based on the above.