Fix GraphicsLayerUpdater::UpdateContext::compositingContainer() for corner cases

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

 /*
  * Copyright (C) 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2014 Google 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
@@ skipping 38 matching lines @@
         compositingState != PaintsIntoGroupedBacking) {
       m_compositingAncestor = &layer;
       if (layer.stackingNode()->isStackingContext())
         m_compositingStackingContext = &layer;
     }
   }
 
   const PaintLayer* compositingContainer(const PaintLayer& layer) const {
     if (layer.stackingNode()->isStacked())
       return m_compositingStackingContext;
-
-    // TODO(wangxianzhu, chrishtr): This is incorrect if m_compositingAncestor
-    // is inline and there is any non-layer floating object between layer and
-    // m_compositingAncestor. Should use the logic in PaintLayer::
-    // containingLayer().
-    if (layer.layoutObject().isFloatingWithNonContainingBlockParent())
-      return layer.enclosingLayerWithCompositedLayerMapping(ExcludeSelf);

[Xianzhu, 2017/03/30 22:23:12]

The test (PaintLayerTest.CompositingContainerUnderFloat[UnderInline]) for the
above code still passes with the above code removed. Do you remember why we
added it? Perhaps the bug situation is also fixed by some other change after the
above code was added.

[chrishtr, 2017/03/31 00:12:28]

The sequence of patches here was:

https://codereview.chromium.org/2611233003

then 

https://codereview.chromium.org/2624843002 (fixes a bug introduced in
the above)

Maybe a test modeled on the situation in the tests in those CLs will
expose the condition? It was a very obscure situation involving non-self-
painting layers IIRC.

[Xianzhu, 2017/03/31 04:14:43]

I just review the above CLs and my later related changes
https://codereview.chromium.org/2650873002/ and
https://codereview.chromium.org/2682933002/.

It seems that we don't need the condition above because it seems to just return
m_compositingAncestor. I added DCHECK_EQ(m_compositingAncestor,
layer.enclosingLayerWithCompositedLayerMapping(ExcludeSelf)) here in the latest
patch set, and all unit tests and paint/, compositing/ layout tests passed
locally.

-
     return m_compositingAncestor;
   }
 
   const PaintLayer* compositingStackingContext() const {
     return m_compositingStackingContext;
   }
 
  private:
   const PaintLayer* m_compositingStackingContext;
   const PaintLayer* m_compositingAncestor;
@@ skipping 55 matching lines @@
         ->assertNeedsToUpdateGraphicsLayerBitsCleared();
 
   for (PaintLayer* child = layer.firstChild(); child;
        child = child->nextSibling())
     assertNeedsToUpdateGraphicsLayerBitsCleared(*child);
 }
 
 #endif
 
 }  // namespace blink