951186 - BasicCompositor / ContentClientSingle/DoubleBuffered broken on Mac, WARNING: Tried to upload without a compositor. Skipping texture upload...: file /Users/mstange/code/mozilla/gfx/layers/composite/TextureHost.cpp, line 450

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Markus Stange [:mstange]]

Steps to reproduce on OS X:

 1a. Set layers.acceleration.disable = true and
     layers.offmainthreadcomposition.force-basic = true
or
 1b. Set layers.acceleration.disable = true and
     browser.tabs.remote = true
or
 1c. Remove the lines
#ifdef XP_MACOSX
  if (backend == LAYERS_OPENGL) {
    return new ContentClientIncremental(aForwarder);
  }
#endif
     from ContentClient::CreateContentClient.

 2. Restart the browser.

The window stays blank and debug builds print this warning:
WARNING: Tried to upload without a compositor. Skipping texture upload...: file /Users/mstange/code/mozilla/gfx/layers/composite/TextureHost.cpp, line 450

I don't know when this regressed.

Comment 1

[Markus Stange [:mstange]]

Attachment: call SetCompositor in ContentHostBase::UseTextureHost

This fixes it, but I don't know if it's the right way to fix it.

SetCompositor is called while mTextureHost is still null, with this call stack:

> #0  mozilla::layers::ContentHostBase::SetCompositor (this=0x113398580, aCompositor=0x11c6221f0) at ContentHost.cpp:257
> #1  mozilla::layers::CompositableHost::Attach (this=0x113398580, aLayer=0x11c8a6800, aCompositor=0x11c6221f0, aFlags=0) at CompositableHost.h:249
> #2  mozilla::layers::LayerTransactionParent::Attach (this=0x1212af230, aLayerParent=0x112464440, aCompositable=0x115476760, aIsAsyncVideo=false) at LayerTransactionParent.cpp:530
> #3  mozilla::layers::LayerTransactionParent::RecvUpdate (this=0x1212af230, cset=@0x116a80280, targetConfig=@0x116a80258, isFirstPaint=@0x116a80257, reply=0x116a80240) at LayerTransactionParent.cpp:421

Later, mTextureHost is initialized in UseTexture with this call stack:

> #0  mozilla::layers::ContentHostBase::UseTextureHost (this=0x113398580, aTexture=0x1154769a0) at ContentHost.cpp:242
> #1  mozilla::layers::CompositableParentManager::ReceiveCompositableUpdate (this=0x1212af280, aEdit=@0x11245988c, replyv=@0x116a80028) at CompositableTransactionParent.cpp:229
> #2  mozilla::layers::LayerTransactionParent::RecvUpdate (this=0x1212af230, cset=@0x116a80280, targetConfig=@0x116a80258, isFirstPaint=@0x116a80257, reply=0x116a80240) at LayerTransactionParent.cpp:415

Comment 2

[Nicolas Silva [:nical]]

Comment on attachment 8348780 [details] [diff] [review]
call SetCompositor in ContentHostBase::UseTextureHost

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

Looks good. The trickiness of setting/unsetting the compositor comes from that compositable/textures can jump from a compositor to another, for example when you play a video with async-video turned on, and you drag-n-drop the tab containing the video into another window. The general idea is that a given texture should use the same compositor as it's compositable(s) and that it's safe to set the compositor too often in doubt, since if the compositor doesn't change, we don't do anything fancy like deallocating the underlying textures.

Comment 3

[Markus Stange [:mstange]]

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

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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