Closed Bug 634063 Opened 11 years ago Closed 6 years ago

pref layers.acceleration.force-enabled is queried directly in multiple places

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: bjacob, Assigned: milan)

Details

Attachments

(1 file, 2 obsolete files)

http://mxr.mozilla.org/mozilla-central/search?string=acceleration.force-enabled&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

/widget/src/windows/nsWindow.cpp (View Hg log or Hg annotations)

    line 3333 -- prefs->GetBoolPref("layers.acceleration.force-enabled",

/widget/src/xpwidgets/nsBaseWidget.cpp (View Hg log or Hg annotations)

    line 828 -- prefs->GetBoolPref("layers.acceleration.force-enabled",

/gfx/layers/d3d9/LayerManagerD3D9.cpp (View Hg log or Hg annotations)

    line 79 -- prefs->GetBoolPref("layers.acceleration.force-enabled",


Is that really necessary? Couldn't this kind of redundancy slow down our startup? CC'ing a couple of people.
QA Contact: thebes → bjacob
Assignee: nobody → milan
This may (or may not) be fixed by bug 1115352 (didn't check but part of the point of that bug is to centralize the decision making to one place, which implies reading prefs from that one place).
Summary: pref layers.acceleration.force-enabled is queried at 3 different places on Windows → pref layers.acceleration.force-enabled is queried directly in multiple places
The original places were taken care of, but there was one place remaining in OS X, so instead of closing this and opening a new bug just for that, modifying this bug instead.
Attachment #8688062 - Flags: review?(nical.bugzilla)
Comment on attachment 8688062 [details] [diff] [review]
Use gfxPrefs for layers.acceleration... pref evaluation. r=nical

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

::: toolkit/xre/nsAppRunner.cpp
@@ +4643,5 @@
>  #if !defined(E10S_TESTING_ONLY)
>    // When running tests with 'layers.offmainthreadcomposition.testing.enabled' and
>    // autostart set to true, return enabled.  These tests must be allowed to run
>    // remotely. Otherwise remote isn't allowed in non-nightly builds.
> +  bool testPref = LayersOffMainThreadCompositionTestingEnabled();

I may have a slightly outdated version of the tree, but I'd assume you need to prefix this with "gfxPrefs::". Anyway, as long as it builds.

@@ +4672,5 @@
>    // e10s auto start on mac.
>    if (gBrowserTabsRemoteAutostart) {
> +    // Initialize the graphics prefs, some of the paths need them before
> +    // any other graphics is initialized (e.g., showing the profile chooser.)
> +    gfxPrefs::GetSingleton();

Wouldn't you have to initialize the singleton before fetching LayersOffMainThreadCompositionTestingEnabled, a few lines above, then?
Attachment #8688062 - Attachment is obsolete: true
Attachment #8688062 - Flags: review?(nical.bugzilla)
Comment on attachment 8688507 [details] [diff] [review]
Use gfxPrefs for layers.acceleration... pref evaluation. r=nical

Inlining GetSingleton() to make sure it's initialized.
Attachment #8688507 - Flags: review?(nical.bugzilla)
Attachment #8688507 - Flags: review?(nical.bugzilla) → review+
Hi, i had to back this out again, this conflicts with something that landed on m-c and so during a merge from m-i to m-c it causes

Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ hg merge && hg commit -m "merge mozilla-inbound to mozilla-central a=merge"
merging toolkit/xre/nsAppRunner.cpp
warning: conflicts during merge.
merging toolkit/xre/nsAppRunner.cpp incomplete! (edit conflicts, then use 'hg resolve --mark')
500 files updated, 0 files merged, 3 files removed, 1 files unresolved

i guess this is caused by https://bugzilla.mozilla.org/show_bug.cgi?id=1226487
Flags: needinfo?(milan)
Rebase.
Attachment #8688507 - Attachment is obsolete: true
Flags: needinfo?(milan)
Attachment #8691457 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/5efb9a11196b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.