Directly composited images are images: ASSERT that

Source/core/rendering/compositing/CompositedLayerMapping.cpp

Index: Source/core/rendering/compositing/CompositedLayerMapping.cpp
diff --git a/Source/core/rendering/compositing/CompositedLayerMapping.cpp b/Source/core/rendering/compositing/CompositedLayerMapping.cpp
index 1924a3268a64f8c9bdbe32b0526ef477db51102e..2c233e77a0663f2c1df514b8d09c2cd73c04feaf 100644
--- a/Source/core/rendering/compositing/CompositedLayerMapping.cpp
+++ b/Source/core/rendering/compositing/CompositedLayerMapping.cpp
@@ -457,10 +457,10 @@ bool CompositedLayerMapping::updateGraphicsLayerConfiguration()
 
     updateBackgroundColor();
 
-    if (isDirectlyCompositedImage())
-        updateImageContents();
-
-    if (WebLayer* layer = platformLayerForPlugin(renderer)) {
+    if (renderer->isImage()) {
+        if (isDirectlyCompositedImage())
+            updateImageContents();
+    } else if (WebLayer* layer = platformLayerForPlugin(renderer)) {
         m_graphicsLayer->setContentsToPlatformLayer(layer);

[Ian Vollick, 2014/08/21 03:43:29]

IIUC, it looks like previously, we could still execute one of the following
conditional blocks, even if renderer->isImage() was true, but now we won't. Is
that true (and is this correct)?

[Noel Gordon, 2014/08/21 06:16:19]

Good point, and I wonder which of them would be entered if isImage() was true?

I believe isImage() is true for RenderImage, StyleGeneratedImage, and
RenderListMarker when list's marker is an image.  Given that:

   if ( .... platformLayerForPlugin(renderer)
     plugins are not isImage(), right?

  if (renderer->node() && renderer->node()->isFrameOwnerElement()
     nope frame owners are not isImage(), right?

  if (renderer->isVideo())
     nope, isVideo() -> isMedia() and MediaElement isImage() returns false.

  if (isAcceleratedCanvas(renderer))
     nope, canvas is not isImage() accelerated or not.

concur?

     } else if (renderer->node() && renderer->node()->isFrameOwnerElement() && toHTMLFrameOwnerElement(renderer->node())->contentFrame()) {
         WebLayer* layer = toHTMLFrameOwnerElement(renderer->node())->contentFrame()->remotePlatformLayer();
@@ -1698,8 +1698,8 @@ bool CompositedLayerMapping::containsPaintedContent() const
     if (paintsIntoCompositedAncestor() || m_owningLayer.isReflection())
         return false;
 
-    if (isDirectlyCompositedImage())
-        return false;
+    if (renderer()->isImage())
+        return !isDirectlyCompositedImage();

[Ian Vollick, 2014/08/21 03:43:29]

Similarly, it looks like we used to be possible to execute the rest of this
function when renderer()->isImage().

[Noel Gordon, 2014/08/21 06:16:19]

Originally had this as:

  if (renderer()->isImage() && isDirectlyCompositedImage())
    return false;

and maybe we should do that?

 
     RenderObject* renderObject = renderer();
     // FIXME: we could optimize cases where the image, video or canvas is known to fill the border box entirely,
@@ -1742,9 +1742,10 @@ bool CompositedLayerMapping::containsPaintedContent() const
 // that require painting. Direct compositing saves backing store.
 bool CompositedLayerMapping::isDirectlyCompositedImage() const
 {
-    RenderObject* renderObject = renderer();
+    ASSERT(renderer()->isImage());
 
-    if (!renderObject->isImage() || m_owningLayer.hasBoxDecorationsOrBackground() || renderObject->hasClip())
+    RenderObject* renderObject = renderer();
+    if (m_owningLayer.hasBoxDecorationsOrBackground() || renderObject->hasClip())
         return false;
 
     RenderImage* imageRenderer = toRenderImage(renderObject);
@@ -1791,6 +1792,8 @@ void CompositedLayerMapping::updateImageContents()
 
     // This is a no-op if the layer doesn't have an inner layer for the image.
     m_graphicsLayer->setContentsToImage(image);
+
+    // Prevent double-drawing: https://bugs.webkit.org/show_bug.cgi?id=58632
     updateDrawsContent();
 
     // Image animation is "lazy", in that it automatically stops unless someone is drawing