Closed Bug 1264409 Opened 4 years ago Closed 4 years ago

Make it easier to resolve MozAfterPaint to a particular transaction ID

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: mconley, Assigned: mconley)

References

Details

Attachments

(1 file)

We have a number of Talos tests that use MozAfterPaint to try to detect when things are presented to the user.

Traditionally, the wisdom has been "do a thing that should cause a paint, and the next MozAfterPaint means that it's been shown to the user".

With OMTC, there are opportunities for MozAfterPaint events to be misleading, and for the above nugget of wisdom to not hold true.

One example is if the previous composite completes very close to where the painting occurs. This will enqueue a MozAfterPaint event, and when the parent processes that event, script that's waiting for the MozAfterPaint will see it and assume that's the one that had our painted stuff in it. This is not true - it's the next composite that will contain it.

Here's an example of that occurring in the tps test for non-e10s:

https://cleopatra.io/#report=eb76c61e5ca17981dbcc7f6cdbfa64ab7f01d9f1&filter=%5B%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A54414,%22end%22%3A56333%7D,%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A54475,%22end%22%3A54675%7D%5D&selection=0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,348,349,350,351,352,366,367,368,369,370,371,372,373,374,375,376,377,378,376,379,378,376,379,380,376,379,378,376,379,380,376,379,378,376,379,378,376,379,378,376,379,378,376,379,380,376,379,378,376,379,378,376,379,378,376,379,381,376,373,382,373,374,375,376,383,376,384,376,385,386,376,385,386,376,385,386,376,385,386,376,385,386,376,385,386,376,385,386,376,385,393,394,395,396,397,398,399,400,401,402,403,404,6045,406,407,408,409,410,411,412,413,416,564

At time 54576, script is run which starts a tab switch, and sets a MozAfterPaint event listener. There was an earlier paint at 54575 which is ready to be composited that has nothing to do with our tab switch. At the next composite at 54589, the MozAfterPaint event fires for the earlier paint, and then our script assumes that the change we'd made has been presented.

The problem is that there's really no way of differentiating between MozAfterPaint's.

What'd be ideal is if script is allowed to access the next layer transaction ID, and for the MozAfterPaint event to include the layer transaction it's for in its details.

That way, script that's paying close attention to MozAfterPaint can get the next layer transaction ID, make their changes causing paint, and wait for MozAfterPaint events _until they get one with the matching transaction ID_.
Assignee: nobody → mconley
Hey mrbkap - this mostly a review request for mattwoodrow. However, I need a DOM peer review for the WebIDL change, which is what I'm roping you into this for.
You said that you wanted to be able to access the 'next layer transaction ID', but it looks like the code is returning the previous one.

If you make changes from JS that will require a paint, then the transaction ID that we want to listen for won't be available until after the refresh driver runs.

Don't you want a 'peek transaction id' function that returns the last ID+1?
Attachment #8741495 - Flags: review?(mrbkap) → review+
Comment on attachment 8741495 [details]
MozReview Request: Bug 1264409 - Make last transaction ID available via nsIDOMWindowUtils, and pass transaction ID through MozAfterPaint. r?mattwoodrow

https://reviewboard.mozilla.org/r/46511/#review43213
(In reply to Matt Woodrow (:mattwoodrow) from comment #3)
> You said that you wanted to be able to access the 'next layer transaction
> ID', but it looks like the code is returning the previous one.
> 
> If you make changes from JS that will require a paint, then the transaction
> ID that we want to listen for won't be available until after the refresh
> driver runs.
> 
> Don't you want a 'peek transaction id' function that returns the last ID+1?

I considered it, but considering the fact that we might end up skipping some frames, I figured it was safest to always just have access to the last one that we know was used.

That way, after we do something that should cause a paint, we just need to ensure that a MozAfterPaint fires with a layer transaction id that is greater than the one that was exposed as the last transaction id.

Is my thinking okay there?
Flags: needinfo?(matt.woodrow)
Yeah, I guess that works.
Flags: needinfo?(matt.woodrow)
Attachment #8741495 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8741495 [details]
MozReview Request: Bug 1264409 - Make last transaction ID available via nsIDOMWindowUtils, and pass transaction ID through MozAfterPaint. r?mattwoodrow

https://reviewboard.mozilla.org/r/46511/#review43219
https://hg.mozilla.org/mozilla-central/rev/2f5f51545d0e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.