Closed Bug 1161590 Opened 9 years ago Closed 9 years ago

Create a pref (not in about:config) to allow working around graphics blacklisting for testing purposes

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: u279076, Assigned: milan)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

Milan and I were discussing our graphics regression testruns with Betabreakers today. We realized that some of them might be testing on blacklisted systems since some of their test systems use older hardware and drivers by design. We thought it might be a good idea to provide them a way to work around the blacklist and re-enable features on their systems for testing purposes. 

Milan's idea was to create a hidden pref that disables the blacklist. For security purposes this pref would have to be manually entered in about:config and only work in daily builds (Nightly & Aurora).

Making this a confidential bug since we want to limit exposure of this pref to the public.
Assignee: nobody → milan
OS: Unspecified → All
Hardware: Unspecified → All
We can eventually (perhaps as a part of the meta bug 838845) cover the scenario where we want to ignore/force blocklisting for individual features - this one is the big hammer that just does everything wholesale.
This is not particularly secret, we just don't want people to start using it outside of particular testing functionality we need.  Perhaps it may be worth doing telemetry on this.
Comment on attachment 8602348 [details] [diff] [review]
Option to force or ignore blocklisting. r=kats

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

The gfxPrefs bit looks sane to me but I don't really know the blocklisting code so somebody else should probably look over that and make sure it's sane.

::: widget/GfxInfoBase.cpp
@@ +1009,5 @@
> +  static int32_t blocklistAll = 0;
> +  if (!blocklistAllInit) {
> +    MOZ_ASSERT(gfxPrefs::SingletonExists());
> +    blocklistAll = gfxPrefs::BlocklistAll();
> +    blocklistAllInit = true;

Is there a point to this cache? I thought gfxPrefs already did this caching internally and was cheap to call.
Attachment #8602348 - Flags: review?(bugmail.mozilla) → review?(jmuizelaar)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> Comment on attachment 8602348 [details] [diff] [review]
> Option to force or ignore blocklisting. r=kats
> 
> Review of attachment 8602348 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The gfxPrefs bit looks sane to me but I don't really know the blocklisting
> code so somebody else should probably look over that and make sure it's sane.
> 
> ::: widget/GfxInfoBase.cpp
> @@ +1009,5 @@
> > +  static int32_t blocklistAll = 0;
> > +  if (!blocklistAllInit) {
> > +    MOZ_ASSERT(gfxPrefs::SingletonExists());
> > +    blocklistAll = gfxPrefs::BlocklistAll();
> > +    blocklistAllInit = true;
> 
> Is there a point to this cache? I thought gfxPrefs already did this caching
> internally and was cheap to call.

Valid point; I can probably just remove that whole thing.
(In reply to Milan Sreckovic [:milan] from comment #5)
> ...
> > 
> > ::: widget/GfxInfoBase.cpp
> > @@ +1009,5 @@
> > > +  static int32_t blocklistAll = 0;
> > > +  if (!blocklistAllInit) {
> > > +    MOZ_ASSERT(gfxPrefs::SingletonExists());
> > > +    blocklistAll = gfxPrefs::BlocklistAll();
> > > +    blocklistAllInit = true;
> > 
> > Is there a point to this cache? I thought gfxPrefs already did this caching
> > internally and was cheap to call.
> 
> Valid point; I can probably just remove that whole thing.

Just to document - I originally put this in anticipating there will be "many" places (at least platform specific, and perhaps in more than one place per platform) where we had to check this value.  From what I can tell, it is only needed in one place, so just checking the pref directly should be fine.
Use pref directly, it's only checked in one place in the code.
Attachment #8602348 - Attachment is obsolete: true
Attachment #8602348 - Flags: review?(jmuizelaar)
Attachment #8602746 - Flags: review?(jmuizelaar)
Attachment #8602746 - Flags: review?(jmuizelaar) → review+
https://treeherder.mozilla.org/#/jobs?repo=try&revision=24e2b063e06f - need to initialize graphics preferences so that xpcshell tests can get to them.  Will get a pre-patch for that.
Attachment #8602746 - Attachment description: Option to force or ignore blocklisting. r=jmuizelaar → Part 2. Option to force or ignore blocklisting. r=jmuizelaar
This bug should be opened up.  We have many other secret prefs, and I don't remember ever landing the code for them in a confidential bug.  In fact the only places where I've seen us land code in a confidential bug for have been stuff that we couldn't do publicly because of NDAs.  Milan, do you agree?
Flags: needinfo?(milan)
Attachment #8611353 - Flags: review?(ehsan) → review+
Comment on attachment 8602746 [details] [diff] [review]
Part 2. Option to force or ignore blocklisting. r=jmuizelaar

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

::: gfx/thebes/gfxPrefs.h
@@ +200,5 @@
>    DECL_GFX_PREF(Once, "gfx.android.rgb16.force",               AndroidRGB16Force, bool, false);
>  #if defined(ANDROID)
>    DECL_GFX_PREF(Once, "gfx.apitrace.enabled",                  UseApitrace, bool, false);
>  #endif
> +#if defined(NIGHTLY_BUILD) || defined(MOZ_DEV_EDITION)

Nit: please use RELEASE_BUILD instead.  It should be functionally equivalent to this.
Group: mozilla-employee-confidential
Flags: needinfo?(milan)
Use defined(RELEASE_BUILD) instead.
Attachment #8602746 - Attachment is obsolete: true
Attachment #8612342 - Flags: review+
Summary: Create a hidden pref to allow working around graphics blacklisting for testing purposes → Create a pref (not in about:config) to allow working around graphics blacklisting for testing purposes
https://hg.mozilla.org/mozilla-central/rev/2df087f15d6e
https://hg.mozilla.org/mozilla-central/rev/5bd5921f4b3b
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Depends on: 1171171
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: