Last Comment Bug 608030 - Disable MozAfterPaint for content by default
: Disable MozAfterPaint for content by default
Status: VERIFIED FIXED
[softblocker][fx4-fixed-bugday][sg:low]
: dev-doc-complete
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: P3 normal (vote)
: mozilla2.0b10
Assigned To: David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-28 11:04 PDT by Collin Jackson
Modified: 2011-05-05 00:51 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
betaN+


Attachments
patch (10.10 KB, patch)
2011-01-18 17:05 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
roc: review+
Details | Diff | Splinter Review
pre-patch: clean up reftest pref handling (4.95 KB, patch)
2011-01-20 14:46 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
roc: review+
Details | Diff | Splinter Review
patch (13.41 KB, patch)
2011-01-20 14:47 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
roc: review+
Details | Diff | Splinter Review
honor the force_srgb pref even when it's a default pref (1.91 KB, patch)
2011-01-24 13:24 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
jmuizelaar: review+
Details | Diff | Splinter Review

Description Collin Jackson 2010-10-28 11:04:50 PDT
In bug 557579 comment 6, I pointed out that doing anything conditionally on the visitedness of a link has the potential to leak private history state. Currently the behavior of MozAfterPaint is influenced by the visitedness of links, so this event can be used to detect sites the user has or hasn't been to before.

In bug 557579 comment 9, dbaron suggests disabling MozAfterPaint for content by default, but have a pref that allows people who want to do performance testing with it to enable it.

See also: bug 600025, bug 147777
Comment 1 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-12-23 07:47:39 PST
I should do this after bug 612190 lands, to avoid merge conflicts.
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-01-18 11:43:04 PST
The relevant part of bug 612190 is now landed (see bug 612190 comment 26).
Comment 3 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-01-18 17:05:04 PST
Created attachment 504924 [details] [diff] [review]
patch
Comment 4 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-01-20 14:46:20 PST
Created attachment 505559 [details] [diff] [review]
pre-patch: clean up reftest pref handling

As part of fixing tests for the main patch, I realized I needed to add setting of a new pref to reftest and mochitest.  But before I did that, I decided to clean up the reftest prefs code a bit, so that:
 * it doesn't risk changing the profile, even if interrupted
 * prefs that are needed for reftests to work are in the harness, not the optional runreftest.py
Comment 5 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-01-20 14:47:05 PST
Created attachment 505560 [details] [diff] [review]
patch

With some additional test changes so that we pass tests.

It seemed easier to just flip the pref by default for our tests than to make the individual tests flip it.
Comment 6 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-01-23 20:24:37 PST
http://hg.mozilla.org/mozilla-central/rev/ffebdc3ddb62
http://hg.mozilla.org/mozilla-central/rev/3248feddc867
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-01-23 22:01:43 PST
Backed out:
http://hg.mozilla.org/mozilla-central/rev/f5cf80c6dae4
http://hg.mozilla.org/mozilla-central/rev/80266029824b
Comment 8 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-01-24 13:24:04 PST
Created attachment 506495 [details] [diff] [review]
honor the force_srgb pref even when it's a default pref

This fixes some highly unusual use of the pref API.  I simplified the reftest pref-setting code by making it set the prefs as default prefs (which avoids having to unset them), but the code reading the force_srgb pref was explicitly checking that the pref was a user-set and not a default pref.  It shouldn't do that.

The pref API exposes "unset" by throwing from GetBoolPref (etc.), so the NS_SUCCEEEDED(rv) check on the result of GetBoolPref is sufficient here.

This code originated in http://hg.mozilla.org/mozilla-central/rev/4b2977a03aba
Comment 11 Colby Russell :crussell (no longer Mozilla-ing) 2011-01-29 23:38:25 PST
From what I gather, this bug is about not sending MozAfterPaint for (to) content, but the docs just say it's not sent (at all, as I take it).

Does MozAfterPaint still work for trusted listeners, or no?
Comment 12 Eric Shepherd [:sheppy] 2011-01-31 13:27:13 PST
(In reply to comment #11)
> From what I gather, this bug is about not sending MozAfterPaint for (to)
> content, but the docs just say it's not sent (at all, as I take it).
> 
> Does MozAfterPaint still work for trusted listeners, or no?

You're right; I've clarified that. Nice catch, thank you.
Comment 13 Alexey Perepyolkin [:chaos] 2011-02-04 12:59:22 PST
Verified with Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre

Note You need to log in before you can comment on or make changes to this bug.