Save/restore scroll offsets while printing.

mstensho · Commit Bot · commit eb97bb4a01a1 · 2020-09-24T08:00:56.000Z
When attempting to clamp the scroll offset during printing, save the scroll offset before clamping, so that it can be restored after printing. Note that there are still issues to fix here. Right now, this CL doesn't fix anything at all, but it will prevent a regression that would otherwise be introduced by crrev.com/2362749. This is what's tested in PrintBrowserTest.NoScrollingVerticalRl PrintBrowserTest.NoScrolling tests scrolling of an iframe and a scrollable DIV, and another scrollable DIV that turns into a non-scrollable flexbox when printed. The iframe part also passes without this CL, and the scrollable DIV ones fail both without and with this CL (and is therefore disabled for now). Same problem for framesets - still not fixed. Added a test. The remaining known scrolling issues when printing are reported as bug 1131598. Based on WIP CL:2402190 by szager. Bug: 1125972 Change-Id: I6c3f5853d329e4731b02b33d67cbd4b016f2675f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418651 Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Reviewed-by: Stefan Zager <szager@chromium.org> Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Cr-Commit-Position: refs/heads/master@{#810103}
diff --git a/chrome/browser/printing/print_browsertest.cc b/chrome/browser/printing/print_browsertest.cc
@@ -530,6 +530,69 @@ IN_PROC_BROWSER_TEST_F(PrintBrowserTest, SelectionContainsIframe) {
   PrintAndWaitUntilPreviewIsReady(/*print_only_selection=*/true);
 }
 
+// https://crbug.com/1125972
+// https://crbug.com/1131598
+IN_PROC_BROWSER_TEST_F(PrintBrowserTest, NoScrolling) {
+  ASSERT_TRUE(embedded_test_server()->Started());
+  GURL url(embedded_test_server()->GetURL("/printing/with-scrollable.html"));
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  auto* contents = browser()->tab_strip_model()->GetActiveWebContents();
+  const char kExpression1[] = "iframe.contentWindow.scrollY";
+  const char kExpression2[] = "scrollable.scrollTop";
+  const char kExpression3[] = "shapeshifter.scrollTop";
+
+  double old_scroll1 = content::EvalJs(contents, kExpression1).ExtractDouble();
+  double old_scroll2 = content::EvalJs(contents, kExpression2).ExtractDouble();
+  double old_scroll3 = content::EvalJs(contents, kExpression3).ExtractDouble();
+
+  PrintAndWaitUntilPreviewIsReady(/*print_only_selection=*/false);
+
+  double new_scroll1 = content::EvalJs(contents, kExpression1).ExtractDouble();
+
+  // TODO(crbug.com/1131598): Perform the corresponding EvalJs() calls here and
+  // assign to new_scroll2 and new_scroll3, once the printing code has been
+  // fixed to handle these cases. Right now, the scroll offset jumps.
+  double new_scroll2 = old_scroll2;
+  double new_scroll3 = old_scroll3;
+
+  EXPECT_EQ(old_scroll1, new_scroll1);
+  EXPECT_EQ(old_scroll2, new_scroll2);
+  EXPECT_EQ(old_scroll3, new_scroll3);
+}
+
+// https://crbug.com/1131598
+IN_PROC_BROWSER_TEST_F(PrintBrowserTest, DISABLED_NoScrollingFrameset) {
+  ASSERT_TRUE(embedded_test_server()->Started());
+  GURL url(embedded_test_server()->GetURL("/printing/frameset.html"));
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  auto* contents = browser()->tab_strip_model()->GetActiveWebContents();
+  const char kExpression[] =
+      "document.getElementById('frame').contentWindow.scrollY";
+
+  double old_scroll = content::EvalJs(contents, kExpression).ExtractDouble();
+
+  PrintAndWaitUntilPreviewIsReady(/*print_only_selection=*/false);
+
+  double new_scroll = content::EvalJs(contents, kExpression).ExtractDouble();
+
+  EXPECT_EQ(old_scroll, new_scroll);
+}
+
+// https://crbug.com/1125972
+IN_PROC_BROWSER_TEST_F(PrintBrowserTest, NoScrollingVerticalRl) {
+  ASSERT_TRUE(embedded_test_server()->Started());
+  GURL url(embedded_test_server()->GetURL("/printing/vertical-rl.html"));
+  ui_test_utils::NavigateToURL(browser(), url);
+  PrintAndWaitUntilPreviewIsReady(/*print_only_selection=*/false);
+
+  // Test that entering print preview didn't mess up the scroll position.
+  EXPECT_EQ(
+      0, content::EvalJs(browser()->tab_strip_model()->GetActiveWebContents(),
+                         "window.scrollX"));
+}
+
 // Printing frame content for the main frame of a generic webpage.
 // This test passes when the printed result is sent back and checked in
 // TestPrintRenderFrame::OnDidPrintFrameContent().
diff --git a/chrome/test/data/printing/frameset.html b/chrome/test/data/printing/frameset.html
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<frameset cols="100%">
+  <frame id="frame" src="scrolled-frame.html"></frame>
+</frameset>
diff --git a/chrome/test/data/printing/iframe-with-square.html b/chrome/test/data/printing/iframe-with-square.html
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<style>
+  body { margin:0; }
+</style>
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+<div id="target" style="width:100px; height:100px; background:green;"></div>
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
+filler filler filler filler filler filler filler filler filler filler filler
diff --git a/chrome/test/data/printing/scrolled-frame.html b/chrome/test/data/printing/scrolled-frame.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<style>
+  body { margin: 0; }
+</style>
+<script>
+  for (var i = 0; i < 10000; i++)
+      document.write("filler ");
+</script>
+
+<div id="target" style="width:100px; height:100px; background:green;"></div>
+
+<script>
+  for (var i = 0; i < 10000; i++)
+      document.write("filler ");
+  window.scrollTo(0, target.offsetTop);
+</script>
diff --git a/chrome/test/data/printing/vertical-rl.html b/chrome/test/data/printing/vertical-rl.html
@@ -0,0 +1,5 @@
+<!DOCTYPE html>
+<style>
+  html { writing-mode:vertical-rl; }
+</style>
+<div style="block-size:10000px;"></div>
diff --git a/chrome/test/data/printing/with-scrollable.html b/chrome/test/data/printing/with-scrollable.html
@@ -0,0 +1,60 @@
+<!DOCTYPE html>
+<style>
+  #iframe {
+      display: block;
+      width: 100%;
+      height: 100px;
+  }
+  #scrollable {
+      position: relative;
+      overflow: auto;
+      height: 100px;
+  }
+  #shapeshifter {
+      position: relative;
+      overflow: auto;
+      height: 100px;
+  }
+  @media print {
+      #shapeshifter {
+          display: flex;
+          overflow: visible;
+      }
+
+      #shapeshifter * {
+          display: none;
+      }
+  }
+</style>
+<iframe id="iframe" src="iframe-with-square.html"></iframe>
+<div id="scrollable">
+  <script>
+    for (var i = 0; i < 200; i++)
+        document.write("filler ");
+  </script>
+  <div id="scrollabletarget" style="width:100px; height:100px; background:green;"></div>
+  <script>
+    for (var i = 0; i < 200; i++)
+        document.write("filler ");
+  </script>
+</div>
+<div id="shapeshifter">
+  <script>
+    for (var i = 0; i < 200; i++)
+        document.write("filler ");
+  </script>
+  <div id="shapeshiftertarget" style="width:100px; height:100px; background:green;"></div>
+  <script>
+    for (var i = 0; i < 200; i++)
+        document.write("filler ");
+  </script>
+</div>
+
+<script>
+  onload = function() {
+      var scroll = iframe.contentWindow.document.getElementById("target").offsetTop;
+      iframe.contentWindow.scrollTo(0, scroll);
+      scrollable.scrollTop = scrollabletarget.offsetTop;
+      shapeshifter.scrollTop = shapeshiftertarget.offsetTop;
+  }
+</script>
diff --git a/components/printing/renderer/print_render_frame_helper.cc b/components/printing/renderer/print_render_frame_helper.cc
@@ -825,7 +825,6 @@ class PrepareFrameAndViewForPrint : public blink::WebViewClient,
   bool owns_web_view_ = false;
   blink::WebPrintParams web_print_params_;
   gfx::Size prev_view_size_;
-  gfx::Size prev_scroll_offset_;
   uint32_t expected_pages_count_ = 0;
   base::OnceClosure on_ready_;
   const bool should_print_backgrounds_;
@@ -895,15 +894,6 @@ void PrepareFrameAndViewForPrint::ResizeForPrinting() {
   if (IsPrintingNodeOrPdfFrame(frame(), node_to_print_))
     return;
 
-  // Backup size and offset if it's a local frame.
-  blink::WebView* web_view = frame_.view();
-  if (blink::WebFrame* web_frame = web_view->MainFrame()) {
-    // TODO(lukasza, weili): Support restoring scroll offset of a remote main
-    // frame - https://crbug.com/734815.
-    if (web_frame->IsWebLocalFrame())
-      prev_scroll_offset_ = web_frame->ToWebLocalFrame()->GetScrollOffset();
-  }
-
   prev_view_size_ = frame()->LocalRoot()->FrameWidget()->Size();
   frame()->LocalRoot()->FrameWidget()->Resize(print_layout_size);
 }
@@ -1036,13 +1026,6 @@ void PrepareFrameAndViewForPrint::RestoreSize() {
     return;
 
   frame()->LocalRoot()->FrameWidget()->Resize(prev_view_size_);
-  blink::WebView* web_view = frame_.GetFrame()->View();
-  if (blink::WebFrame* web_frame = web_view->MainFrame()) {
-    // TODO(lukasza, weili): Support restoring scroll offset of a remote main
-    // frame - https://crbug.com/734815.
-    if (web_frame->IsWebLocalFrame())
-      web_frame->ToWebLocalFrame()->SetScrollOffset(prev_scroll_offset_);
-  }
 }
 
 void PrepareFrameAndViewForPrint::FinishPrinting() {
diff --git a/third_party/blink/renderer/core/dom/container_node.cc b/third_party/blink/renderer/core/dom/container_node.cc
@@ -658,6 +658,10 @@ void ContainerNode::WillRemoveChildren() {
       ChildFrameDisconnector::kDescendantsOnly);
 }
 
+LayoutBox* ContainerNode::GetLayoutBoxForScrolling() const {
+  return GetLayoutBox();
+}
+
 void ContainerNode::Trace(Visitor* visitor) const {
   visitor->Trace(first_child_);
   visitor->Trace(last_child_);
diff --git a/third_party/blink/renderer/core/dom/container_node.h b/third_party/blink/renderer/core/dom/container_node.h
@@ -385,6 +385,11 @@ class CORE_EXPORT ContainerNode : public Node {
 
   virtual bool ChildrenCanHaveStyle() const { return true; }
 
+  // This is similar to GetLayoutBox(), but returns nullptr if it's not
+  // scrollable. Some elements override this to delegate scroll operations to
+  // a descendant LayoutBox.
+  virtual LayoutBox* GetLayoutBoxForScrolling() const;
+
   void Trace(Visitor*) const override;
 
  protected:
diff --git a/third_party/blink/renderer/core/dom/element.h b/third_party/blink/renderer/core/dom/element.h
@@ -323,10 +323,7 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable {
   void scrollBy(const ScrollToOptions*);
   void scrollTo(double x, double y);
   void scrollTo(const ScrollToOptions*);
-  // This is similar to GetLayoutBox(), but returns nullptr if it's not
-  // scrollable. Some elements override this to delegate scroll operations to
-  // a descendant LayoutBox.
-  virtual LayoutBox* GetLayoutBoxForScrolling() const;
+  LayoutBox* GetLayoutBoxForScrolling() const override;
 
   IntRect BoundsInViewport() const;
   // Returns an intersection rectangle of the bounds rectangle and the visual
diff --git a/third_party/blink/renderer/core/frame/local_frame.cc b/third_party/blink/renderer/core/frame/local_frame.cc
@@ -155,6 +155,7 @@
 #include "third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h"
 #include "third_party/blink/renderer/core/paint/compositing/paint_layer_compositor.h"
 #include "third_party/blink/renderer/core/paint/object_painter.h"
+#include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
 #include "third_party/blink/renderer/core/paint/paint_timing.h"
 #include "third_party/blink/renderer/core/probe/core_probes.h"
 #include "third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.h"
@@ -464,6 +465,7 @@ void LocalFrame::Trace(Visitor* visitor) const {
   visitor->Trace(main_frame_receiver_);
   visitor->Trace(high_priority_frame_receiver_);
   visitor->Trace(text_fragment_selector_generator_);
+  visitor->Trace(saved_scroll_offsets_);
   Frame::Trace(visitor);
   Supplementable<LocalFrame>::Trace(visitor);
 }
@@ -969,10 +971,12 @@ scoped_refptr<InspectorTaskRunner> LocalFrame::GetInspectorTaskRunner() {
 void LocalFrame::StartPrinting(const FloatSize& page_size,
                                const FloatSize& original_page_size,
                                float maximum_shrink_ratio) {
+  DCHECK(!saved_scroll_offsets_);
   SetPrinting(true, page_size, original_page_size, maximum_shrink_ratio);
 }
 
 void LocalFrame::EndPrinting() {
+  RestoreScrollOffsets();
   SetPrinting(false, FloatSize(), FloatSize(), 0);
 }
 
@@ -1046,6 +1050,33 @@ bool LocalFrame::ShouldUsePrintingLayout() const {
                       : Client()->UsePrintingLayout();
 }
 
+void LocalFrame::EnsureSaveScrollOffset(Node& node) {
+  const auto* scrollable_area = PaintLayerScrollableArea::FromNode(node);
+  if (!scrollable_area)
+    return;
+  if (!saved_scroll_offsets_)
+    saved_scroll_offsets_ = MakeGarbageCollected<SavedScrollOffsets>();
+  // Retain the first scroll offset saved for each scrollable area.
+  if (!saved_scroll_offsets_->Contains(&node))
+    saved_scroll_offsets_->Set(&node, scrollable_area->GetScrollOffset());
+}
+
+void LocalFrame::RestoreScrollOffsets() {
+  if (!saved_scroll_offsets_)
+    return;
+
+  // Restore scroll offsets unconditionally (i.e. without clamping) in case
+  // layout or view sizes haven't been updated yet.
+  for (auto& entry : *saved_scroll_offsets_) {
+    auto* scrollable_area = PaintLayerScrollableArea::FromNode(*entry.key);
+    if (!scrollable_area)
+      continue;
+    scrollable_area->SetScrollOffsetUnconditionally(
+        entry.value, mojom::blink::ScrollType::kProgrammatic);
+  }
+  saved_scroll_offsets_ = nullptr;
+}
+
 FloatSize LocalFrame::ResizePageRectsKeepingRatio(
     const FloatSize& original_size,
     const FloatSize& expected_size) const {
diff --git a/third_party/blink/renderer/core/frame/local_frame.h b/third_party/blink/renderer/core/frame/local_frame.h
@@ -297,6 +297,14 @@ class CORE_EXPORT LocalFrame final
 
   void EndPrinting();
   bool ShouldUsePrintingLayout() const;
+
+  // Save the current scroll offset of the scrollable area associated with the
+  // given node (if not already saved). All saved scroll offsets can be restored
+  // via RestoreScrollOffsets() (this will also clear all entries for saved
+  // scroll offsets).
+  void EnsureSaveScrollOffset(Node&);
+  void RestoreScrollOffsets();
+
   FloatSize ResizePageRectsKeepingRatio(const FloatSize& original_size,
                                         const FloatSize& expected_size) const;
 
@@ -906,6 +914,9 @@ class CORE_EXPORT LocalFrame final
   // Access to the global raw/unsanitized system clipboard
   Member<RawSystemClipboard> raw_system_clipboard_;
 
+  using SavedScrollOffsets = HeapHashMap<Member<Node>, ScrollOffset>;
+  Member<SavedScrollOffsets> saved_scroll_offsets_;
+
   mojom::blink::BlinkOptimizationGuideHintsPtr optimization_guide_hints_;
 
   Member<TextFragmentSelectorGenerator> text_fragment_selector_generator_;
diff --git a/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc b/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
@@ -169,6 +169,11 @@ PaintLayerScrollableArea::~PaintLayerScrollableArea() {
   CHECK(HasBeenDisposed());
 }
 
+PaintLayerScrollableArea* PaintLayerScrollableArea::FromNode(const Node& node) {
+  const LayoutBox* box = node.GetLayoutBox();
+  return box ? box->GetScrollableArea() : nullptr;
+}
+
 void PaintLayerScrollableArea::DidScroll(const FloatPoint& position) {
   ScrollableArea::DidScroll(position);
   // This should be alive if it receives composited scroll callbacks.
@@ -1156,6 +1161,18 @@ void PaintLayerScrollableArea::ClampScrollOffsetAfterOverflowChange() {
     return;
   }
 
+  const Document& document = GetLayoutBox()->GetDocument();
+  if (document.IsCapturingLayout()) {
+    // Scrollable elements may change size when generating layout for printing,
+    // which may require them to change the scroll position in order to keep the
+    // same content within view. In vertical-rl writing-mode, even the root
+    // frame may be attempted scrolled, because a viewport size change may
+    // affect scroll origin. Save all scroll offsets before clamping, so that
+    // everything can be restored the way it was after printing.
+    if (Node* node = EventTargetNode())
+      document.GetFrame()->EnsureSaveScrollOffset(*node);
+  }
+
   UpdateScrollDimensions();
   if (ScrollOriginChanged()) {
     SetScrollOffsetUnconditionally(ClampScrollOffset(GetScrollOffset()));
diff --git a/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h b/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h
@@ -252,6 +252,9 @@ class CORE_EXPORT PaintLayerScrollableArea final
   explicit PaintLayerScrollableArea(PaintLayer&);
   ~PaintLayerScrollableArea() override;
 
+  // Return the PaintLayerScrollableArea (if any) associated with the Node.
+  static PaintLayerScrollableArea* FromNode(const Node&);
+
   void ForceVerticalScrollbarForFirstLayout() { SetHasVerticalScrollbar(true); }
   bool HasHorizontalScrollbar() const { return HorizontalScrollbar(); }
   bool HasVerticalScrollbar() const { return VerticalScrollbar(); }
