1089008 - enable e10s on windows even without hardware acceleration

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Tomislav Jovanovic :zombie]

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).

Comment 1

[Jim Mathies [:jimm]]

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.

Comment 2

[Jim Mathies [:jimm]]

Attachment: patch

Comment 3

[Dave Townsend [:mossop]]

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

Comment 4

[:Felipe Gomes (needinfo for replies!)]

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

Comment 5

[Jim Mathies [:jimm]]

Attachment: patch (r=Mossop)

I think I've updated the right code in nsBrowserGlue.js.. felipe?

Comment 6

[:Felipe Gomes (needinfo for replies!)]

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

Comment 7

[Jim Mathies [:jimm]]

Attachment: patch (r=Mossop, felipe)

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=aa2b33217c85

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8be2344dcff3

Comment 9

[Cameron McCormack (:heycam)]

Backed out for OS X bustage.

https://tbpl.mozilla.org/php/getParsedLog.php?id=52594875&tree=Mozilla-Inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e8496d85ba2

Comment 10

[Jim Mathies [:jimm]]

Attachment: patch (r=Mossop, felipe)

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

Comment 11

[Jim Mathies [:jimm]]

green this time.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cbcafb38ad62

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/cbcafb38ad62