PDF Viewer: Remove logic for topToolbarHeight in new Viewport class.

When PDFViewerUpdate is on, |topToolbarHeight| is always zero. The code
that uses |topToolbarHeight| in calculations is effectively a no-op and
can be deleted.

This CL reverts a subset of the changes in r789720, as they are no
longer needed.

Bug: None
Change-Id: Icd6a6b7b91ae2137e9a81dd68c3c53ad031a3840
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2541607Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Auto-Submit: dpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828298}

chrome/browser/resources/pdf/pdf_viewer.js

@@ -333,11 +333,6 @@ export class PDFViewerElement extends PDFViewerBaseElement {
     return this.toolbarEnabled_ ? MATERIAL_TOOLBAR_HEIGHT : 0;
   }
 
-  /** @override */
-  hasFixedToolbar() {
-    return this.pdfViewerUpdateEnabled_;
-  }
-
   /** @override */
   getContent() {
     return /** @type {!HTMLDivElement} */ (this.$$('#content'));

chrome/browser/resources/pdf/pdf_viewer_base.js

@@ -115,11 +115,6 @@ export class PDFViewerBaseElement extends PolymerElement {
     return 0;
   }
 
-  /** @return {boolean} Whether the top toolbar is fixed (does not auto-hide) */
-  hasFixedToolbar() {
-    return false;
-  }
-
   /**
    * @return {!HTMLDivElement}
    * @protected
@@ -248,8 +243,7 @@ export class PDFViewerBaseElement extends PolymerElement {
 
     this.viewport_ = new Viewport(
         scrollContainer, this.getSizer(), this.getContent(),
-        getScrollbarWidth(), defaultZoom, this.getToolbarHeight(),
-        this.hasFixedToolbar());
+        getScrollbarWidth(), defaultZoom, this.getToolbarHeight());
     this.viewport_.setViewportChangedCallback(() => this.viewportChanged_());
     this.viewport_.setBeforeZoomCallback(
         () => this.currentController.beforeZoom());

chrome/browser/resources/pdf/viewport.js

@@ -75,12 +75,10 @@ export class Viewport {
    * @param {number} defaultZoom The default zoom level.
    * @param {number} topToolbarHeight The number of pixels that should initially
    *     be left blank above the document for the toolbar.
-   * @param {boolean} topToolbarFixed True if the top toolbar is fixed and does
-   *     not automatically disappear in fit to page mode.
    */
   constructor(
       scrollParent, sizer, content, scrollbarWidth, defaultZoom,
-      topToolbarHeight, topToolbarFixed) {
+      topToolbarHeight) {
     /** @private {!HTMLElement} */
     this.window_ = scrollParent;
 
@@ -99,9 +97,6 @@ export class Viewport {
     /** @private {number} */
     this.topToolbarHeight_ = topToolbarHeight;
 
-    /** @private {boolean} */
-    this.topToolbarFixed_ = topToolbarFixed;
-
     /** @private {function():void} */
     this.viewportChangedCallback_ = function() {};
 
@@ -812,13 +807,9 @@ export class Viewport {
             'true.');
 
     // First compute the zoom without scrollbars.
-    let height = this.window_.offsetHeight;
-    if (this.topToolbarFixed_) {
-      height -= this.topToolbarHeight_;
-    }
     let zoom = this.computeFittingZoomGivenDimensions_(
-        fitWidth, fitHeight, this.window_.offsetWidth, height,
-        pageDimensions.width, pageDimensions.height);
+        fitWidth, fitHeight, this.window_.offsetWidth,
+        this.window_.offsetHeight, pageDimensions.width, pageDimensions.height);
 
     // Check if there needs to be any scrollbars.
     const needsScrollbars = this.documentNeedsScrollbars(zoom);
@@ -844,7 +835,7 @@ export class Viewport {
     // Compute available window space.
     const windowWithScrollbars = {
       width: this.window_.offsetWidth,
-      height: height,
+      height: this.window_.offsetHeight,
     };
     if (needsScrollbars.horizontal) {
       windowWithScrollbars.height -= scrollbarWidth;
@@ -940,10 +931,9 @@ export class Viewport {
       };
       this.setZoomInternal_(this.computeFittingZoom_(dimensions, false, true));
       if (scrollToTopOfPage) {
-        const offset = this.topToolbarFixed_ ? this.topToolbarHeight_ : 0;
         this.position = {
           x: 0,
-          y: this.pageDimensions_[page].y * this.getZoom() - offset,
+          y: this.pageDimensions_[page].y * this.getZoom(),
         };
       }
       this.updateViewport_();
@@ -976,10 +966,9 @@ export class Viewport {
       };
       this.setZoomInternal_(this.computeFittingZoom_(dimensions, true, true));
       if (scrollToTopOfPage) {
-        const offset = this.topToolbarFixed_ ? this.topToolbarHeight_ : 0;
         this.position = {
           x: 0,
-          y: this.pageDimensions_[page].y * this.getZoom() - offset,
+          y: this.pageDimensions_[page].y * this.getZoom(),
         };
       }
       this.updateViewport_();
@@ -1239,7 +1228,7 @@ export class Viewport {
       // Unless we're in fit to page or fit to height mode, scroll above the
       // page by |this.topToolbarHeight_| so that the toolbar isn't covering it
       // initially.
-      if (!this.isPagedMode_() || this.topToolbarFixed_) {
+      if (!this.isPagedMode_()) {
         toolbarOffset = this.topToolbarHeight_;
       }
       this.position = {

chrome/test/data/pdf/test_util.js

@@ -318,7 +318,7 @@ export function getZoomableViewport(
   const viewport = new Viewport(
       /** @type {!HTMLElement} */ (scrollParent),
       /** @type {!HTMLDivElement} */ (sizer), dummyContent, scrollbarWidth,
-      defaultZoom, topToolbarHeight, false);
+      defaultZoom, topToolbarHeight);
   viewport.setZoomFactorRange([0.25, 0.4, 0.5, 1, 2]);
   return viewport;
 }

chrome/test/data/pdf/viewport_test.js

@@ -447,77 +447,6 @@ const tests = [
     chrome.test.succeed();
   },
 
-  // Verifies that fitting computations work correctly for a fixed toolbar. In
-  // this case, the viewport should fit the document to the area below the
-  // toolbar, rather than the full window.
-  function testFitToPageFixedToolbar() {
-    const mockWindow = new MockElement(100, 100, null);
-    const mockSizer = new MockSizer();
-    const mockCallback = new MockViewportChangedCallback();
-    const dummyContent =
-        /** @type {!HTMLDivElement} */ (document.createElement('div'));
-    document.body.appendChild(dummyContent);
-    const toolbarHeight = 10;
-    /** @suppress {invalidCasts} */
-    const viewport = new Viewport(
-        /** @type {!HTMLElement} */ (mockWindow),
-        /** @type {!HTMLDivElement} */ (mockSizer), dummyContent, 0, 1,
-        toolbarHeight, true);
-    viewport.setZoomFactorRange([0.25, 0.4, 0.5, 1, 2]);
-    viewport.setViewportChangedCallback(mockCallback.callback);
-    const documentDimensions = new MockDocumentDimensions();
-
-    function assertZoomed(expectedMockWidth, expectedMockHeight, expectedZoom) {
-      chrome.test.assertEq(FittingType.FIT_TO_PAGE, viewport.fittingType);
-      chrome.test.assertTrue(mockCallback.wasCalled);
-      chrome.test.assertEq(`${expectedMockWidth}px`, mockSizer.style.width);
-      chrome.test.assertEq(`${expectedMockHeight}px`, mockSizer.style.height);
-      chrome.test.assertEq(expectedZoom, viewport.getZoom());
-      // Should also make sure the viewport is correctly scrolled so that the
-      // entire page is visible, rather than covered by the toolbar.
-      chrome.test.assertEq(-1 * toolbarHeight, viewport.position.y);
-      chrome.test.assertEq(0, viewport.position.x);
-    }
-
-    function testForSize(
-        pageWidth, pageHeight, expectedMockWidth, expectedMockHeight,
-        expectedZoom) {
-      documentDimensions.reset();
-      documentDimensions.addPage(pageWidth, pageHeight);
-      viewport.setDocumentDimensions(documentDimensions);
-      viewport.setZoom(0.1);
-      mockCallback.reset();
-      viewport.fitToPage();
-      assertZoomed(expectedMockWidth, expectedMockHeight, expectedZoom);
-    }
-
-    // Note: Toolbar height is added to all expected heights below, as the sizer
-    // height includes the height of the toolbar.
-
-    // Page size which matches the window size.
-    testForSize(100, 100, 90, 90 + toolbarHeight, .9);
-
-    // Page size whose width is larger than its height.
-    testForSize(200, 100, 100, 50 + toolbarHeight, 0.5);
-
-    // Page size whose height is larger than its width.
-    testForSize(100, 200, 45, 90 + toolbarHeight, 0.45);
-
-    // Page size smaller than the window size in width but not height.
-    testForSize(50, 100, 45, 90 + toolbarHeight, .9);
-
-    // Page size smaller than the window size in height but not width.
-    testForSize(100, 50, 100, 50 + toolbarHeight, 1);
-
-    // Page size smaller than the window size in both width and height.
-    testForSize(25, 50, 45, 90 + toolbarHeight, 1.8);
-
-    // Page size smaller in one dimension and bigger in another.
-    testForSize(60, 200, 27, 90 + toolbarHeight, 0.45);
-
-    chrome.test.succeed();
-  },
-
   function testFitToHeight() {
     const mockWindow = new MockElement(100, 100, null);
     const mockSizer = new MockSizer();