Figure out how we can use MozAfterPaint.paintTimeStamp wherever we're using MozAfterPaint to measure paint times
Categories
(Testing :: Talos, enhancement, P5)
Tracking
(Not tracked)
People
(Reporter: jwatt, Unassigned)
Details
MozAfterPaint.paintTimeStamp was added in bug 1264798 back in January 2017. It was then made ChromeOnly in bug 1334957 in February 2017. I'm not sure what the best way forward to being able to use paintTimeStamp in Talos tests is. Since Firefox 57 (when add-on support was dropped) was released at the end of 2017, maybe we don't need to worry about add-ons flipping the pref to expose MozAfterPaint to content and creating a security issue any more?
Comment 1•6 years ago
|
||
I am not sure of what is actionable here- :jwatt, is this something you need to investigate, or is there something I can do to help out?
Reporter | ||
Comment 2•6 years ago
|
||
Perhaps the best thing to do is needinfo bz, but I was reluctant to do that until I'd found the time to go through bug 1264798 and understand if we can back that out, or something, so we can start using paintTimeStamp in Talos. It's not a top priority for me yet though.
Comment 3•6 years ago
|
||
We made things chromeonly in bug 1334957 on general principle and due to lack of use cases. If we have a use case we should evaluate accordingly. It would not be hard to expose this property based on a pref or IsInAutomation() or a combination of the two.
Comment 4•5 years ago
|
||
:jwatt is this still something we're interested in? Do you need any assistance?
Reporter | ||
Comment 5•5 years ago
|
||
What I was thinking about at the time was that the way that we measure times could be more accurate using this method (reduce noise). However, I'm not thinking about that right now so I'm going to defer to the performance team who hopefully have been/will look into this. Markus, any opinions, or would anyone else on your team be more appropriate?
Updated•3 years ago
|
Updated•3 years ago
|
Comment 7•3 years ago
|
||
(In reply to Kimberly Sereduck :kimberlythegeek from comment #6)
:davehunt Is this this valid?
It sounds like it's still a potential improvement to our Talos measurements. I'll leave the needinfo for Markus and ping him on Matrix, but I think it's realistic to change this to P5. We'd accept a patch but unlikely to work on this, pending Markus' thoughts.
Comment 8•3 years ago
|
||
I agree with P5. I went through this list to check which Talos tests use MozAfterPaint, and it looks like all the important paint-based tests (twinopen, tresize, a few more) already use the paintTimeStamp.
The pageloader tests in tpmozafterpaint mode don't use paintTimeStamp. They probably could, but I'm not sure it's worth the trouble. Here are the locations in the code where we'd need to take the value into account:
https://searchfox.org/mozilla-central/rev/69babd862de70cabfa1d0a369d38e4881bd41e4d/testing/talos/talos/pageloader/chrome/lh_moz.js#11
https://searchfox.org/mozilla-central/rev/69babd862de70cabfa1d0a369d38e4881bd41e4d/testing/talos/talos/pageloader/chrome/pageloader.js#702
https://searchfox.org/mozilla-central/rev/69babd862de70cabfa1d0a369d38e4881bd41e4d/testing/talos/talos/pageloader/chrome/pageloader.js#763
Description
•