972099 - A preference gfx.color_management.force_srgb needs to move from .js to .py

Status: REOPENED

(Testing :: Reftest, defect)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

Even though it isn't a default, we want to run the tests with gfx.color_management.force_srgb set to true.  Right now, this is accomplished by setting this after the startup.  However, changing that preference after the startup is not safe in general, so we are going to make it "read only on startup" type of pref (see bug 971126).  In order to continue testing with gfx.color_management.force_srgb set to true, even though the default is false, my understanding is that we need to make a change to runreftest.py or some such.

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

Andrew, does the above description make sense, and is runreftest.py the right place for this change?  If not, can you suggest the right approach, or point us to somebody else? Thanks.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'd have hoped that the pref setting in reftest-cmdline.js happens before the pref gets read in the first place.  Are you sure this is needed?

Comment 3

[Andrew Halberstadt [:ahal]]

(In reply to Milan Sreckovic [:milan] from comment #1)
> Andrew, does the above description make sense, and is runreftest.py the
> right place for this change?  If not, can you suggest the right approach, or
> point us to somebody else? Thanks.

Yeah, that makes sense. You can add the pref here, where it will be set as a user_pref in the profile:
http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/runreftestb2g.py#412

(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #2)
> I'd have hoped that the pref setting in reftest-cmdline.js happens before
> the pref gets read in the first place.  Are you sure this is needed?

We can't use reftest-cmdline in b2g because we are starting the emulator binary instead of a gecko binary. The emulator is what starts the gecko process and we don't have control over the command line we can pass it. So instead we kill the b2g process after it has been started, swap in a new profile and restart it manually. There's a bug somewhere to abstract the prefs out of reftest-cmdline and share them with b2g/android, but as of now they are just copied :(

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #2)
> I'd have hoped that the pref setting in reftest-cmdline.js happens before
> the pref gets read in the first place.  Are you sure this is needed?

Sadly, it seems to be. If I register this pref with a callback for a "modified value", the callback happens and the value is modified.

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

Note that, in this particular case, the issue is only relevant on Windows.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Move gfx.color_management.force_srgb from .js to .py

Andrew, I can't tell from the conversation if this is what you're suggesting needs to be done.

Comment 7

[Andrew Halberstadt [:ahal]]

Oh sorry, I didn't realize this was Windows only. I agree with :dbaron then, I'm confused as to why the pref in reftest-cmdline.js isn't being set at startup.

In this case I don't think you want to touch any of the b2g/android files (i.e runtestsb2g, b2g_start_script, and bootstrap). Did you mean to do that (I assume I just confused you with comment 3)?

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

Right - I made the change in there because that preference was being set on b2g - even though it is basically ignored there.  So, for consistency.
I definitely don't know this code well enough to understand why it isn't being set at startup.  I can investigate, but that'll take a while.
Does it make sense to just make the change to runreftest.py (as above) and land that now to unblock another bug and open a bug to figure out why this is happening?

Comment 9

[Andrew Halberstadt [:ahal]]

Comment on attachment 8375558 [details] [diff] [review]
Move gfx.color_management.force_srgb from .js to .py

Review of attachment 8375558 [details] [diff] [review]:
-----------------------------------------------------------------

If this fixes a problem for you, I don't mind landing this. Might want to run it passed :dbaron though..

Comment 10

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Set gfx.color_management.force_srgb preference value in runreftest.py. Carry r=ahalberstadt

David, thoughts?

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8375777 [details] [diff] [review]
Set gfx.color_management.force_srgb preference value in runreftest.py.   Carry r=ahalberstadt

Maybe put this below the extraPrefs handling, but otherwise seems fine.

Comment 12

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Set the value for preference gfx.color_management.force_srgb in the profile. Carry r=ahalberstadt, f=dbaron

Comment 13

[Milan Sreckovic [:milan] (needinfo for best results)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa631f68a4fe

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/aa631f68a4fe

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Backed out for causing bug 951139 to spike. CCing Ben to the bug to explain the reasoning behind it.
https://hg.mozilla.org/integration/mozilla-inbound/rev/e42874936c10

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/e42874936c10