If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in Firefox 45

Status

()

Core
Graphics
RESOLVED FIXED
7 years ago
2 years ago

People

(Reporter: bjacob, Assigned: milan)

Tracking

unspecified
mozilla45
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.

Updated

6 years ago
QA Contact: thebes → bjacob
(Assignee)

Updated

3 years ago
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).
(Assignee)

Updated

2 years ago
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
(Assignee)

Comment 2

2 years ago
Created attachment 8688062 [details] [diff] [review]
Use gfxPrefs for layers.acceleration... pref evaluation. r=nical

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)
(Assignee)

Comment 3

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9eaab9798993
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?
(Assignee)

Comment 5

2 years ago
Created attachment 8688507 [details] [diff] [review]
Use gfxPrefs for layers.acceleration... pref evaluation. r=nical
Attachment #8688062 - Attachment is obsolete: true
Attachment #8688062 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 6

2 years ago
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)

Updated

2 years ago
Attachment #8688507 - Flags: review?(nical.bugzilla) → review+
(Assignee)

Comment 7

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d20da8117ab8
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6995479eb770
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 8

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/098f2c2291ef
Keywords: checkin-needed
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)

Comment 10

2 years ago
Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/60e8107592ff
(Assignee)

Comment 11

2 years ago
Created attachment 8691457 [details] [diff] [review]
Use gfxPrefs for layers.acceleration... pref evaluation. Carry r=nical

Rebase.
Attachment #8688507 - Attachment is obsolete: true
Flags: needinfo?(milan)
Attachment #8691457 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 12

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5efb9a11196b
Keywords: checkin-needed

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5efb9a11196b
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.