1061209 - Nightly only shows a plain white/grey window on OS X in a VM

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[:Gijs (he/him)]

Starting today's nightly from the commandline shows:

Gijss-Mac:~ gijs$ /Applications/FirefoxNightly.app/Contents/MacOS/firefox
ERROR: Changing compositor from 2 to 1.

Trying to find a regression window at the moment.

Comment 1

[:Gijs (he/him)]

m-c: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d697d649c765&tochange=eed9fe35a00d

Still looking at inbound.

Comment 2

[:Gijs (he/him)]

(this is on beta 1, btw... I'll update to beta 2 after I figure out this regression window; maybe that'll fix it? It'd somewhat surprise me because the m-c regression window seems to not include any yosemite-specific stuff, apart from my frontend patch for the urlbar/searchbar styling, and that shouldn't be able to cause this...)

Comment 3

[:Gijs (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d697d649c765&tochange=de5cefa8e52e

which is very strange because this is still huge (although bug 1028288 jumps out at me). Poking mozregression some more to see if I can't narrow this down without resorting to building...

Comment 4

[:Gijs (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0cba3e5a377d&tochange=ab4ef3fa85bd

And looking at that patch, this makes some amount of sense, as the code removed was also run on mac, and not just Linux as the patch description said... Nical, can you have a look?

Comment 5

[:Gijs (he/him)]

See bug 974007 comment 14, I think? Might be broken on other OS Xs in VMs as well, but I don't have any so can't test. Looks like nobody ever figured out why the basic compositor is broken on OS X.

Comment 6

[Markus Stange [:mstange]]

(In reply to :Gijs Kruitbosch from comment #0)
> Starting today's nightly from the commandline shows:
> 
> Gijss-Mac:~ gijs$ /Applications/FirefoxNightly.app/Contents/MacOS/firefox
> ERROR: Changing compositor from 2 to 1.

2 is LAYERS_OPENGL and 1 is LAYERS_BASIC.

I wonder what triggers the change to BasicCompositor.

Gijs, did you get OpenGL compositing in your VM prior to this change, or did it fall back to BasicLayers?

Comment 7

[:Gijs (he/him)]

(In reply to Markus Stange [:mstange] from comment #6)
> (In reply to :Gijs Kruitbosch from comment #0)
> > Starting today's nightly from the commandline shows:
> > 
> > Gijss-Mac:~ gijs$ /Applications/FirefoxNightly.app/Contents/MacOS/firefox
> > ERROR: Changing compositor from 2 to 1.
> 
> 2 is LAYERS_OPENGL and 1 is LAYERS_BASIC.
> 
> I wonder what triggers the change to BasicCompositor.
> 
> Gijs, did you get OpenGL compositing in your VM prior to this change, or did
> it fall back to BasicLayers?

I don't know - how would I check?

Comment 8

[:Gijs (he/him)]

Here's the about:support output from the VM:

Device ID	0x 405
GPU Accelerated Windows	0/1 Basic
Vendor ID	0x15ad
windowLayerManagerRemote	false
AzureCanvasBackend	quartz
AzureContentBackend	quartz
AzureFallbackCanvasBackend	none
AzureSkiaAccelerated	0

so yes, this was using basic before...

Comment 9

[Stefan [:stefanh]]

I filed bug 1060948 yesterday, but even though the symptoms looks quite similar I guess it must be a different issue?

Comment 10

[Nicolas Silva [:nical]]

I'll submit a fix tomorrow.

Comment 11

[Nicolas Silva [:nical]]

Attachment: Restore the previous behavior (disabling basic OMTC) except on Linux

Comment 12

[:Gijs (he/him)]

(In reply to Stefan [:stefanh] from comment #9)
> I filed bug 1060948 yesterday, but even though the symptoms looks quite
> similar I guess it must be a different issue?

It looks different to me, yes - but you identified a regressing cset, which might be the same? I can't tell, see the needinfo in that bug. :-)

Comment 13

[Markus Stange [:mstange]]

Nical, can you describe what went wrong and why Linux and OS X need different behavior?

Comment 14

[Nicolas Silva [:nical]]

(In reply to Markus Stange [:mstange] from comment #13)
> Nical, can you describe what went wrong and why Linux and OS X need
> different behavior?

I don't know what's wrong with basic OMTC on mac. I'll have to look into it and ask around. In the mean time reverting the change that caused people to use basic OMTC on mac lets me focus on the higher priority items for now.
needinfo'ing myself as a remainder for later.

Comment 15

[Markus Stange [:mstange]]

Basic OMTC on Mac is working on my machine. However, it works by creating an OpenGL context and doing very basic operations on it (see GLPresenter in nsChildView.mm). If OpenGL context creation fails on some machines we'll need to think of a different solution. (It probably means that we need to snapshot the composition result, ship it back to the main thread, and only update the window on the main thread.)
Are we planning to use OMTC always for all drawing?

Comment 16

[Matt Woodrow (:mattwoodrow)]

(In reply to Markus Stange [:mstange] from comment #15)
> Are we planning to use OMTC always for all drawing?

Yeah, that was the plan.

Comment 17

[Cameron Kaiser [:spectre]]

(In reply to Markus Stange [:mstange] from comment #15)
> Basic OMTC on Mac is working on my machine. However, it works by creating an
> OpenGL context and doing very basic operations on it (see GLPresenter in
> nsChildView.mm). If OpenGL context creation fails on some machines we'll
> need to think of a different solution. (It probably means that we need to
> snapshot the composition result, ship it back to the main thread, and only
> update the window on the main thread.)

Not to derail this, but is that always the case? TenFourFox blocks acceleration at the OS level, though we probably do create a GL context (I say probably -- GLContextProviderCGL::CreateForWindow likely succeeds on 10.4, I have to check). Nevertheless, OMTC does work for us.

Comment 19

[Nicolas Silva [:nical]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/177fdd6bfb19

Comment 20

[Nicolas Silva [:nical]]

WError fix https://hg.mozilla.org/integration/mozilla-inbound/rev/51d39a24b168

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/177fdd6bfb19
https://hg.mozilla.org/mozilla-central/rev/51d39a24b168

Comment 22

[Markus Stange [:mstange]]

This is still unfixed on Aurora.

[Tracking Requested - why for this release]: bad regression for users that don't get OpenGL (e.g. VMs)

Comment 24

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment 2 said that this was an issue on 33 beta1, which is the current beta cycle. Is 33 affected?

nical - Can you pleas submit an Aurora approval request so that we can fix this in 34?

Comment 25

[Markus Stange [:mstange]]

The word "beta" in comment 2 was referring to the Mac OS 10.10 Beta, not the Firefox Beta.

Comment 26

[Nicolas Silva [:nical]]

(In reply to Markus Stange [:mstange] from comment #25)
> The word "beta" in comment 2 was referring to the Mac OS 10.10 Beta, not the
> Firefox Beta.

Yes, the regression and the fix where on the same train.

Comment 27

[Lawrence Mandel [:lmandel] (use needinfo)]

My mistake. Thank you for clarifying. 

nical - I'm still interested in uplifting the fix to Aurora. Reminder to please submit an approval request.

Comment 28

[Nicolas Silva [:nical]]

Comment on attachment 8482627 [details] [diff] [review]
Restore the previous behavior (disabling basic OMTC) except on Linux

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Crashes on Mac
[Describe test coverage new/current, TBPL]:
[Risks and why]: not risky, restores code that has been used for a long while, has been baking on m-c for a while.
[String/UUID change made/needed]:

Comment 29

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8482627 [details] [diff] [review]
Restore the previous behavior (disabling basic OMTC) except on Linux

Aurora+

Comment 30

[Nicolas Silva [:nical]]

Attachment: upliftable patch (carries r=mattwoodrow)

This contains the above patch folded with the WError fix and applies on aurora

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0ca144377622