Directly composited images are images: ASSERT that

Source/core/rendering/compositing/CompositedLayerMapping.cpp

 /*
  * Copyright (C) 2009, 2010, 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 439 matching lines @@
         if (m_owningLayer.reflectionInfo()->reflectionLayer()->hasCompositedLayerMapping()) {
             GraphicsLayer* reflectionLayer = m_owningLayer.reflectionInfo()->reflectionLayer()->compositedLayerMapping()->mainGraphicsLayer();
             m_graphicsLayer->setReplicatedByLayer(reflectionLayer);
         }
     } else {
         m_graphicsLayer->setReplicatedByLayer(0);
     }
 
     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();
         if (layer)
             m_graphicsLayer->setContentsToPlatformLayer(layer);
     } else if (renderer->isVideo()) {
         HTMLMediaElement* mediaElement = toHTMLMediaElement(renderer->node());
         m_graphicsLayer->setContentsToPlatformLayer(mediaElement->platformLayer());
     } else if (isAcceleratedCanvas(renderer)) {
         HTMLCanvasElement* canvas = toHTMLCanvasElement(renderer->node());
         if (CanvasRenderingContext* context = canvas->renderingContext())
@@ skipping 1216 matching lines @@
     }
 
     return false;
 }
 
 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,
     // and set background color on the layer in that case, instead of allocating backing store and painting.
     if (renderObject->isVideo() && toRenderVideo(renderer())->shouldDisplayVideo())
         return m_owningLayer.hasBoxDecorationsOrBackground();
 
     if (m_owningLayer.hasVisibleBoxDecorations())
         return true;
 
@@ skipping 22 matching lines @@
     }
 
     // FIXME: it's O(n^2). A better solution is needed.
     return paintsChildren();
 }
 
 // An image can be directly compositing if it's the sole content of the layer, and has no box decorations
 // that require painting. Direct compositing saves backing store.
 bool CompositedLayerMapping::isDirectlyCompositedImage() const
 {
+    ASSERT(renderer()->isImage());
+
     RenderObject* renderObject = renderer();
-
-    if (!renderObject->isImage() || m_owningLayer.hasBoxDecorationsOrBackground() || renderObject->hasClip())
+    if (m_owningLayer.hasBoxDecorationsOrBackground() || renderObject->hasClip())
         return false;
 
     RenderImage* imageRenderer = toRenderImage(renderObject);
     if (ImageResource* cachedImage = imageRenderer->cachedImage()) {
         if (!cachedImage->hasImage())
             return false;
 
         Image* image = cachedImage->imageForRenderer(imageRenderer);
         return image->isBitmapImage();
     }
@@ skipping 26 matching lines @@
     Image* image = cachedImage->imageForRenderer(imageRenderer);
     if (!image)
         return;
 
     // We have to wait until the image is fully loaded before setting it on the layer.
     if (!cachedImage->isLoaded())
         return;
 
     // 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
     // the image. So we have to kick the animation each time; this has the downside that the
     // image will keep animating, even if its layer is not visible.
     image->startAnimation();
 }
 
 FloatPoint3D CompositedLayerMapping::computeTransformOrigin(const IntRect& borderBox) const
 {
@@ skipping 474 matching lines @@
     } else if (graphicsLayer == m_scrollingBlockSelectionLayer.get()) {
         name = "Scrolling Block Selection Layer";
     } else {
         ASSERT_NOT_REACHED();
     }
 
     return name;
 }
 
 } // namespace blink