Fix the bug that negative outline-offset is covered up by composited

third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp

Index: third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
diff --git a/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp b/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
index 8a84bb055d86c9b4bfe9f147a63bdde1442615f0..d6d6dff7e8be49421e2b9a48a00948735ad79dc7 100644
--- a/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
+++ b/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
@@ -559,6 +559,13 @@ bool CompositedLayerMapping::updateGraphicsLayerConfiguration() {
     scrollingConfigChanged = true;
   }
 
+  // The decoration layer is currently used for painting outline with negative
+  // offset that would be covered up by the composited scrolling contents layer
+  // and scrollbars.
+  if (updateDecorationLayer(m_owningLayer.getScrollableArea()->usesCompositedScrolling()) &&

[flackr, 2016/10/25 19:48:10]

Can you move this condition to a boolean similar to needsAncestorClip? Also, it
looks like you're only passing usesCompositedScrolling() for
needsDecorationLayer.

[yigu, 2016/10/25 20:58:44]

This bug has been fixed in patch 4.

+      layoutObject->style()->hasOutline() && layoutObject->style()->outlineOffset() < 0)

[flackr, 2016/10/25 19:48:10]

In fact, now that I think about it this should be less than the minimum negative
border width (i.e. same check that we have in
LayoutBoxModelObject::hasLocalEquivalentBackground). Can you remove the check
for negative outline offset there since with this patch we can now composite it
correctly?

[yigu, 2016/10/25 20:58:44]

Done.

+    layerConfigChanged = true;
+
   if (updateOverflowControlsLayers(
           requiresHorizontalScrollbarLayer(), requiresVerticalScrollbarLayer(),
           requiresScrollCornerLayer(), needsAncestorClip))
@@ -944,6 +951,7 @@ void CompositedLayerMapping::updateGraphicsLayerGeometry(
                           relativeCompositingBounds);
   updateForegroundLayerGeometry(contentsSize, clippingBox);
   updateBackgroundLayerGeometry(contentsSize);
+  updateDecorationLayerGeometry(contentsSize);

[flackr, 2016/10/25 19:48:10]

It seems to me that the sizes of these layers will be larger than they need to
be. Consider for example:

outline: 20px solid blue;
outline-offset: -5px;

This should result in only the decoration layer being 15px larger than the
element since the background and foreground layers will no longer draw the
outline. For now, it's probably safe to have the layers be the same size but we
should fix this in the future. Can you leave a TODO to only make the layers as
large as they need to be?

[yigu, 2016/10/25 20:58:44]

TODO added.

   updateScrollingLayerGeometry(localCompositingBounds);
   updateChildClippingMaskLayerGeometry();
 
@@ -1371,6 +1379,20 @@ void CompositedLayerMapping::updateBackgroundLayerGeometry(
       m_graphicsLayer->offsetFromLayoutObject());
 }
 
+void CompositedLayerMapping::updateDecorationLayerGeometry(
+    const FloatSize& relativeCompositingBoundsSize) {
+  if (!m_decorationLayer)
+    return;
+  FloatSize decorationSize = relativeCompositingBoundsSize;
+  m_decorationLayer->setPosition(FloatPoint());
+  if (decorationSize != m_decorationLayer->size()) {
+    m_decorationLayer->setSize(decorationSize);
+    m_decorationLayer->setNeedsDisplay();
+  }
+  m_decorationLayer->setOffsetFromLayoutObject(
+      m_graphicsLayer->offsetFromLayoutObject());
+}
+
 void CompositedLayerMapping::registerScrollingLayers() {
   // Register fixed position layers and their containers with the scrolling
   // coordinator.
@@ -1438,6 +1460,10 @@ void CompositedLayerMapping::updateInternalHierarchy() {
   if (m_layerForScrollCorner)
     m_overflowControlsHostLayer->addChild(m_layerForScrollCorner.get());
 
+  // Now add the DecorationLayer as a subtree to GraphicsLayer
+  bottomLayer = m_graphicsLayer.get();
+  updateBottomLayer(m_decorationLayer.get());
+
   // The squashing containment layer, if it exists, becomes a no-op parent.
   if (m_squashingLayer) {
     ASSERT((m_ancestorClippingLayer && !m_squashingContainmentLayer) ||
@@ -1572,6 +1598,9 @@ void CompositedLayerMapping::updateDrawsContent() {
   if (m_backgroundLayer)
     m_backgroundLayer->setDrawsContent(hasPaintedContent);
 
+  if (m_decorationLayer)
+    m_decorationLayer->setDrawsContent(hasPaintedContent);
+
   if (m_maskLayer)
     m_maskLayer->setDrawsContent(true);
 
@@ -1771,12 +1800,14 @@ enum ApplyToGraphicsLayersModeFlags {
       (1 << 6),  // layers between m_graphicsLayer and children
   ApplyToNonScrollingContentLayers = (1 << 7),
   ApplyToScrollingContentLayers = (1 << 8),
+  ApplyToDecorationLayer = (1 << 9),
   ApplyToAllGraphicsLayers =
       (ApplyToSquashingLayer | ApplyToScrollbarLayers | ApplyToBackgroundLayer |
        ApplyToMaskLayers |
        ApplyToLayersAffectedByPreserve3D |
        ApplyToContentLayers |
-       ApplyToScrollingContentLayers)
+       ApplyToScrollingContentLayers |
+       ApplyToDecorationLayer)
 };
 typedef unsigned ApplyToGraphicsLayersMode;
 
@@ -1839,6 +1870,10 @@ static void ApplyToGraphicsLayers(const CompositedLayerMapping* mapping,
     f(mapping->layerForVerticalScrollbar());
   if ((mode & ApplyToScrollbarLayers) && mapping->layerForScrollCorner())
     f(mapping->layerForScrollCorner());
+
+  if (((mode & ApplyToDecorationLayer) ||
+       (mode & ApplyToNonScrollingContentLayers)) && mapping->decorationLayer())
+    f(mapping->decorationLayer());
 }
 
 struct UpdateRenderingContextFunctor {
@@ -2017,6 +2052,23 @@ bool CompositedLayerMapping::updateBackgroundLayer(bool needsBackgroundLayer) {
   return layerChanged;
 }
 
+bool CompositedLayerMapping::updateDecorationLayer(bool needsDecorationLayer) {
+  bool layerChanged = false;
+  if (needsDecorationLayer) {
+    if (!m_decorationLayer) {
+      m_decorationLayer =
+          createGraphicsLayer(CompositingReasonLayerForDecoration);
+      m_decorationLayer->setPaintingPhase(GraphicsLayerPaintDecoration);
+      layerChanged = true;
+    }
+  } else if (m_decorationLayer) {
+    m_decorationLayer = nullptr;
+    layerChanged = true;
+  }
+
+  return layerChanged;
+}
+
 bool CompositedLayerMapping::updateMaskLayer(bool needsMaskLayer) {
   bool layerChanged = false;
   if (needsMaskLayer) {
@@ -2955,6 +3007,8 @@ void CompositedLayerMapping::paintContents(
     paintLayerFlags |= PaintLayerPaintingOverflowContents;
   if (graphicsLayerPaintingPhase & GraphicsLayerPaintCompositedScroll)
     paintLayerFlags |= PaintLayerPaintingCompositingScrollingPhase;
+  if (graphicsLayerPaintingPhase & GraphicsLayerPaintDecoration)
+    paintLayerFlags |= PaintLayerPaintingCompositingDecorationPhase;
 
   if (graphicsLayer == m_backgroundLayer.get())
     paintLayerFlags |= PaintLayerPaintingRootBackgroundOnly;
@@ -2967,7 +3021,8 @@ void CompositedLayerMapping::paintContents(
       graphicsLayer == m_backgroundLayer.get() ||
       graphicsLayer == m_maskLayer.get() ||
       graphicsLayer == m_childClippingMaskLayer.get() ||
-      graphicsLayer == m_scrollingContentsLayer.get()) {
+      graphicsLayer == m_scrollingContentsLayer.get() ||
+      graphicsLayer == m_decorationLayer.get()) {
     bool paintRootBackgroundOntoScrollingContentsLayer =
         m_backgroundPaintsOntoScrollingContentsLayer;
     DCHECK(!paintRootBackgroundOntoScrollingContentsLayer ||
@@ -3248,6 +3303,8 @@ String CompositedLayerMapping::debugName(
     name = "Scrolling Layer";
   } else if (graphicsLayer == m_scrollingContentsLayer.get()) {
     name = "Scrolling Contents Layer";
+  } else if (graphicsLayer == m_decorationLayer.get()) {
+    name = "Decoration Layer";
   } else {
     ASSERT_NOT_REACHED();
   }