1444336 - Make NotifyPaintEvent and its attributes not chrome only

Status: RESOLVED WONTFIX

(Core :: DOM: Events, defect, P2)

Description

[Xidorn Quan [:xidorn] UTC+10]

MozAfterPaint event is not dispatched to content by default, and it only does so when "dom.send_after_paint_to_content" is set to true manually, so changing its availability to content shouldn't cause web compat issue.

Hiding it behind ChromeOnly does have a disadvantage that, it creates a barrier for Firefox / Gecko developers who want to use MozAfterPaint event to measure performance.

For example, tabpaint test reads boundingClientRect from the event, [1] which cannot be done outside chrome at the moment. That makes it harder to profile this test outside talos harness.


[1] https://searchfox.org/mozilla-central/rev/588d8120aa11738657da93e09a03378bcd1ba8ec/testing/talos/talos/tests/tabpaint/content/target.html#23

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1444336 - Make NotifyPaintEvent not ChromeOnly.

Review commit: https://reviewboard.mozilla.org/r/226404/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/226404/

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8957501 [details]
Bug 1444336 - Make NotifyPaintEvent not ChromeOnly.

https://reviewboard.mozilla.org/r/226404/#review232466

This will fail test_interfaces.html, because you are exposing the interface object unconditionally.

For the rest, we had chromeonly checks on the various accessors on this event so that if a user toggles the pref they don't create a security/privacy hole.  I would like to understand exactly what the proposed security/privacy story is here.

Comment 3

[Boris Zbarsky [:bzbarsky]]

And in particular, see bug 1334957 comment 14 for why a user may end up with the pref toggled.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #3)
> And in particular, see bug 1334957 comment 14 for why a user may end up with
> the pref toggled.

We have dropped support for legacy addons since 56, don't we? And I don't think Web Extensions are allowed to flip arbitrary pref like this.

There may be some users who previously use those extensions that have switched the pref toggled, but I don't think there should be a lot.

And what I don't understand is

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #2)
> For the rest, we had chromeonly checks on the various accessors on this
> event so that if a user toggles the pref they don't create a
> security/privacy hole.  I would like to understand exactly what the proposed
> security/privacy story is here.

what security/privacy hole would this create?

For clientRects and boundingClientRect I guess the concern is that they may expose privacy issue related to :visited? If they do, I believe MozAfterPaint event itself should have already been enough to expose it. It may make the detection to have a higher bandwidth if applicable.

Given the information from PaintRequest, I guess it is similiar to paintRequests.

The transactionId isn't interesting for any security/privacy concern I suppose. It is just a number created sequentially indicating how many repaints have happened.

paintTimeStamp may be used for various rendering performance detection, but that can already be done with various performance measurement API I believe.

So I don't really think this change has such concern. The bigger concern is MozAfterPaint itself. If we think that legacy extensions' leftover is a concern here, we can probably rename this pref so that we reset them for all the users.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> And I don't think Web Extensions are allowed to flip arbitrary pref like this.

OK, good.

> but I don't think there should be a lot.

But we still shouldn't expose them to privacy holes.

> what security/privacy hole would this create?

Last I checked you got information about what's going on painting-wise in cross-origin iframes, no?

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #5)
> > what security/privacy hole would this create?
> 
> Last I checked you got information about what's going on painting-wise in
> cross-origin iframes, no?

Isn't the dispatching of MozAfterPaint itself already leaking some information in cross-origin iframes?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Some, yes.  Not nearly as fine-grained as being able to get the various geometry bits from the event.

Comment 8

[Boris Zbarsky [:bzbarsky]]

We could also consider restricting this stuff to the "in automation" case, but it sounds like the problem is exactly that people want to use this outside automation?

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

Kind of yes. Actually it's me want to profile tabpaint outside the talos harness. I guess we can probably have the whole non-chrome MozAfterPaint behind an environment or something like that rather than a pref, if that solves your concern.

Comment 10

[Xidorn Quan [:xidorn] UTC+10]

environment variable, I mean.