RTL fixes for views::ScrollView

When scrolling with layers, flip the viewport with layer transforms
under RTL. This is necessary because the layer-backed scrolling
machinery is not privy to the automatic UI flipping done in
toolkit-views. ScrollOffsets must be positive, so scrolling
horizontally in RTL currently moves the contents to the left. Flipping
the viewport layer resolves this and scrolls to the right but reverses
the text, so flip the contents to compensate.

Testing exposed a quirk of View::ScrollRectToVisible() under RTL on
all platforms. It seems only Views::TreeView uses ScrollRectToVisible()
to affect horizontal scrolling, but it's currently broken (see
http://crbug.com/840236). Fix it so we can use it in the test.

Bug: 840236, 828361
Change-Id: Ib16724ff73829a6383be82e7bd11893b3a7e2ffc
Reviewed-on: https://chromium-review.googlesource.com/999076Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557778}

ui/views/controls/scroll_view.cc

@@ -512,6 +512,30 @@ void ScrollView::Layout() {
     container_size.SetToMax(viewport_bounds.size());
     contents_->SetBoundsRect(gfx::Rect(container_size));
     contents_->layer()->SetScrollable(viewport_bounds.size());
+
+    // Flip the viewport with layer transforms under RTL. Note the net effect is
+    // to flip twice, so the text is not mirrored. This is necessary because
+    // compositor scrolling is not RTL-aware. So although a toolkit-views layout
+    // will flip, increasing a horizontal gfx::ScrollOffset will move content to
+    // the left, regardless of RTL. A gfx::ScrollOffset must be positive, so to
+    // move (unscrolled) content to the right, we need to flip the viewport
+    // layer. That would flip all the content as well, so flip (and translate)
+    // the content layer. Compensating in this way allows the scrolling/offset
+    // logic to remain the same when scrolling via layers or bounds offsets.
+    if (base::i18n::IsRTL()) {
+      gfx::Transform flip;
+      flip.Translate(viewport_bounds.width(), 0);
+      flip.Scale(-1, 1);
+      contents_viewport_->layer()->SetTransform(flip);
+
+      // Add `contents_->width() - viewport_width` to the translation step. This
+      // is to prevent the top-left of the (flipped) contents aligning to the
+      // top-left of the viewport. Instead, the top-right should align in RTL.
+      gfx::Transform shift;
+      shift.Translate(2 * contents_->width() - viewport_bounds.width(), 0);
+      shift.Scale(-1, 1);
+      contents_->layer()->SetTransform(shift);
+    }
   }
 
   header_viewport_->SetBounds(contents_x, contents_y,

ui/views/controls/tree/tree_view.cc

@@ -261,8 +261,13 @@ void TreeView::SetSelectedNode(TreeModelNode* model_node) {
     SchedulePaintForNode(selected_node_);
   }
 
-  if (selected_node_)
-    ScrollRectToVisible(GetForegroundBoundsForNode(selected_node_));
+  if (selected_node_) {
+    // GetForegroundBoundsForNode() returns RTL-flipped coordinates for paint.
+    // Un-flip before passing to ScrollRectToVisible(), which uses layout
+    // coordinates.
+    ScrollRectToVisible(
+        GetMirroredRect(GetForegroundBoundsForNode(selected_node_)));
+  }
 
   // Notify controller if the old selection was empty to handle the case of
   // remove explicitly resetting selected_node_ before invoking this.
@@ -738,6 +743,9 @@ void TreeView::LayoutEditor() {
   DCHECK(selected_node_);
   // Position the editor so that its text aligns with the text we drew.
   gfx::Rect row_bounds = GetForegroundBoundsForNode(selected_node_);
+
+  // GetForegroundBoundsForNode() returns a "flipped" x for painting. First, un-
+  // flip it for the following calculations and ScrollRectToVisible().
   row_bounds.set_x(
       GetMirroredXWithWidthInView(row_bounds.x(), row_bounds.width()));
   row_bounds.set_x(row_bounds.x() + text_offset_);

ui/views/view.cc

@@ -514,7 +514,10 @@ gfx::Transform View::GetTransform() const {
 
   gfx::Transform transform = layer()->transform();
   gfx::ScrollOffset scroll_offset = layer()->CurrentScrollOffset();
-  transform.Translate(-scroll_offset.x(), -scroll_offset.y());
+  // Offsets for layer-based scrolling are never negative, but the horizontal
+  // scroll direction is reversed in RTL via canvas flipping.
+  transform.Translate((base::i18n::IsRTL() ? 1 : -1) * scroll_offset.x(),
+                      -scroll_offset.y());
   return transform;
 }
 
@@ -1464,13 +1467,8 @@ void View::OnAccessibilityEvent(ax::mojom::Event event_type) {}
 // Scrolling -------------------------------------------------------------------
 
 void View::ScrollRectToVisible(const gfx::Rect& rect) {
-  // We must take RTL UI mirroring into account when adjusting the position of
-  // the region.
-  if (parent_) {
-    gfx::Rect scroll_rect(rect);
-    scroll_rect.Offset(GetMirroredX(), y());
-    parent_->ScrollRectToVisible(scroll_rect);
-  }
+  if (parent_)
+    parent_->ScrollRectToVisible(rect + bounds().OffsetFromOrigin());
 }
 
 void View::ScrollViewToVisible() {