1341294 - MozAfterPaint handlers should only fire after the next layer transaction by default

Status: RESOLVED DUPLICATE of bug 1302071

(Core :: Graphics, defect, P3)

Description

[Mike Conley (:mconley) (:⚙️)]

I believe the way MozAfterPaint works is that it fires after a composite has occurred for the window that the listener is attached to.

Usually, this is fine - we attach a MozAfterPaint listener, eventually do something that should cause a paint, the layer transaction occurs, the composite occurs, and then our MozAfterPaint listener fires as intended.

That's the expected case. Here's an unexpected case:

Something (X) that we don't care about triggers a paint. A layer transaction occurs for X. Then our MozAfterPaint event listener is attached. We do something that we care about that triggers another paint (Y). A layer transaction occurs for Y. A composite occurs for X, and our MozAfterPaint event listener fires early.

This sort of bug has cropped up before in our Talos tests. I originally worked around this in bug 1264409 by adding a transactionId on the MozAfterPaint event that can be compared against the last transaction ID that was used (see my mailing list post that I sent after landing bug 1264409 here: https://groups.google.com/forum/#!searchin/mozilla.dev.platform/lastTransactionId%7Csort:relevance/mozilla.dev.platform/pCLwWdYc_GY/j9A-vWm3AgAJ).

I think there might be a better solution that puts less onus on the writer of the MozAfterPaint event handler.

Specifically, what I suggest is that when we attach a MozAfterPaint listener, we map that listener to the last transaction ID used for that window. When a composite occurs, and we go through each of the MozAfterPaint listeners, we should only fire the ones for layer transactions that have IDs that are greater than what we'd stored.

Does this sound feasible? Any glaring problems with the idea?

Comment 1

[Matt Woodrow (:mattwoodrow)]

Seems pretty reasonable to me!

We might have tests depending on the old behaviour, but those should be fixed regardless.

Comment 2

[Mantaroh Yoshinaga[:mantaroh]]

(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews and needinfos from comment #0)
> I think there might be a better solution that puts less onus on the writer
> of the MozAfterPaint event handler.
> 
> Specifically, what I suggest is that when we attach a MozAfterPaint
> listener, we map that listener to the last transaction ID used for that
> window. When a composite occurs, and we go through each of the MozAfterPaint
> listeners, we should only fire the ones for layer transactions that have IDs
> that are greater than what we'd stored.
> 

I think this solution has already fixed in bug 1302071[1]. So I marked this as duplicate.

[1] https://hg.mozilla.org/mozilla-central/rev/a535bf3bfff1