1371332 - Rename MozAfterPaint to something closer to what it actually represents

Status: NEW

(Core :: Web Painting, enhancement, P3)

Description

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

I hit this in bug 1369662, and I think ehsan is hitting something similar tackling bug 1343728.

We seem to have this case where MozAfterPaint can fire even though nothing has technically been painted. If this is deliberate, we should probably understand the cases where this occurs and update our documentation of the event. If this is not deliberate, we should try to remove those cases, I think.

Until I saw this "empty rect" MozAfterPaint event in bug 1369662, I was under the assumption that MozAfterPaint meant that something had been painted and forwarded to the compositor. At least for the case that this bug is about, that appears to not 100% be true all of the time.

Comment 1

[(no longer active)]

As far as I know:

  * MozAfterPaint means a composite happened in the parent process, not that a paint happened in the child process(!)
  * The invalidated rects attached to the event object allow you to check the invalidated regions that triggered the composite, and those are the invalidated regions that were repainted
  * There is nothing to ensure that a composite happened as a result of some invalidated region actually getting repainted, so the rects can indeed be empty.

Comment 2

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

Okay, in that case, it sounds like this isn't a bug so much as a really poorly named event. Maybe the event name made sense back in the day, but hoooo boy it sure doesn't now.

I understand that some add-ons might still use MozAfterPaint. Once 57 ships, we should just rename this thing.

MozAfterVsync perhaps?

Comment 3

[Matt Woodrow (:mattwoodrow)]

Bit late here, but are you using isMozAfterPaintPending?

We return true from that if we've got a paint scheduled, but until it actually happens we don't know if the thing that scheduled the paint actually causes an invalidation (maybe it's offscreen!).

Once we've promised to a caller that MozAfterPaint is pending, we don't have any way to revoke that, so we instead fire MozAfterPaint with an empty rect [1].

These APIs were written when we did invalidation synchronously, so we knew in advance if a pending paint was going to draw anything or not. What we have now is a bit of a compromise so that existing code still basically works the same way.

The best long term solution might be to try come up with a new API that reflects reality better, and start transitioning code to using that.


[1] https://searchfox.org/mozilla-central/source/layout/base/nsPresContext.cpp#189

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

> The best long term solution might be to try come up with a new API that reflects reality better, and start transitioning code to using that.


Any chance you mean something like bug 1388157?

Comment 5

[Matt Woodrow (:mattwoodrow)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #4)
> > The best long term solution might be to try come up with a new API that reflects reality better, and start transitioning code to using that.
> 
> 
> Any chance you mean something like bug 1388157?

Those are for the first paint on a document load, MozAfterPaint is used by tests that want to know when their current changes (on a loaded document) have made it to the screen.

I was thinking of a promise version of isMozAfterPaintPending/MozAfterPaint so that we can notify callers that a paint is no longer pending.