Fix crash in FrameView::updateLayersAndCompositingAfterScrollIfNeeded

third_party/WebKit/Source/core/frame/FrameViewTest.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/frame/FrameView.h"
 
 #include <memory>
 
 #include "bindings/core/v8/ExceptionState.h"
 #include "core/frame/Settings.h"
 #include "core/html/HTMLElement.h"
 #include "core/layout/LayoutBoxModelObject.h"
 #include "core/layout/LayoutObject.h"
 #include "core/loader/EmptyClients.h"
 #include "core/page/Page.h"
+#include "core/paint/PaintLayer.h"
 #include "core/testing/DummyPageHolder.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "platform/geometry/IntSize.h"
 #include "platform/graphics/paint/PaintArtifact.h"
 #include "platform/heap/Handle.h"
 #include "platform/testing/RuntimeEnabledFeaturesTestHelpers.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using testing::_;
@@ skipping 106 matching lines @@
 
 // NoOverflowInIncrementVisuallyNonEmptyPixelCount tests fail if the number of
 // pixels is calculated in 32-bit integer, because 65536 * 65536 would become 0
 // if it was calculated in 32-bit and thus it would be considered as empty.
 TEST_P(FrameViewTest, NoOverflowInIncrementVisuallyNonEmptyPixelCount) {
   EXPECT_FALSE(document().view()->isVisuallyNonEmpty());
   document().view()->incrementVisuallyNonEmptyPixelCount(IntSize(65536, 65536));
   EXPECT_TRUE(document().view()->isVisuallyNonEmpty());
 }
 
+// This test addresses http://crbug.com/696173, in which a call to
+// FrameView::updateLayersAndCompositingAfterScrollIfNeeded during layout caused
+// a crash as the code was incorrectly assuming that the ancestor overflow layer
+// would always be valid.
+TEST_P(FrameViewTest, ViewportConstrainedObjectsHandledCorrectlyDuringLayout) {
+  // When using root layer scrolling there is no concept of viewport constrained
+  // objects, so skip this test.
+  if (RuntimeEnabledFeatures::rootLayerScrollingEnabled())

[flackr, 2017/02/27 16:42:09]

Don't we run into the same problem with updateLayerPositionsAfterOverflowScroll
on non-root layers?

[flackr, 2017/02/27 16:43:14]

Nevermind, on that we update top-down so you didn't have the additional ancestor
overflow checks.

[bokan, 2017/02/28 13:07:44]

I'm not sure I understand why we can't run this with RLS enabled. Sticky will
still have to work in that world (though I'd believe it doesn't yet) so we
should test there too. If RLS doesn't support sticky yet and so this test
*can't* work, note that in the comment so that we know to remove the skip once
RLS is far along enough.

[smcgruer, 2017/02/28 13:13:47]

I guess its black-box testing.
FrameView::updateLayersAndCompositingAfterScrollIfNeeded early-exits
(immediately) if there are no viewport constrained objects, and as far as I
understand it (which may be wrong), RLS implies no viewport constrained objects.

However its still fair to say that the call shouldn't crash, so I have removed
the special-case.

+    return;
+
+  document().body()->setInnerHTML(
+      "<style>.container { height: 200%; }"
+      "#sticky { position: sticky; top: 0; height: 50px; }</style>"
+      "<div class='container'><div id='sticky'></div></div>");
+  document().view()->updateAllLifecyclePhases();
+
+  LayoutBoxModelObject* sticky = toLayoutBoxModelObject(
+      document().getElementById("sticky")->layoutObject());
+
+  // Deliberately invalidate the ancestor overflow layer. This approximates
+  // http://crbug.com/696173, in which the ancestor overflow layer can be null
+  // during layout.
+  sticky->layer()->updateAncestorOverflowLayer(nullptr);
+
+  // This call should not crash.
+  document().view()->setScrollOffset(ScrollOffset(0, 100), ProgrammaticScroll);
+}
+
 TEST_P(FrameViewTest, StyleChangeUpdatesViewportConstrainedObjects) {
   // When using root layer scrolling there is no concept of viewport constrained
   // objects, so skip this test.
   if (RuntimeEnabledFeatures::rootLayerScrollingEnabled())
     return;
 
   document().body()->setInnerHTML(
       "<style>.container { height: 200%; }"
       "#sticky { position: sticky; top: 0; height: 50px; }</style>"
       "<div class='container'><div id='sticky'></div></div>");
@@ skipping 17 matching lines @@
   // And making it sticky again should put it back in that list.
   document().getElementById("sticky")->setAttribute(HTMLNames::styleAttr, "");
   document().view()->updateAllLifecyclePhases();
 
   EXPECT_TRUE(
       document().view()->viewportConstrainedObjects()->contains(sticky));
 }
 
 }  // namespace
 }  // namespace blink