Fix for ScrollableArea pre-finalizer CHECK failure

PaintLayer holds a persistent pointer to PaintLayerScrollableArea, so
it's guaranteed that the PaintLayer will be destructed before the PLSA
pre-finalizer is called. PLSA::Dispose is called by the PaintLayer
destructor to clean up state, and this is the appropriate time to run
complex finalizer code such as RunScrollCompleteCallbacks.

The other ScrollableArea sub-classes are RootFrameViewport and
VisualViewport. They only get destructed when the entire page is being
torn down, in which case it's pointless to try and run scroll complete
callbacks; so remove that code path for those classes.

BUG=983809
R=omerkatz@chromium.org,pdr@chromium.org

Change-Id: I28bee057db042ba92bacea62aeeae130b7594fdb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1713593Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680633}

third_party/blink/renderer/core/frame/visual_viewport.cc

@@ -1226,7 +1226,6 @@ void VisualViewport::DisposeImpl() {
   scroll_node_.reset();
   horizontal_scrollbar_effect_node_.reset();
   vertical_scrollbar_effect_node_.reset();
-  ScrollableArea::DisposeImpl();
 }
 
 }  // namespace blink

third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc

@@ -159,7 +159,9 @@ void PaintLayerScrollableArea::DidScroll(const FloatPoint& position) {
   CHECK(!HasBeenDisposed());
 }
 
-void PaintLayerScrollableArea::Dispose() {
+void PaintLayerScrollableArea::DisposeImpl() {
+  rare_data_.reset();
+
   if (InResizeMode() && !GetLayoutBox()->DocumentBeingDestroyed()) {
     if (LocalFrame* frame = GetLayoutBox()->GetFrame())
       frame->GetEventHandler().ResizeScrollableAreaDestroyed();
@@ -212,11 +214,9 @@ void PaintLayerScrollableArea::Dispose() {
   if (SmoothScrollSequencer* sequencer = GetSmoothScrollSequencer())
     sequencer->DidDisposeScrollableArea(*this);
 
-  layer_ = nullptr;
-}
+  RunScrollCompleteCallbacks();
 
-bool PaintLayerScrollableArea::HasBeenDisposed() const {
-  return !layer_;
+  layer_ = nullptr;
 }
 
 void PaintLayerScrollableArea::Trace(blink::Visitor* visitor) {
@@ -2929,11 +2929,4 @@ bool PaintLayerScrollableArea::ScrollingBackgroundDisplayItemClient::
       ->PaintedOutputOfObjectHasNoEffectRegardlessOfSize();
 }
 
-void PaintLayerScrollableArea::DisposeImpl() {
-  if (!HasBeenDisposed())
-    Dispose();
-  rare_data_.reset();
-  ScrollableArea::DisposeImpl();
-}
-
 }  // namespace blink

third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h

@@ -249,8 +249,6 @@ class CORE_EXPORT PaintLayerScrollableArea final
 
   explicit PaintLayerScrollableArea(PaintLayer&);
   ~PaintLayerScrollableArea() override;
-  void Dispose();
-  bool HasBeenDisposed() const override;
 
   void ForceVerticalScrollbarForFirstLayout() { SetHasVerticalScrollbar(true); }
   bool HasHorizontalScrollbar() const { return HorizontalScrollbar(); }
@@ -545,7 +543,7 @@ class CORE_EXPORT PaintLayerScrollableArea final
   }
 
   void DisposeImpl() override;
-  
+
  private:
   bool NeedsScrollbarReconstruction() const;
 

third_party/blink/renderer/core/scroll/scroll_animator_test.cc

@@ -138,10 +138,7 @@ class MockScrollableAreaForAnimatorTest
     ScrollableArea::Trace(visitor);
   }
 
-  void DisposeImpl() override {
-      timer_task_runner_.reset();
-      ScrollableArea::DisposeImpl();
-  }
+  void DisposeImpl() override { timer_task_runner_.reset(); }
 
  private:
   bool scroll_animator_enabled_;

third_party/blink/renderer/core/scroll/scrollable_area.cc

@@ -90,18 +90,18 @@ ScrollableArea::ScrollableArea()
       scrollbars_hidden_if_overlay_(true),
       scrollbar_captured_(false),
       mouse_over_scrollbar_(false),
+      has_been_disposed_(false),
       needs_show_scrollbar_layers_(false),
       uses_composited_scrolling_(false) {}
 
 ScrollableArea::~ScrollableArea() = default;
 
 void ScrollableArea::Dispose() {
+  if (HasBeenDisposed())
+    return;
   DisposeImpl();
-}
-
-void ScrollableArea::DisposeImpl() {
-  RunScrollCompleteCallbacks();
   fade_overlay_scrollbars_timer_.reset();
+  has_been_disposed_ = true;
 }
 
 void ScrollableArea::ClearScrollableArea() {
@@ -431,6 +431,7 @@ void ScrollableArea::UpdateScrollOffsetFromInternals(const IntSize& offset) {
 }
 
 void ScrollableArea::RegisterScrollCompleteCallback(ScrollCallback callback) {
+  DCHECK(!HasBeenDisposed());
   pending_scroll_complete_callbacks_.push_back(std::move(callback));
 }
 

third_party/blink/renderer/core/scroll/scrollable_area.h

@@ -351,7 +351,7 @@ class CORE_EXPORT ScrollableArea : public GarbageCollectedMixin {
   virtual ~ScrollableArea();
 
   void Dispose();
-  virtual void DisposeImpl();
+  virtual void DisposeImpl() {}
 
   // Called when any of horizontal scrollbar, vertical scrollbar and scroll
   // corner is setNeedsPaintInvalidation.
@@ -465,7 +465,7 @@ class CORE_EXPORT ScrollableArea : public GarbageCollectedMixin {
   // then reset to alpha, causing spurrious "visibilityChanged" calls.
   virtual void ScrollbarVisibilityChanged() {}
 
-  virtual bool HasBeenDisposed() const { return false; }
+  bool HasBeenDisposed() const { return has_been_disposed_; }
 
  private:
   FRIEND_TEST_ALL_PREFIXES(ScrollableAreaTest,
@@ -504,6 +504,7 @@ class CORE_EXPORT ScrollableArea : public GarbageCollectedMixin {
   unsigned scrollbars_hidden_if_overlay_ : 1;
   unsigned scrollbar_captured_ : 1;
   unsigned mouse_over_scrollbar_ : 1;
+  unsigned has_been_disposed_ : 1;
 
   // Indicates that the next compositing update needs to call
   // cc::Layer::ShowScrollbars() on our scroll layer. Ignored if not composited.