Scroll with Layers in views::ScrollView

ui/views/controls/scroll_view.cc

Index: ui/views/controls/scroll_view.cc
diff --git a/ui/views/controls/scroll_view.cc b/ui/views/controls/scroll_view.cc
index a3e1e9787a2ae32b3118c2d0ed9837d808f94d25..8c7495f1df342985b0336cc6c251796e3be97603 100644
--- a/ui/views/controls/scroll_view.cc
+++ b/ui/views/controls/scroll_view.cc
@@ -4,14 +4,16 @@
 
 #include "ui/views/controls/scroll_view.h"
 
+#include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "ui/events/event.h"
 #include "ui/gfx/canvas.h"
 #include "ui/native_theme/native_theme.h"
+#include "ui/views/background.h"
 #include "ui/views/border.h"
 #include "ui/views/style/platform_style.h"
-#include "ui/views/widget/root_view.h"
+#include "ui/views/widget/widget.h"
 
 namespace views {
 
@@ -19,6 +21,15 @@ const char ScrollView::kViewClassName[] = "ScrollView";
 
 namespace {
 
+const base::Feature kToolkitViewsScrollWithLayers {
+  "ToolkitViewsScrollWithLayers",
+#if defined(OS_MACOSX)
+      base::FEATURE_ENABLED_BY_DEFAULT
+#else
+      base::FEATURE_DISABLED_BY_DEFAULT
+#endif
+};
+
 // Subclass of ScrollView that resets the border when the theme changes.
 class ScrollViewWithBorder : public views::ScrollView {
  public:
@@ -64,15 +75,23 @@ int CheckScrollBounds(int viewport_size, int content_size, int current_pos) {
 }
 
 // Make sure the content is not scrolled out of bounds
-void CheckScrollBounds(View* viewport, View* view) {
+void CheckScrollBounds(View* viewport, View* container, View* view) {
   if (!view)
     return;
 
-  int x = CheckScrollBounds(viewport->width(), view->width(), -view->x());
-  int y = CheckScrollBounds(viewport->height(), view->height(), -view->y());
+  gfx::ScrollOffset offset = container
+                                 ? container->layer()->CurrentScrollOffset()
+                                 : gfx::ScrollOffset(-view->x(), -view->y());
+
+  int x = CheckScrollBounds(viewport->width(), view->width(), offset.x());
+  int y = CheckScrollBounds(viewport->height(), view->height(), offset.y());
 
-  // This is no op if bounds are the same
-  view->SetBounds(-x, -y, view->width(), view->height());
+  if (container) {
+    container->layer()->SetScrollOffset(gfx::ScrollOffset(x, y));
+  } else {
+    // This is no op if bounds are the same
+    view->SetBounds(-x, -y, view->width(), view->height());
+  }
 }
 
 // Used by ScrollToPosition() to make sure the new position fits within the
@@ -89,6 +108,19 @@ int AdjustPosition(int current_position,
   return (new_position > max_position) ? max_position : new_position;
 }
 
+class ScrollViewContainer : public View {
+ public:
+  ScrollViewContainer() {}
+
+  // View:
+  void ChildPreferredSizeChanged(View* child) override {
+    PreferredSizeChanged();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ScrollViewContainer);
+};
+
 }  // namespace
 
 // Viewport contains the contents View of the ScrollView.
@@ -105,9 +137,18 @@ class ScrollView::Viewport : public View {
 
     View* contents = child_at(0);
     gfx::Rect scroll_rect(rect);
-    scroll_rect.Offset(-contents->x(), -contents->y());
-    static_cast<ScrollView*>(parent())->ScrollContentsRegionToBeVisible(
-        scroll_rect);
+
+    ScrollView* scroll_view = static_cast<ScrollView*>(parent());
+    if (contents == scroll_view->contents_container_) {
+      // With layer scrolling, there's no need to "undo" the offset done in the
+      // child's View::ScrollRectToVisible() before it calls this.
+      DCHECK_EQ(0, contents->x());
+      DCHECK_EQ(0, contents->y());
+    } else {
+      scroll_rect.Offset(-contents->x(), -contents->y());
+    }
+
+    scroll_view->ScrollContentsRegionToBeVisible(scroll_rect);
   }
 
   void ChildPreferredSizeChanged(View* child) override {
@@ -121,6 +162,7 @@ class ScrollView::Viewport : public View {
 
 ScrollView::ScrollView()
     : contents_(NULL),
+      contents_container_(nullptr),
       contents_viewport_(new Viewport()),
       header_(NULL),
       header_viewport_(new Viewport()),
@@ -142,6 +184,26 @@ ScrollView::ScrollView()
   vert_sb_->SetVisible(false);
   vert_sb_->set_controller(this);
   corner_view_->SetVisible(false);
+
+  if (!base::FeatureList::IsEnabled(kToolkitViewsScrollWithLayers))
+    return;
+
+  contents_container_ = new ScrollViewContainer();
+  contents_viewport_->AddChildView(contents_container_);
+  contents_viewport_->SetPaintToLayer(true);
+
+  // TODO(tapted): Support setting the background to something other than white,

[tapted, 2016/08/05 07:12:31]

Without this TODO - the ExtensionInstall dialog would get a white scrollview on
a grey background :/. But it's a small change to include it (done) - see what
you think. We're close to launching a dev-channel experiment
(http://crbug.com/633436), so I shouldn't regress things..

+  // if something ends up needing that.
+  contents_viewport_->set_background(
+      Background::CreateSolidBackground(SK_ColorWHITE));
+  contents_viewport_->layer()->SetMasksToBounds(true);
+
+  contents_container_->SetPaintToLayer(true);
+  contents_container_->set_background(
+      Background::CreateSolidBackground(SK_ColorWHITE));
+  contents_container_->layer()->SetScrollable(
+      contents_viewport_->layer(),
+      base::Bind(&ScrollView::OnLayerScrolled, base::Unretained(this)));
 }
 
 ScrollView::~ScrollView() {
@@ -158,7 +220,12 @@ ScrollView* ScrollView::CreateScrollViewWithBorder() {
 }
 
 void ScrollView::SetContents(View* a_view) {
-  SetHeaderOrContents(contents_viewport_, a_view, &contents_);
+  // Protect against clients passing a contents view that has its own Layer.
+  // While this will probably work, it's wasteful since its container will have
+  // a layer of its own.
+  DCHECK(!a_view->layer());

[tapted, 2016/08/04 13:19:01]

added a dcheck here per the comments in the overview CL - crrev/2189583004 . Of
course, client code could just create a layer after calling SetContents, so I'm
not sure how useful it is..

I also did an audit with combinations of git grep and codesearch -- I didn't
find anything currently using a layer-backed contents. The closest was
message_center_view.cc setting the ScrollView itself to have a layer, but not
the contents -
https://cs.chromium.org/chromium/src/ui/message_center/views/message_center_v...
(this went in for r153049 - I think so it can be swiped off the screen in ash
smoothly)

+  View* parent = contents_container_ ? contents_container_ : contents_viewport_;
+  SetHeaderOrContents(parent, a_view, &contents_);
 }
 
 void ScrollView::SetHeader(View* header) {
@@ -168,8 +235,9 @@ void ScrollView::SetHeader(View* header) {
 gfx::Rect ScrollView::GetVisibleRect() const {
   if (!contents_)
     return gfx::Rect();
-  return gfx::Rect(-contents_->x(), -contents_->y(),
-                   contents_viewport_->width(), contents_viewport_->height());
+  gfx::ScrollOffset offset = CurrentOffset();
+  return gfx::Rect(offset.x(), offset.y(), contents_viewport_->width(),
+                   contents_viewport_->height());
 }
 
 void ScrollView::ClipHeightTo(int min_height, int max_height) {
@@ -267,10 +335,15 @@ void ScrollView::Layout() {
   // Update the bounds right now so the inner views can fit in it.
   contents_viewport_->SetBoundsRect(viewport_bounds);
 
-  // Give |contents_| a chance to update its bounds if it depends on the
-  // viewport.
-  if (contents_)
+  // Give |contents_| a chance to update its bounds if it depends on its parent
+  // bounds.
+  if (contents_) {
+    // The container bounds may need to be made larger later, but use the
+    // viewport bounds to layout the contents.
+    if (contents_container_)
+      contents_container_->SetBoundsRect(viewport_bounds);
     contents_->Layout();
+  }
 
   bool should_layout_contents = false;
   bool horiz_sb_required = false;
@@ -326,16 +399,29 @@ void ScrollView::Layout() {
 
   // Update to the real client size with the visible scrollbars.
   contents_viewport_->SetBoundsRect(viewport_bounds);
-  if (should_layout_contents && contents_)
+  if (should_layout_contents && contents_) {
+    if (contents_container_)
+      contents_container_->SetBoundsRect(viewport_bounds);
     contents_->Layout();
+  }
+
+  // Even when |contents_| needs to scroll, it can still be narrower or wider
+  // the viewport. So ensure the scrolling layer can fill the viewport, so that
+  // events will correctly hit it, and overscroll looks correct.
+  if (contents_container_) {
+    gfx::Size container_size = contents_ ? contents_->size() : gfx::Size();
+    container_size.SetToMax(viewport_bounds.size());
+    contents_container_->SetBoundsRect(gfx::Rect(container_size));
+  }
 
   header_viewport_->SetBounds(contents_x, contents_y,
                               viewport_bounds.width(), header_height);
   if (header_)
     header_->Layout();
 
-  CheckScrollBounds(header_viewport_, header_);
-  CheckScrollBounds(contents_viewport_, contents_);
+  CheckScrollBounds(header_viewport_, nullptr, header_);
+  CheckScrollBounds(contents_viewport_, contents_container_, contents_);
+
   SchedulePaint();
   UpdateScrollBarPositions();
 }
@@ -402,28 +488,32 @@ const char* ScrollView::GetClassName() const {
   return kViewClassName;
 }
 
+ScrollView* ScrollView::EnclosingScrollView() {
+  return this;
+}
+
 void ScrollView::ScrollToPosition(ScrollBar* source, int position) {
   if (!contents_)
     return;
 
+  gfx::ScrollOffset offset = CurrentOffset();
   if (source == horiz_sb_ && horiz_sb_->visible()) {
-    position = AdjustPosition(contents_->x(), position, contents_->width(),
+    position = AdjustPosition(offset.x(), position, contents_->width(),
                               contents_viewport_->width());
-    if (-contents_->x() == position)
+    if (offset.x() == position)
       return;
-    contents_->SetX(-position);
-    if (header_) {
-      header_->SetX(-position);
-      header_->SchedulePaintInRect(header_->GetVisibleBounds());
-    }
+    offset.set_x(position);
   } else if (source == vert_sb_ && vert_sb_->visible()) {
-    position = AdjustPosition(contents_->y(), position, contents_->height(),
+    position = AdjustPosition(offset.y(), position, contents_->height(),
                               contents_viewport_->height());
-    if (-contents_->y() == position)
+    if (offset.y() == position)
       return;
-    contents_->SetY(-position);
+    offset.set_y(position);
   }
-  contents_->SchedulePaintInRect(contents_->GetVisibleBounds());
+  ScrollToOffset(offset);
+
+  if (!contents_container_)
+    contents_->SchedulePaintInRect(contents_->GetVisibleBounds());
 }
 
 int ScrollView::GetScrollIncrement(ScrollBar* source, bool is_page,
@@ -504,10 +594,7 @@ void ScrollView::ScrollContentsRegionToBeVisible(const gfx::Rect& rect) {
       (vis_rect.y() > y) ? y : std::max(0, max_y -
                                         contents_viewport_->height());
 
-  contents_->SetX(-new_x);
-  if (header_)
-    header_->SetX(-new_x);
-  contents_->SetY(-new_y);
+  ScrollToOffset(gfx::ScrollOffset(new_x, new_y));
   UpdateScrollBarPositions();
 }
 
@@ -555,17 +642,52 @@ void ScrollView::UpdateScrollBarPositions() {
   if (!contents_)
     return;
 
+  const gfx::ScrollOffset offset = CurrentOffset();
   if (horiz_sb_->visible()) {
     int vw = contents_viewport_->width();
     int cw = contents_->width();
-    int origin = contents_->x();
-    horiz_sb_->Update(vw, cw, -origin);
+    horiz_sb_->Update(vw, cw, offset.x());
   }
   if (vert_sb_->visible()) {
     int vh = contents_viewport_->height();
     int ch = contents_->height();
-    int origin = contents_->y();
-    vert_sb_->Update(vh, ch, -origin);
+    vert_sb_->Update(vh, ch, offset.y());
+  }
+}
+
+gfx::ScrollOffset ScrollView::CurrentOffset() const {
+  if (contents_container_)
+    return contents_container_->layer()->CurrentScrollOffset();
+  return gfx::ScrollOffset(-contents_->x(), -contents_->y());
+}
+
+void ScrollView::ScrollToOffset(const gfx::ScrollOffset& offset) {
+  if (contents_container_) {
+    contents_container_->layer()->SetScrollOffset(offset);
+
+    // TODO(tapted): Remove this call to OnLayerScrolled(). It's unnecessary,
+    // but will only be invoked (asynchronously) when a Compositor is present
+    // and commits a frame, which isn't true in some tests.
+    OnLayerScrolled();
+  } else {
+    contents_->SetPosition(gfx::Point(-offset.x(), -offset.y()));
+    ScrollHeader();
+  }
+}
+
+void ScrollView::OnLayerScrolled() {
+  UpdateScrollBarPositions();
+  ScrollHeader();
+}
+
+void ScrollView::ScrollHeader() {
+  if (!header_)
+    return;
+
+  int x_offset = CurrentOffset().x();
+  if (header_->x() != -x_offset) {
+    header_->SetX(-x_offset);
+    header_->SchedulePaintInRect(header_->GetVisibleBounds());
   }
 }