enable e10s on windows even without hardware acceleration

RESOLVED FIXED in mozilla36

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: zombie, Assigned: jimm)

Tracking

unspecified
mozilla36
All
Windows 7
Points:
---

Firefox Tracking Flags

(e10sm4+)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Reporter

Description

5 years ago
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).
Reporter

Updated

5 years ago
tracking-e10s: --- → ?
Assignee: nobody → dtownsend+bugmail
No longer depends on: 1095559
Assignee

Updated

5 years ago
Assignee: dtownsend+bugmail → jmathies
Assignee

Updated

5 years ago
URL: :billm
Assignee

Comment 1

5 years ago
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.
Assignee

Comment 2

5 years ago
Posted patch patch (obsolete) — Splinter Review
Assignee

Updated

5 years ago
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
Assignee

Comment 5

5 years ago
Posted 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+
Assignee

Updated

5 years ago
Keywords: checkin-needed
Assignee

Comment 10

5 years ago
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
Assignee

Comment 11

5 years ago
green this time.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/cbcafb38ad62
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.