Closed Bug 619488 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: cjones, Assigned: romaxa)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
Attached patch Possible fix. (obsolete) — Splinter Review
Attachment #498225 - Flags: review?(jones.chris.g)
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.
Attachment #498225 - Flags: review?(jones.chris.g)
Attached patch Nice organized (obsolete) — Splinter Review
Assignee: nobody → romaxa
Attachment #498225 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #498498 - Flags: review?(jones.chris.g)
-  if (mParentBackend == LAYERS_NONE) {
+  if (mParentBackend == LayerManager::LAYERS_NONE) {
Attachment #498498 - Attachment is obsolete: true
Attachment #498502 - Flags: review?(jones.chris.g)
Attachment #498498 - Flags: review?(jones.chris.g)
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.
Attachment #498502 - Flags: review?(jones.chris.g) → review+
Attached patch init with NONESplinter Review
Attachment #499553 - Flags: approval2.0?
This blocks a blocker.
tracking-fennec: --- → ?
http://hg.mozilla.org/mozilla-central/rev/75928a70efe7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.