1502431 - Figure out how we can use MozAfterPaint.paintTimeStamp wherever we're using MozAfterPaint to measure paint times

Status: NEW

(Testing :: Talos, enhancement, P5)

Description

[Jonathan Watt [:jwatt]]

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

[Joel Maher ( :jmaher ) (UTC -8)]

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?

Comment 2

[Jonathan Watt [:jwatt]]

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

[Boris Zbarsky [:bzbarsky]]

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

[Dave Hunt [:davehunt] [he/him] ⌚BST]

:jwatt is this still something we're interested in? Do you need any assistance?

Comment 5

[Jonathan Watt [:jwatt]]

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?

Comment 6

[Kimberly Sereduck :kimberlythegeek]

:davehunt Is this this valid?

Comment 7

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(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

[Markus Stange [:mstange]]

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