Closed Bug 1089008 Opened 6 years ago Closed 6 years ago

enable e10s on windows even without hardware acceleration

Categories

(Core :: Graphics: Layers, defect)

All
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
e10s m4+ ---

People

(Reporter: zombie, Assigned: jimm)

References

Details

Attachments

(1 file, 3 obsolete files)

i believe turning off e10s when hardware acceleration is off on windows in bug 1068199 was overkill.

the reported crashes were all on OSX. several people (including me) showed up in the #e10s channel asking why e10s was disabled for them (it's also hard to find out why).
tracking-e10s: --- → ?
Assignee: nobody → dtownsend+bugmail
Depends on: 1095559
No longer depends on: 1095559
Assignee: dtownsend+bugmail → jmathies
URL: :billm
Gfx assures me that the compositor is up and running in all cases on Windows, even when acceleration is off. I don't remember the exact discussion we had over bug 1068199 and why Windows was disabled, but I don't know of any reasons to keep it that way at this point.
Attached patch patch (obsolete) — Splinter Review
Attachment #8521628 - Flags: review?(dtownsend+bugmail)
Comment on attachment 8521628 [details] [diff] [review]
patch

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

::: toolkit/xre/nsAppRunner.cpp
@@ +4634,3 @@
>          for (unsigned int idx = 0; idx < ArrayLength(flagsToCheck); idx++) {
>            if (NS_SUCCEEDED(gfxInfo->GetFeatureStatus(flagsToCheck[idx], &status))) {
>              if (status != nsIGfxInfo::FEATURE_STATUS_OK) {

Only one flag to check here now so you can remove the for loop and flagsToCheck variable entirely
Attachment #8521628 - Flags: review?(dtownsend+bugmail) → review+
Not sure if we're even going to reuse the e10s offer prompt anymore, but it would be good to also update it to offer to Win users even if hw-accel is disabled. 
http://mxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js#2505
Attached patch patch (r=Mossop) (obsolete) — Splinter Review
I think I've updated the right code in nsBrowserGlue.js.. felipe?
Attachment #8521628 - Attachment is obsolete: true
Attachment #8522357 - Flags: review?(felipc)
Comment on attachment 8522357 [details] [diff] [review]
patch (r=Mossop)

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

::: browser/components/nsBrowserGlue.js
@@ +2500,1 @@
>          isHardwareAccelerated = winutils.layerManagerType != "Basic";

might as well wrap the entire try block in the ifdef. And just add a comment explaining the ifdef saying that other platforms are ok with hw-accel off
Attachment #8522357 - Flags: review?(felipc) → review+
Keywords: checkin-needed
a missing paren here, should have pushed this to osx as well. lets see if this fixes things - 

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0bc6833dce0f
Attachment #8522472 - Attachment is obsolete: true
green this time.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/cbcafb38ad62
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.