811570 - Gmail has an enormous component alpha layer

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Matt Woodrow (:mattwoodrow)]

Attachment: Make overflow:hidden documents inactive initially

Gmail has a position:fixed white div between the background and content. This is causing us to put the entirety of the content in a component alpha layer.

The attached patch makes the root content scrollframe inactive initially if it is overflow:hidden. If it ever gets scrolled, then we make it active from then on.

Comment 1

[Matt Woodrow (:mattwoodrow)]

https://tbpl.mozilla.org/?tree=Try&rev=98175550e15c

(I also added a comment to the header).

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 681300 [details] [diff] [review]
Make overflow:hidden documents inactive initially

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

::: layout/generic/nsGfxScrollFrame.cpp
@@ +1694,5 @@
> +  // Unless this is the root scrollframe for a non-chrome document 
> +  // which is the direct child of a chrome document, we default to not
> +  // being "active".
> +  // XXX maybe we should extend this so that IFRAMEs which fill the
> +  // entire viewport (like GMail used to!) are always active

Delete this XXX comment since it no longer applies to GMail!

Comment 3

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/24c0c55af0ed

Comment 4

[(dormant account)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #3)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/24c0c55af0ed

fyi, the try build is roughly 2x the perf on gmail tab switch. Good work!

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/24c0c55af0ed

Comment 6

[Matt Woodrow (:mattwoodrow)]

Attachment: Followup - allow the scroll layer to become active again

Looks like this made overflow:hidden pages end up permanently inactive.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 682271 [details] [diff] [review]
Followup -  allow the scroll layer to become active again

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

I think the correct fix here is simply to have nsGfxScrollFrameInner::MarkActive unconditionally set mScrollingActive = true; before calling IsAlwaysActive.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 682271 [details] [diff] [review]
Followup -  allow the scroll layer to become active again

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

Sorry, I meant to r- that

Comment 9

[Matt Woodrow (:mattwoodrow)]

Attachment: Followup - allow the scroll layer to become active again v2

Comment 10

[Alfred Kayser]

Marked as fixed, but there is a followup not committed yet?

Comment 11

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3a057b8667fb

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3a057b8667fb

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 681300 [details] [diff] [review]
Make overflow:hidden documents inactive initially

[Approval Request Comment]
Bug caused by (feature/regressing bug #): None
User impact if declined: GMail considerably slower than necessary for six weeks
Testing completed (on m-c, etc.): 10 days on m-c
Risk to taking this patch (and alternatives if risky): Pretty low risk. Simple patches and we're just messing with some performance heuristics.
String or UUID changes made by this patch: None.

The followup patch in this bug needs approval as well.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 681300 [details] [diff] [review]
Make overflow:hidden documents inactive initially

Oops, this is already on Aurora since it landed before the uplift.