Open Bug 972099 Opened 6 years ago Updated Last year

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

Categories

(Testing :: Reftest, defect)

x86
Windows 7
defect
Not set

Tracking

(Not tracked)

REOPENED

People

(Reporter: milan, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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.
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.
Blocks: 971126
Flags: needinfo?(ahalberstadt)
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?
(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 :(
Flags: needinfo?(ahalberstadt)
(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.
OS: Mac OS X → Windows 7
Note that, in this particular case, the issue is only relevant on Windows.
Andrew, I can't tell from the conversation if this is what you're suggesting needs to be done.
Attachment #8375558 - Flags: review?(ahalberstadt)
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)?
Flags: needinfo?(milan)
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?
Flags: needinfo?(milan) → needinfo?(ahalberstadt)
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..
Attachment #8375558 - Flags: review?(ahalberstadt) → review+
Flags: needinfo?(ahalberstadt)
David, thoughts?
Attachment #8375558 - Attachment is obsolete: true
Attachment #8375777 - Flags: review+
Attachment #8375777 - Flags: feedback?(dbaron)
Flags: needinfo?(dbaron)
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.
Attachment #8375777 - Flags: feedback?(dbaron) → feedback+
Flags: needinfo?(dbaron)
Assignee: nobody → milan
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/aa631f68a4fe
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Depends on: 951139
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla30 → ---
No longer blocks: 910860
Assignee: milaninbugzilla → nobody
You need to log in before you can comment on or make changes to this bug.