Remove LocalFrameView::FrameIsScrollableDidChange

The flag has been no use after BGPT.

Change-Id: I00e71065799516bc1388ba9fe00dd4217292ea3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1945949
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722721}

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

@@ -1169,11 +1169,6 @@ void LocalFrameView::AddBackgroundAttachmentFixedObject(LayoutObject* object) {
   DCHECK(!background_attachment_fixed_objects_.Contains(object));
 
   background_attachment_fixed_objects_.insert(object);
-  if (ScrollingCoordinator* scrolling_coordinator =
-          this->GetScrollingCoordinator()) {
-    scrolling_coordinator
-        ->FrameViewHasBackgroundAttachmentFixedObjectsDidChange(this);
-  }
 
   // Ensure main thread scrolling reasons are recomputed.
   SetNeedsPaintPropertyUpdate();
@@ -1186,11 +1181,6 @@ void LocalFrameView::RemoveBackgroundAttachmentFixedObject(
   DCHECK(background_attachment_fixed_objects_.Contains(object));
 
   background_attachment_fixed_objects_.erase(object);
-  if (ScrollingCoordinator* scrolling_coordinator =
-          this->GetScrollingCoordinator()) {
-    scrolling_coordinator
-        ->FrameViewHasBackgroundAttachmentFixedObjectsDidChange(this);
-  }
 
   // Ensure main thread scrolling reasons are recomputed.
   SetNeedsPaintPropertyUpdate();
@@ -1221,24 +1211,12 @@ void LocalFrameView::AddViewportConstrainedObject(LayoutObject& object) {
   if (!viewport_constrained_objects_)
     viewport_constrained_objects_ = std::make_unique<ObjectSet>();
 
-  if (!viewport_constrained_objects_->Contains(&object)) {
-    viewport_constrained_objects_->insert(&object);
-
-    if (ScrollingCoordinator* scrolling_coordinator =
-            this->GetScrollingCoordinator())
-      scrolling_coordinator->FrameViewFixedObjectsDidChange(this);
-  }
+  viewport_constrained_objects_->insert(&object);
 }
 
 void LocalFrameView::RemoveViewportConstrainedObject(LayoutObject& object) {
-  if (viewport_constrained_objects_ &&
-      viewport_constrained_objects_->Contains(&object)) {
+  if (viewport_constrained_objects_)
     viewport_constrained_objects_->erase(&object);
-
-    if (ScrollingCoordinator* scrolling_coordinator =
-            this->GetScrollingCoordinator())
-      scrolling_coordinator->FrameViewFixedObjectsDidChange(this);
-  }
 }
 
 void LocalFrameView::ViewportSizeChanged(bool width_changed,
@@ -3385,17 +3363,6 @@ void LocalFrameView::ScheduleAnimation(base::TimeDelta delay) {
     client->ScheduleAnimation(this, delay);
 }
 
-bool LocalFrameView::FrameIsScrollableDidChange() {
-  DCHECK(GetFrame().IsLocalRoot());
-  return GetScrollingContext()->WasScrollable() !=
-         LayoutViewport()->ScrollsOverflow();
-}
-
-void LocalFrameView::ClearFrameIsScrollableDidChange() {
-  GetScrollingContext()->SetWasScrollable(
-      GetFrame().LocalFrameRoot().View()->LayoutViewport()->ScrollsOverflow());
-}
-
 void LocalFrameView::ScrollableAreasDidChange() {
   // Layout may update scrollable area bounding boxes. It also sets the same
   // dirty flag making this one redundant (See

third_party/blink/renderer/core/frame/local_frame_view.h

@@ -640,12 +640,6 @@ class CORE_EXPORT LocalFrameView final
 
   const cc::Layer* RootCcLayer() const;
 
-  // Keeps track of whether the scrollable state for the LocalRoot has changed
-  // since ScrollingCoordinator last checked. Only ScrollingCoordinator should
-  // ever call the clearing function.
-  bool FrameIsScrollableDidChange();
-  void ClearFrameIsScrollableDidChange();
-
   // Should be called whenever this LocalFrameView adds or removes a
   // scrollable area, or gains/loses a composited layer.
   void ScrollableAreasDidChange();

third_party/blink/renderer/core/page/BUILD.gn

@@ -81,7 +81,6 @@ blink_core_sources("page") {
     "scrolling/scroll_state_callback.h",
     "scrolling/scrolling_coordinator.cc",
     "scrolling/scrolling_coordinator.h",
-    "scrolling/scrolling_coordinator_context.cc",
     "scrolling/scrolling_coordinator_context.h",
     "scrolling/snap_coordinator.cc",
     "scrolling/snap_coordinator.h",

third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc

@@ -76,7 +76,6 @@ void ScrollingCoordinator::Trace(blink::Visitor* visitor) {
 void ScrollingCoordinator::NotifyGeometryChanged(LocalFrameView* frame_view) {
   frame_view->GetScrollingContext()->SetScrollGestureRegionIsDirty(true);
   frame_view->GetScrollingContext()->SetTouchEventTargetRectsAreDirty(true);
-  frame_view->GetScrollingContext()->SetShouldScrollOnMainThreadIsDirty(true);
 }
 
 ScrollableArea*
@@ -128,6 +127,8 @@ void ScrollingCoordinator::DidChangeScrollbarsHidden(
 }
 
 void ScrollingCoordinator::UpdateAfterPaint(LocalFrameView* frame_view) {
+  DCHECK(!RuntimeEnabledFeatures::CompositeAfterPaintEnabled());
+
   LocalFrame* frame = &frame_view->GetFrame();
   DCHECK(frame->IsLocalRoot());
 
@@ -135,14 +136,9 @@ void ScrollingCoordinator::UpdateAfterPaint(LocalFrameView* frame_view) {
       frame_view->GetScrollingContext()->ScrollGestureRegionIsDirty();
   bool touch_event_rects_dirty =
       frame_view->GetScrollingContext()->TouchEventTargetRectsAreDirty();
-  bool should_scroll_on_main_thread_dirty =
-      frame_view->GetScrollingContext()->ShouldScrollOnMainThreadIsDirty();
-  bool frame_scroller_dirty = FrameScrollerIsDirty(frame_view);
 
-  if (!(scroll_gesture_region_dirty || touch_event_rects_dirty ||
-        should_scroll_on_main_thread_dirty || frame_scroller_dirty)) {
+  if (!scroll_gesture_region_dirty && !touch_event_rects_dirty)
     return;
-  }
 
   SCOPED_UMA_AND_UKM_TIMER(frame_view->EnsureUkmAggregator(),
                            LocalFrameUkmAggregator::kScrollingCoordinator);
@@ -153,24 +149,10 @@ void ScrollingCoordinator::UpdateAfterPaint(LocalFrameView* frame_view) {
     frame_view->GetScrollingContext()->SetScrollGestureRegionIsDirty(false);
   }
 
-  if (!(touch_event_rects_dirty || should_scroll_on_main_thread_dirty ||
-        frame_scroller_dirty)) {
-    return;
-  }
-
   if (touch_event_rects_dirty) {
     UpdateTouchEventTargetRectsIfNeeded(frame);
     frame_view->GetScrollingContext()->SetTouchEventTargetRectsAreDirty(false);
   }
-
-  if (should_scroll_on_main_thread_dirty ||
-      frame_view->FrameIsScrollableDidChange()) {
-    // TODO(pdr): Now that BlinkGenPropertyTrees has launched, we should remove
-    // FrameIsScrollableDidChange.
-    frame_view->GetScrollingContext()->SetShouldScrollOnMainThreadIsDirty(
-        false);
-  }
-  frame_view->ClearFrameIsScrollableDidChange();
 }
 
 template <typename Function>
@@ -478,7 +460,6 @@ void ScrollingCoordinator::Reset(LocalFrame* frame) {
 
   horizontal_scrollbars_.clear();
   vertical_scrollbars_.clear();
-  frame->View()->ClearFrameIsScrollableDidChange();
 }
 
 void ScrollingCoordinator::TouchEventTargetRectsDidChange(LocalFrame* frame) {
@@ -565,29 +546,6 @@ bool ScrollingCoordinator::CoordinatesScrollingForFrameView(
   return layout_view->UsesCompositing();
 }
 
-void ScrollingCoordinator::
-    FrameViewHasBackgroundAttachmentFixedObjectsDidChange(
-        LocalFrameView* frame_view) {
-  DCHECK(IsMainThread());
-  DCHECK(frame_view);
-
-  if (!CoordinatesScrollingForFrameView(frame_view))
-    return;
-
-  frame_view->GetScrollingContext()->SetShouldScrollOnMainThreadIsDirty(true);
-}
-
-void ScrollingCoordinator::FrameViewFixedObjectsDidChange(
-    LocalFrameView* frame_view) {
-  DCHECK(IsMainThread());
-  DCHECK(frame_view);
-
-  if (!CoordinatesScrollingForFrameView(frame_view))
-    return;
-
-  frame_view->GetScrollingContext()->SetShouldScrollOnMainThreadIsDirty(true);
-}
-
 bool ScrollingCoordinator::IsForMainFrame(
     ScrollableArea* scrollable_area) const {
   if (!IsA<LocalFrame>(page_->MainFrame()))
@@ -609,21 +567,4 @@ void ScrollingCoordinator::FrameViewRootLayerDidChange(
   NotifyGeometryChanged(frame_view);
 }
 
-bool ScrollingCoordinator::FrameScrollerIsDirty(
-    LocalFrameView* frame_view) const {
-  DCHECK(frame_view);
-  // TODO(bokan): This should probably be checking the root scroller in the
-  // FrameView, rather than the frame_view.
-  if (frame_view->FrameIsScrollableDidChange())
-    return true;
-
-  if (cc::Layer* scroll_layer =
-          frame_view->LayoutViewport()->LayerForScrolling()) {
-    return static_cast<gfx::Size>(
-               frame_view->LayoutViewport()->ContentsSize()) !=
-           scroll_layer->bounds();
-  }
-  return false;
-}
-
 }  // namespace blink

third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.h

@@ -84,18 +84,10 @@ class CORE_EXPORT ScrollingCoordinator final
   // Called when any frame has done its layout or compositing has changed.
   void NotifyGeometryChanged(LocalFrameView*);
 
-  // Update non-fast scrollable regions, touch event target rects, main thread
-  // scrolling reasons, and whether the visual viewport is user scrollable.
+  // Update non-fast scrollable regions and touch event target rects.
   // TODO(pdr): Refactor this out of ScrollingCoordinator.
   void UpdateAfterPaint(LocalFrameView*);
 
-  // Should be called whenever the slow repaint objects counter changes between
-  // zero and one.
-  void FrameViewHasBackgroundAttachmentFixedObjectsDidChange(LocalFrameView*);
-
-  // Should be called whenever the set of fixed objects changes.
-  void FrameViewFixedObjectsDidChange(LocalFrameView*);
-
   // Should be called whenever the root layer for the given frame view changes.
   void FrameViewRootLayerDidChange(LocalFrameView*);
 
@@ -172,8 +164,6 @@ class CORE_EXPORT ScrollingCoordinator final
                                             ScrollbarOrientation);
   void RemoveScrollbarLayer(ScrollableArea*, ScrollbarOrientation);
 
-  bool FrameScrollerIsDirty(LocalFrameView*) const;
-
   cc::AnimationHost* animation_host_ = nullptr;
   std::unique_ptr<CompositorAnimationTimeline>
       programmatic_scroll_animator_timeline_;

third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_context.cc

-// Copyright 2018 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 "third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_context.h"
-
-namespace blink {
-
-void ScrollingCoordinatorContext::SetAnimationTimeline(
-    std::unique_ptr<CompositorAnimationTimeline> timeline) {
-  animation_timeline_ = std::move(timeline);
-}
-
-void ScrollingCoordinatorContext::SetAnimationHost(cc::AnimationHost* host) {
-  animation_host_ = host;
-}
-
-CompositorAnimationTimeline*
-ScrollingCoordinatorContext::GetCompositorAnimationTimeline() {
-  return animation_timeline_.get();
-}
-
-cc::AnimationHost* ScrollingCoordinatorContext::GetCompositorAnimationHost() {
-  return animation_host_;
-}
-
-bool ScrollingCoordinatorContext::ScrollGestureRegionIsDirty() const {
-  return scroll_gesture_region_is_dirty_;
-}
-
-bool ScrollingCoordinatorContext::TouchEventTargetRectsAreDirty() const {
-  return touch_event_target_rects_are_dirty_;
-}
-
-bool ScrollingCoordinatorContext::ShouldScrollOnMainThreadIsDirty() const {
-  return should_scroll_on_main_thread_is_dirty_;
-}
-
-bool ScrollingCoordinatorContext::WasScrollable() const {
-  return was_scrollable_;
-}
-
-void ScrollingCoordinatorContext::SetScrollGestureRegionIsDirty(bool dirty) {
-  scroll_gesture_region_is_dirty_ = dirty;
-}
-
-void ScrollingCoordinatorContext::SetTouchEventTargetRectsAreDirty(bool dirty) {
-  touch_event_target_rects_are_dirty_ = dirty;
-}
-
-void ScrollingCoordinatorContext::SetShouldScrollOnMainThreadIsDirty(
-    bool dirty) {
-  should_scroll_on_main_thread_is_dirty_ = dirty;
-}
-
-void ScrollingCoordinatorContext::SetWasScrollable(bool was_scrollable) {
-  was_scrollable_ = was_scrollable;
-}
-
-}  // namespace blink

third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_context.h

@@ -25,27 +25,33 @@ class CORE_EXPORT ScrollingCoordinatorContext final {
   ScrollingCoordinatorContext() {}
   virtual ~ScrollingCoordinatorContext() {}
 
-  void SetAnimationTimeline(std::unique_ptr<CompositorAnimationTimeline>);
-  void SetAnimationHost(cc::AnimationHost*);
+  void SetAnimationTimeline(
+      std::unique_ptr<CompositorAnimationTimeline> timeline) {
+    animation_timeline_ = std::move(timeline);
+  }
+  void SetAnimationHost(cc::AnimationHost* host) { animation_host_ = host; }
 
-  CompositorAnimationTimeline* GetCompositorAnimationTimeline();
-  cc::AnimationHost* GetCompositorAnimationHost();
+  CompositorAnimationTimeline* GetCompositorAnimationTimeline() {
+    return animation_timeline_.get();
+  }
+  cc::AnimationHost* GetCompositorAnimationHost() { return animation_host_; }
 
   // Non-fast scrollable regions need updating by ScrollingCoordinator.
-  bool ScrollGestureRegionIsDirty() const;
+  bool ScrollGestureRegionIsDirty() const {
+    return scroll_gesture_region_is_dirty_;
+  }
   // Touch event target rects need updating by ScrollingCoordinator.
-  bool TouchEventTargetRectsAreDirty() const;
-  // ScrollingCoordinator should update whether or not scrolling for this
-  // subtree has to happen on the main thread.
-  bool ShouldScrollOnMainThreadIsDirty() const;
-  bool WasScrollable() const;
+  bool TouchEventTargetRectsAreDirty() const {
+    return touch_event_target_rects_are_dirty_;
+  }
 
   // Only ScrollingCoordinator should ever set |dirty| to |false|.
-  void SetScrollGestureRegionIsDirty(bool dirty);
-  void SetTouchEventTargetRectsAreDirty(bool dirty);
-  void SetShouldScrollOnMainThreadIsDirty(bool dirty);
-
-  void SetWasScrollable(bool was_scrollable);
+  void SetScrollGestureRegionIsDirty(bool dirty) {
+    scroll_gesture_region_is_dirty_ = dirty;
+  }
+  void SetTouchEventTargetRectsAreDirty(bool dirty) {
+    touch_event_target_rects_are_dirty_ = dirty;
+  }
 
  private:
   std::unique_ptr<CompositorAnimationTimeline> animation_timeline_;
@@ -53,8 +59,6 @@ class CORE_EXPORT ScrollingCoordinatorContext final {
 
   bool scroll_gesture_region_is_dirty_ = false;
   bool touch_event_target_rects_are_dirty_ = false;
-  bool should_scroll_on_main_thread_is_dirty_ = false;
-  bool was_scrollable_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(ScrollingCoordinatorContext);
 };

third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_test.cc

@@ -1206,37 +1206,6 @@ TEST_P(ScrollingCoordinatorTest, setupScrollbarLayerShouldSetScrollLayerOpaque)
             contents_layer->contents_opaque());
 }
 
-// LocalFrameView::FrameIsScrollableDidChange is used as a dirty bit and is
-// set to clean in ScrollingCoordinator::UpdateAfterPaint. This test ensures
-// that the dirty bit is set and unset properly.
-TEST_P(ScrollingCoordinatorTest, FrameIsScrollableDidChange) {
-  LoadHTML(R"HTML(
-    <div id='bg' style='background: red; width: 10px; height: 10px;'></div>
-    <div id='forcescroll' style='height: 5000px;'></div>
-  )HTML");
-
-  // Initially there is a change but that goes away after a compositing update.
-  EXPECT_TRUE(GetFrame()->View()->FrameIsScrollableDidChange());
-  ForceFullCompositingUpdate();
-  EXPECT_FALSE(GetFrame()->View()->FrameIsScrollableDidChange());
-
-  // A change to background color should not change the frame's scrollability.
-  auto* background = GetFrame()->GetDocument()->getElementById("bg");
-  background->removeAttribute(html_names::kStyleAttr);
-  EXPECT_FALSE(GetFrame()->View()->FrameIsScrollableDidChange());
-
-  ForceFullCompositingUpdate();
-
-  // Making the frame not scroll should change the frame's scrollability.
-  auto* forcescroll = GetFrame()->GetDocument()->getElementById("forcescroll");
-  forcescroll->removeAttribute(html_names::kStyleAttr);
-  GetFrame()->View()->UpdateLifecycleToLayoutClean();
-  EXPECT_TRUE(GetFrame()->View()->FrameIsScrollableDidChange());
-
-  ForceFullCompositingUpdate();
-  EXPECT_FALSE(GetFrame()->View()->FrameIsScrollableDidChange());
-}
-
 TEST_P(ScrollingCoordinatorTest, NestedIFramesMainThreadScrollingRegion) {
   // This page has an absolute IFRAME. It contains a scrollable child DIV
   // that's nested within an intermediate IFRAME.

third_party/blink/renderer/core/paint/compositing/compositing_layer_assigner.cc

@@ -292,15 +292,6 @@ void CompositingLayerAssigner::AssignLayersToBackingsInternal(
             layer, composited_layer_update)) {
       layers_needing_paint_invalidation.push_back(layer);
       layers_changed_ = true;
-      if (ScrollingCoordinator* scrolling_coordinator =
-              layer->GetScrollingCoordinator()) {
-        if (layer->GetLayoutObject()
-                .StyleRef()
-                .HasViewportConstrainedPosition()) {
-          scrolling_coordinator->FrameViewFixedObjectsDidChange(
-              layer->GetLayoutObject().View()->GetFrameView());
-        }
-      }
     }
 
     if (composited_layer_update != kNoCompositingStateChange) {

third_party/blink/renderer/core/paint/compositing/paint_layer_compositor.cc

@@ -563,14 +563,6 @@ bool PaintLayerCompositor::AllocateOrClearCompositedLayerMapping(
 
   layer->ClearClipRects(kPaintingClipRects);
 
-  // If a fixed position layer gained/lost a compositedLayerMapping or the
-  // reason not compositing it changed, the scrolling coordinator needs to
-  // recalculate whether it can do fast scrolling.
-  if (ScrollingCoordinator* scrolling_coordinator = GetScrollingCoordinator()) {
-    scrolling_coordinator->FrameViewFixedObjectsDidChange(
-        layout_view_.GetFrameView());
-  }
-
   // Compositing state affects whether to create paint offset translation of
   // this layer, and amount of paint offset translation of descendants.
   layer->GetLayoutObject().SetNeedsPaintPropertyUpdate();