619488 - Inform child processes of the compositor's layer-manager type

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We will need to do some things differently in shadow forwarders depending on the type of shadow manager.

Initially, it should be enough to make the PLayers ctor 'sync' and return the parent-side type from the ctor.

This bakes in our compositor-layer-manager-doesn't-change assumption a little more, but nothing that makes fixing that fundamentally harder.

Comment 1

[Oleg Romashin (:romaxa)]

Attachment: Possible fix.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 498225 [details] [diff] [review]
Possible fix.

We need to add

   enum LayersBackend {
+     LAYERS_NONE = 0,
...
+     LAYERS_LAST
   };

so that we can easily check for legal values.

>diff --git a/gfx/layers/ipc/ShadowLayers.cpp b/gfx/layers/ipc/ShadowLayers.cpp
>--- a/gfx/layers/ipc/ShadowLayers.cpp
>+++ b/gfx/layers/ipc/ShadowLayers.cpp
>+LayerManager::LayersBackend
>+ShadowLayerForwarder::GetParentBackendType()
>+{
>+  if (mParentBackend == -1) {

s/-1/LAYERS_NONE/.

>+    LayerManager::LayersBackend backend;

s/LayerManager:://.

>+    if (mShadowManager->SendGetParentType(&backend)) {
>+      mParentBackend = backend;
>+    }
>+  }
>+  return (LayerManager::LayersBackend)mParentBackend;

Won't need this cast after requested changes.

>diff --git a/gfx/layers/ipc/ShadowLayers.h b/gfx/layers/ipc/ShadowLayers.h
>--- a/gfx/layers/ipc/ShadowLayers.h
>+++ b/gfx/layers/ipc/ShadowLayers.h
>@@ -300,16 +300,18 @@ public:

Add a

public:
  typedef LayerManager::LayersBackend LayersBackend;

>   void DestroySharedSurface(SurfaceDescriptor* aSurface);
> 
>   /**
>    * Construct a shadow of |aLayer| on the "other side", at the
>    * ShadowLayerManager.
>    */
>   PLayerChild* ConstructShadowFor(ShadowableLayer* aLayer);
> 
>+  LayerManager::LayersBackend GetParentBackendType();
>+

s/LayerManager:://.

>+  PRUint32 mParentBackend;

LayersBackend mParentBackend;

>diff --git a/ipc/glue/IPCMessageUtils.h b/ipc/glue/IPCMessageUtils.h
>--- a/ipc/glue/IPCMessageUtils.h
>+++ b/ipc/glue/IPCMessageUtils.h> 
>+  static bool Read(const Message* msg, void** iter, paramType* result)
>+  {
>+    int32 filter;

Hmm s/filter/type/?

>+    if (!ReadParam(msg, iter, &filter))
>+      return false;
>+
>+    *result = paramType(filter);
>+    return true;

  if (ReadParam(...) &&
      LAYERS_NONE < type < LAYERS_LAST) {
    *result = paramType(filter);
    return true;
  }
  return false;

This patch looks fine, my comments are all nitty.  Would like to see
another rev though.

Comment 3

[Oleg Romashin (:romaxa)]

Attachment: Nice organized

Comment 4

[Oleg Romashin (:romaxa)]

Attachment: Minor compile fix

-  if (mParentBackend == LAYERS_NONE) {
+  if (mParentBackend == LayerManager::LAYERS_NONE) {

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 498502 [details] [diff] [review]
Minor compile fix

>diff --git a/gfx/layers/ipc/ShadowLayers.cpp b/gfx/layers/ipc/ShadowLayers.cpp
>--- a/gfx/layers/ipc/ShadowLayers.cpp
>+++ b/gfx/layers/ipc/ShadowLayers.cpp
>@@ -118,17 +118,19 @@ private:
>-ShadowLayerForwarder::ShadowLayerForwarder() : mShadowManager(NULL)
>+ShadowLayerForwarder::ShadowLayerForwarder()
>+ : mShadowManager(NULL)
>+ , mParentBackend(LayerManager::LAYERS_BASIC)

Big problem: this is going to cause GetBackendType to always return BASIC.  Need to initialize to NONE.

r=me with that fixed.

Comment 6

[Oleg Romashin (:romaxa)]

Attachment: init with NONE

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This blocks a blocker.

Comment 8

[Oleg Romashin (:romaxa)]

http://hg.mozilla.org/mozilla-central/rev/75928a70efe7