978274 - gfxPrefs initialization, internal cleanup and preparation for moving gfx.color_management.* to it

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

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

1. With automated tests, gfxPlatform doesn't get called early enough, so we need another place to initialize gfxPrefs.

2. Rename One -> GetSingleton, Destroy to DestroySingleton, Exists to SingletonExists.

3. Remove gfxPlatform.MigratePrefs, which deals with some obsolete gfx.color_management preferences.

Comment 1

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

Attachment: Initialize gfxPrefs earlier, rename some methods, delete MigratePrefs from gfxPlatform

Some of these changes were reviewed or suggested in bug 910860; that one is being delayed for dependencies, but some of those changes aren't really related to that bug so may as well live separately.
https://tbpl.mozilla.org/?tree=Try&rev=e4ab23074ea7

Comment 2

[Benoit Girard (:BenWa)]

Comment on attachment 8384840 [details] [diff] [review]
Initialize gfxPrefs earlier, rename some methods, delete MigratePrefs from gfxPlatform

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

::: gfx/tests/gtest/TestGfxPrefs.cpp
@@ +23,5 @@
> +  gfxPrefs::GetSingleton();
> +  ASSERT_TRUE(gfxPrefs::SingletonExists());
> +
> +  // Live boolean, default false
> +  ASSERT_FALSE(gfxPrefs::CanvasAzureAccelerated());

NIT: It's going to be a bit annoying to have these test regress when there's preference are changed.

Comment 3

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

(In reply to Benoit Girard (:BenWa) from comment #2)
> Comment on attachment 8384840 [details] [diff] [review]
> Initialize gfxPrefs earlier, rename some methods, delete MigratePrefs from
> gfxPlatform
> 
> Review of attachment 8384840 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/tests/gtest/TestGfxPrefs.cpp
> @@ +23,5 @@
> > +  gfxPrefs::GetSingleton();
> > +  ASSERT_TRUE(gfxPrefs::SingletonExists());
> > +
> > +  // Live boolean, default false
> > +  ASSERT_FALSE(gfxPrefs::CanvasAzureAccelerated());
> 
> NIT: It's going to be a bit annoying to have these test regress when there's
> preference are changed.

Agreed, but it actually found a bug (I had the macro setting a different value than the actual preference), so it does have value.  I will add a comment in the test pointing out that if the default for the preference changes, we should just modify the test.

Comment 4

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

(Try: https://tbpl.mozilla.org/?tree=Try&rev=e4ab23074ea7)

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

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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