1009786 - Unify the concepts "has scrollable layer with displayport" and "is actively scrolled"

Status: ASSIGNED

(Core :: Layout, defect)

Description

[Markus Stange [:mstange]]

In the pre-APZ world, scrolling a scrollable frame creates a scrollable layer for that frame, and that layer stays separate until ScrollFrameHelper::MarkInactive() sets mScrollingActive to false after a timeout. Some frames are never considered inactive, for example the root scroll frame of a root content document, or frames with will-change: scroll-position.
In the APZ world, scroll frames get a scrollable layer if a display port was set on them. IIRC a display port is set once and will never go away for a frame.

These concepts are very similar and should be unified. The result would have these properties:
 - With APZ on desktop, we should always build a scrollable layer with a display port for the root scrollframe of a root content document (which, from APZ's perspective, is a subframe).
 - Display ports for temporarily scrolled subframes should be discarded after a timeout.
 - will-change:scroll-position should cause display ports.
 - If the root frame is not scrollable, and there's a scrollable subframe, that subframe should have mScrollingActive == true even without APZ. Or in other words, the solution for bug 982141 should be extended to cover bug 945367.

Comment 1

[Botond Ballo [:botond]]

Sounds like a good idea.

It's possible that the changes in bug 967844 will make this easier to implement, but I'm not sure.

Comment 2

[Markus Stange [:mstange]]

Attachment: v1

Comment 3

[Timothy Nikkel (:tnikkel)]

Sorry for not having this review done by now. I should get to it soon.

Comment 4

[Markus Stange [:mstange]]

Timothy, any news here?

Comment 5

[Markus Stange [:mstange]]

Comment on attachment 8437773 [details] [diff] [review]
v1

I need to update this patch after multi-layer-apz.

Comment 6

[Markus Stange [:mstange]]

Attachment: v1

Comment 7

[Timothy Nikkel (:tnikkel)]

Some issues to check I discussed with Markus.

We might want the timeout for removing a displayport to be much larger than what we currently have for the current active scroll frame timeout.

We need to make sure everything can handle a displayport being removed as we've never done that before.

Some callers of IsAlwaysActive might not expect the scroll frame to become inactive (if mIsFirstScrollableFrameInWindow becomes false on a later paint).

Comment 8

[Markus Stange [:mstange]]

Lots of B2G reftest failures: https://tbpl.mozilla.org/?tree=Try&rev=4384d7d0c8c1

Comment 9

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8494800 [details] [diff] [review]
v1

Clearing review for now based on the failures in comment 8, since those will need to be fixed before this can land. It's odd this would cause reftest failures.

I'm fine with this patch other than potentially any actual problems from the things mentioned in comment 7.