Closed Bug 977833 Opened 9 years ago Closed 9 years ago

gfxPrefs not initialized for GTest runs

Categories

(Testing :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla30

People

(Reporter: milan, Assigned: milan)

References

Details

Attachments

(1 file, 1 obsolete file)

Initialize gfxPrefs for the test runs to avoid the assert of initializing it at the first preference access.
Or, should we initialize this in each individual test? With bug 971943, ASync... test needs it.
Assignee: nobody → milan
Attachment #8383295 - Flags: review?(bgirard)
Comment on attachment 8383295 [details] [diff] [review]
Initialize gfxPrefs in GTestRunner

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

::: testing/gtest/mozilla/GTestRunner.cpp
@@ +84,5 @@
>  
>    PR_SetEnv("XPCOM_DEBUG_BREAK=stack-and-abort");
>  
>    ScopedXPCOM xpcom("AsyncPanZoomController");
> +  gfxPrefs::One();

IMO 'One' isn't a good name for 'GetSingleton'. I'd like to see a better name for this function because it makes the call site pretty bad.

Ideally the tests would be able to setup and tear down the state they need to run which would include gfxPrefs. I came imagine this is very impracticable so this is ok.
Attachment #8383295 - Flags: review?(bgirard) → review+
(In reply to Milan Sreckovic [:milan] from comment #1)
> Created attachment 8383295 [details] [diff] [review]
> Initialize gfxPrefs in GTestRunner
> 
> Or, should we initialize this in each individual test? With bug 971943,
> ASync... test needs it.

Ideally yes, but then we'd need to support tear down code for this which is sucky.
Instead of the global approach, here's the one that does it in each test that needs it.  Less intrusive.
Attachment #8383295 - Attachment is obsolete: true
Attachment #8383784 - Flags: review?(bgirard)
(In reply to Benoit Girard (:BenWa) from comment #3)
> ...
> IMO 'One' isn't a good name for 'GetSingleton'. I'd like to see a better
> name for this function because it makes the call site pretty bad.

Fair enough.  I'll make that change once I clear a patch queue with related changes.
Attachment #8383784 - Flags: review?(bgirard) → review+
https://hg.mozilla.org/mozilla-central/rev/6111f616591a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.