OOP PDF: Fix incorrect scrolling during fit-to-page

chrome/browser/resources/pdf/viewport.js

Index: chrome/browser/resources/pdf/viewport.js
diff --git a/chrome/browser/resources/pdf/viewport.js b/chrome/browser/resources/pdf/viewport.js
index 29b86ceb27de5392a8df010176fe06f5a16caf2f..2b5b47dacdb8e1a47679bc6ed1ef8950dca123d3 100644
--- a/chrome/browser/resources/pdf/viewport.js
+++ b/chrome/browser/resources/pdf/viewport.js
@@ -155,7 +155,7 @@ Viewport.prototype = {
    */
   resize_: function() {
     if (this.fittingType_ == Viewport.FittingType.FIT_TO_PAGE)
-      this.fitToPage();
+      this.fitToPageAndScroll_(false);
     else if (this.fittingType_ == Viewport.FittingType.FIT_TO_WIDTH)
       this.fitToWidth();
     else
@@ -407,10 +407,13 @@ Viewport.prototype = {
   },
 
   /**
-   * Zoom the viewport so that a page consumes the entire viewport. Also scrolls
-   * to the top of the most visible page.
+   * @private
+   * Zoom the viewport so that a page consumes the entire viewport.
+   * @param {boolean} scrollToTopOfPage Set to true if the viewport should be
+   *     scrolled to the top of the current page. Set to false if the viewport
+   *     should remain at the current scroll position.
    */
-  fitToPage: function() {
+  fitToPageAndScroll_: function(scrollToTopOfPage) {

[Sam McNally, 2015/02/17 03:42:21]

This name suggests that it will always scroll somewhere.

[raymes, 2015/02/17 03:51:57]

Done.

     this.mightZoom_(function() {
       this.fittingType_ = Viewport.FittingType.FIT_TO_PAGE;
       if (!this.documentDimensions_)
@@ -421,13 +424,26 @@ Viewport.prototype = {
         width: this.documentDimensions_.width,
         height: this.pageDimensions_[page].height,
       };
+      // Track the last y-position to stay at the same position after zooming.
+      var oldY = this.window_.pageYOffset / this.zoom_;
       this.setZoomInternal_(this.computeFittingZoom_(dimensions, false));
-      this.window_.scrollTo(0, this.pageDimensions_[page].y * this.zoom_);
+      if (scrollToTopOfPage)
+        this.window_.scrollTo(0, this.pageDimensions_[page].y * this.zoom_);
+      else
+        this.window_.scrollTo(0, oldY * this.zoom_);

[Sam McNally, 2015/02/17 03:42:21]

Shouldn't setZoomInternal_ do this?

[raymes, 2015/02/17 03:51:57]

I guess the only difference is we want to scroll to 0 x-offset. I made this
change.

       this.updateViewport_();
     }.bind(this));
   },
 
   /**
+   * Zoom the viewport so that a page consumes the entire viewport. Also scrolls
+   * the viewport to the top of the current page.
+   */
+  fitToPage: function() {
+    this.fitToPageAndScroll_(true);
+  },
+
+  /**
    * Zoom out to the next predefined zoom level.
    */
   zoomOut: function() {