[Backport] Use PaintLayer pointer from PaintLayerScrollableArea as PaintLayer is destructed first.

This is a partial reland of e7a9977f4d7072cd809992208d875a8ecd6ef5e8 as flakiness observed on
https://crbug.com/804813 was determined to be likely unrelated.

The accessibility lifetime issue was partially resolved by https://chromium-review.googlesource.com/902775 but there are still accesses after destruction from ScrollableArea and the potential for others to a deleted / freed PaintLayer. Using a pointer which we explicitly null out on dispose ensures that we will crash instead of using freed memory if other accesses occur.

Bug: 797298
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/887908
Commit-Queue: Robert Flack <flackr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537049}

Change-Id: I573d33a59320fdb1f33271df73e481f799a4bc36
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

4 files changed, 34 insertions, 18 deletions

diff --git a/chromium/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp b/chromium/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
index 7fbc8470220..5a91c232f62 100644
--- a/chromium/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
+++ b/chromium/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
@@ -112,7 +112,7 @@ PaintLayerScrollableAreaRareData::PaintLayerScrollableAreaRareData() = default;
 const int kResizerControlExpandRatioForTouch = 2;
 
 PaintLayerScrollableArea::PaintLayerScrollableArea(PaintLayer& layer)
-    : layer_(layer),
+    : layer_(&layer),
       next_topmost_scroll_child_(nullptr),
       topmost_scroll_child_(nullptr),
       in_resize_mode_(false),
@@ -130,8 +130,7 @@ PaintLayerScrollableArea::PaintLayerScrollableArea(PaintLayer& layer)
       scroll_corner_(nullptr),
       resizer_(nullptr),
       scroll_anchor_(this),
-      non_composited_main_thread_scrolling_reasons_(0),
-      has_been_disposed_(false) {
+      non_composited_main_thread_scrolling_reasons_(0) {
   Node* node = GetLayoutBox()->GetNode();
   if (node && node->IsElementNode()) {
     // We save and restore only the scrollOffset as the other scroll values are
@@ -146,13 +145,13 @@ PaintLayerScrollableArea::PaintLayerScrollableArea(PaintLayer& layer)
 }
 
 PaintLayerScrollableArea::~PaintLayerScrollableArea() {
-  DCHECK(has_been_disposed_);
+  DCHECK(HasBeenDisposed());
 }
 
 void PaintLayerScrollableArea::DidScroll(const gfx::ScrollOffset& offset) {
   ScrollableArea::DidScroll(offset);
   // This should be alive if it receives composited scroll callbacks.
-  CHECK(!has_been_disposed_);
+  CHECK(!HasBeenDisposed());
 }
 
 void PaintLayerScrollableArea::Dispose() {
@@ -211,7 +210,11 @@ void PaintLayerScrollableArea::Dispose() {
   if (SmoothScrollSequencer* sequencer = GetSmoothScrollSequencer())
     sequencer->DidDisposeScrollableArea(*this);
 
-  has_been_disposed_ = true;
+  layer_ = nullptr;
+}
+
+bool PaintLayerScrollableArea::HasBeenDisposed() const {
+  return !layer_;
 }
 
 void PaintLayerScrollableArea::Trace(blink::Visitor* visitor) {
@@ -676,10 +679,10 @@ void PaintLayerScrollableArea::ScrollbarVisibilityChanged() {
 
   // Paint properties need to be updated, because clip rects
   // are affected by overlay scrollbars.
-  layer_.GetLayoutObject().SetNeedsPaintPropertyUpdate();
+  layer_->GetLayoutObject().SetNeedsPaintPropertyUpdate();
 
   // TODO(chrishr): this should be able to be removed.
-  layer_.ClearClipRects();
+  layer_->ClearClipRects();
 
   if (LayoutView* view = GetLayoutBox()->View())
     view->ClearHitTestCache();
@@ -703,6 +706,8 @@ IntRect PaintLayerScrollableArea::ScrollableAreaBoundingBox() const {
 }
 
 void PaintLayerScrollableArea::RegisterForAnimation() {
+  if (HasBeenDisposed())
+    return;
   if (LocalFrame* frame = GetLayoutBox()->GetFrame()) {
     if (LocalFrameView* frame_view = frame->View())
       frame_view->AddAnimatingScrollableArea(this);
@@ -710,6 +715,8 @@ void PaintLayerScrollableArea::RegisterForAnimation() {
 }
 
 void PaintLayerScrollableArea::DeregisterForAnimation() {
+  if (HasBeenDisposed())
+    return;
   if (LocalFrame* frame = GetLayoutBox()->GetFrame()) {
     if (LocalFrameView* frame_view = frame->View())
       frame_view->RemoveAnimatingScrollableArea(this);
@@ -758,11 +765,11 @@ int PaintLayerScrollableArea::PageStep(ScrollbarOrientation orientation) const {
 }
 
 LayoutBox* PaintLayerScrollableArea::GetLayoutBox() const {
-  return layer_.GetLayoutBox();
+  return layer_->GetLayoutBox();
 }
 
 PaintLayer* PaintLayerScrollableArea::Layer() const {
-  return &layer_;
+  return layer_;
 }
 
 LayoutUnit PaintLayerScrollableArea::ScrollWidth() const {
@@ -1526,7 +1533,7 @@ void PaintLayerScrollableArea::PositionOverflowControls() {
     return;
 
   const IntRect border_box =
-      GetLayoutBox()->PixelSnappedBorderBoxRect(layer_.SubpixelAccumulation());
+      GetLayoutBox()->PixelSnappedBorderBoxRect(layer_->SubpixelAccumulation());
 
   if (Scrollbar* vertical_scrollbar = VerticalScrollbar())
     vertical_scrollbar->SetFrameRect(RectForVerticalScrollbar(border_box));
@@ -1981,7 +1988,7 @@ void PaintLayerScrollableArea::UpdateScrollableAreaSet() {
     frame_view->RemoveScrollableArea(this);
   }
 
-  layer_.DidUpdateScrollsOverflow();
+  layer_->DidUpdateScrollsOverflow();
 }
 
 void PaintLayerScrollableArea::UpdateCompositingLayersAfterScroll() {
@@ -2181,18 +2188,18 @@ void PaintLayerScrollableArea::ResetRebuildScrollbarLayerFlags() {
 
 CompositorAnimationHost* PaintLayerScrollableArea::GetCompositorAnimationHost()
     const {
-  return layer_.GetLayoutObject().GetFrameView()->GetCompositorAnimationHost();
+  return layer_->GetLayoutObject().GetFrameView()->GetCompositorAnimationHost();
 }
 
 CompositorAnimationTimeline*
 PaintLayerScrollableArea::GetCompositorAnimationTimeline() const {
-  return layer_.GetLayoutObject()
+  return layer_->GetLayoutObject()
       .GetFrameView()
       ->GetCompositorAnimationTimeline();
 }
 
 void PaintLayerScrollableArea::GetTickmarks(Vector<IntRect>& tickmarks) const {
-  if (layer_.IsRootLayer()) {
+  if (layer_->IsRootLayer()) {
     tickmarks = GetLayoutBox()
                     ->GetDocument()
                     .Markers()
diff --git a/chromium/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h b/chromium/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
index 4aa99dab352..150ec2c2acc 100644
--- a/chromium/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
+++ b/chromium/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
@@ -226,6 +226,7 @@ class CORE_EXPORT PaintLayerScrollableArea final
 
   ~PaintLayerScrollableArea() override;
   void Dispose();
+  bool HasBeenDisposed() const override;
 
   void ForceVerticalScrollbarForFirstLayout() { SetHasVerticalScrollbar(true); }
   bool HasHorizontalScrollbar() const { return HorizontalScrollbar(); }
@@ -558,7 +559,10 @@ class CORE_EXPORT PaintLayerScrollableArea final
 
   IntRect CornerRect(const IntRect& bounds) const;
 
-  PaintLayer& layer_;
+  // PaintLayer is destructed before PaintLayerScrollable area, during this
+  // time before PaintLayerScrollableArea has been collected layer_ will
+  // be set to nullptr by the Dispose method.
+  PaintLayer* layer_;
 
   PaintLayer* next_topmost_scroll_child_;
   PaintLayer* topmost_scroll_child_;
@@ -615,8 +619,6 @@ class CORE_EXPORT PaintLayerScrollableArea final
 
   // MainThreadScrollingReason due to the properties of the LayoutObject
   uint32_t non_composited_main_thread_scrolling_reasons_;
-
-  bool has_been_disposed_;
 };
 
 DEFINE_TYPE_CASTS(PaintLayerScrollableArea,
diff --git a/chromium/third_party/WebKit/Source/platform/scroll/ScrollableArea.cpp b/chromium/third_party/WebKit/Source/platform/scroll/ScrollableArea.cpp
index eaf0069d3b3..5cf58cd75b0 100644
--- a/chromium/third_party/WebKit/Source/platform/scroll/ScrollableArea.cpp
+++ b/chromium/third_party/WebKit/Source/platform/scroll/ScrollableArea.cpp
@@ -305,6 +305,11 @@ void ScrollableArea::ScrollOffsetChanged(const ScrollOffset& offset,
   // Tell the derived class to scroll its contents.
   UpdateScrollOffset(truncated_offset, scroll_type);
 
+  // If the layout object has been detached as a result of updating the scroll
+  // this object will be cleaned up shortly.
+  if (HasBeenDisposed())
+    return;
+
   // Tell the scrollbars to update their thumb postions.
   // If the scrollbar does not have its own layer, it must always be
   // invalidated to reflect the new thumb offset, even if the theme did not
diff --git a/chromium/third_party/WebKit/Source/platform/scroll/ScrollableArea.h b/chromium/third_party/WebKit/Source/platform/scroll/ScrollableArea.h
index f3756ba4250..9f605119b40 100644
--- a/chromium/third_party/WebKit/Source/platform/scroll/ScrollableArea.h
+++ b/chromium/third_party/WebKit/Source/platform/scroll/ScrollableArea.h
@@ -421,6 +421,8 @@ class PLATFORM_EXPORT ScrollableArea : public GarbageCollectedMixin {
   // then reset to alpha, causing spurrious "visibilityChanged" calls.
   virtual void ScrollbarVisibilityChanged() {}
 
+  virtual bool HasBeenDisposed() const { return false; }
+
  private:
   FRIEND_TEST_ALL_PREFIXES(ScrollableAreaTest,
                            PopupOverlayScrollbarShouldNotFadeOut);