Add front-end support to enable two-up view status.

Implement a two-up view toggle button in PDF Viewer's zoom toolbar.
This button is available when feature "pdf-two-up-view" is enabled.

Bug: 51472
Change-Id: Icf6867e2897f1090b6b9fd8e9494006f648ffdd8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2012837Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Commit-Queue: Hui Yingst <nigi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748840}

chrome/browser/extensions/api/resources_private/resources_private_api.cc

@@ -60,6 +60,8 @@ void AddStringsForPdf(base::DictionaryValue* dict) {
     {"tooltipPrint", IDS_PDF_TOOLTIP_PRINT},
     {"tooltipFitToPage", IDS_PDF_TOOLTIP_FIT_PAGE},
     {"tooltipFitToWidth", IDS_PDF_TOOLTIP_FIT_WIDTH},
+    {"tooltipTwoUpViewEnable", IDS_PDF_TOOLTIP_TWO_UP_VIEW_ENABLE},
+    {"tooltipTwoUpViewDisable", IDS_PDF_TOOLTIP_TWO_UP_VIEW_DISABLE},
     {"tooltipZoomIn", IDS_PDF_TOOLTIP_ZOOM_IN},
     {"tooltipZoomOut", IDS_PDF_TOOLTIP_ZOOM_OUT},
 #if defined(OS_CHROMEOS)
@@ -130,6 +132,9 @@ void AddAdditionalDataForPdf(base::DictionaryValue* dict) {
   dict->SetKey("pdfAnnotationsEnabled",
                base::Value(base::FeatureList::IsEnabled(
                    chrome_pdf::features::kPDFAnnotations)));
+  dict->SetKey("pdfTwoUpViewEnabled",
+               base::Value(base::FeatureList::IsEnabled(
+                   chrome_pdf::features::kPDFTwoUpView)));
 
   bool enable_printing = true;
 #if defined(OS_CHROMEOS)

chrome/browser/pdf/pdf_extension_test.cc

@@ -803,6 +803,10 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionTest, Printing) {
 }
 #endif
 
+IN_PROC_BROWSER_TEST_F(PDFExtensionTest, TwoUpViewFeature) {
+  RunTestsInJsModule("two_up_view_feature_test.js", "test.pdf");
+}
+
 // TODO(tsepez): See https://crbug.com/696650.
 IN_PROC_BROWSER_TEST_F(PDFExtensionTest, DISABLED_NoBeep) {
   // Block the exact query from pdf/main.js while still allowing enough

chrome/browser/resources/pdf/BUILD.gn

@@ -95,6 +95,7 @@ js_library("controller") {
 
 js_library("pdf_viewer") {
   deps = [
+    ":constants",
     ":controller",
     ":metrics",
     ":navigator",

chrome/browser/resources/pdf/constants.js

@@ -12,3 +12,12 @@ export const FittingType = {
   FIT_TO_WIDTH: 'fit-to-width',
   FIT_TO_HEIGHT: 'fit-to-height',
 };
+
+/**
+ * Enumeration of two up view actions.
+ * @enum {string}
+ */
+export const TwoUpViewAction = {
+  TWO_UP_VIEW_ENABLE: 'two-up-view-enable',
+  TWO_UP_VIEW_DISABLE: 'two-up-view-disable',
+};

chrome/browser/resources/pdf/controller.js

@@ -91,6 +91,9 @@ export class ContentController {
   /** @abstract */
   rotateCounterclockwise() {}
 
+  /** @abstract */
+  setTwoUpView(enableTwoUpView) {}
+
   /** Triggers printing of the current document. */
   print() {}
 
@@ -174,6 +177,11 @@ export class InkController extends ContentController {
     // TODO(dstockwell): implement rotation
   }
 
+  /** @override */
+  setTwoUpView(enableTwoUpView) {
+    // TODO(dstockwell): Implement two up view.
+  }
+
   /** @override */
   viewportChanged() {
     this.inkHost_.viewportChanged();
@@ -333,6 +341,14 @@ export class PluginController extends ContentController {
     this.postMessage_({type: 'rotateCounterclockwise'});
   }
 
+  /** @override */
+  setTwoUpView(enableTwoUpView) {
+    this.postMessage_({
+      type: 'setTwoUpView',
+      enableTwoUpView: enableTwoUpView,
+    });
+  }
+
   /** @override */
   print() {
     this.postMessage_({type: 'print'});

chrome/browser/resources/pdf/elements/viewer-zoom-button.html

@@ -77,6 +77,7 @@
     </style>
     <div id="wrapper">
       <cr-icon-button iron-icon="[[visibleIcon_]]" on-click="fireClick_"
-          aria-label$="[[visibleTooltip_]]" title="[[visibleTooltip_]]">
+          aria-label$="[[visibleTooltip_]]" title="[[visibleTooltip_]]"
+          disabled="[[disabled]]">
       </cr-icon-button>
     </div>

chrome/browser/resources/pdf/elements/viewer-zoom-button.js

@@ -25,6 +25,11 @@ Polymer({
       observer: 'delayChanged_',
     },
 
+    disabled: {
+      type: Boolean,
+      value: false,
+    },
+
     /**
      * Icons to be displayed on the FAB. Multiple icons should be separated with
      * spaces, and will be cycled through every time the FAB is clicked.

chrome/browser/resources/pdf/elements/viewer-zoom-toolbar.html

-    <style>
+    <style include="cr-hidden-style">
       :host {
         --button-position-offset: 48px;
         bottom: 0;
@@ -44,21 +44,33 @@
         display: block;
       }
 
+      /*
+       * A larger gap between the fit button and the two-up view button
+       * and above the bottom two zoom buttons.
+       */
+      #two-up-view-button,
+      #zoom-in-button {
+        margin-top: 24px;
+      }
+
       /* A small gap between the zoom in/zoom out buttons. */
       #zoom-out-button {
         margin-top: 10px;
       }
-
-      /* A larger gap between the fit button and bottom two buttons. */
-      #zoom-in-button {
-        margin-top: 24px;
-      }
     </style>
     <div id="zoom-buttons">
-      <viewer-zoom-button id="fit-button" on-fabclick="fitToggle" delay="100"
+      <viewer-zoom-button id="fit-button" on-fabclick="fitToggle"
+          delay="[[fitButtonDelay_]]"
           keyboard-navigation-active="[[keyboardNavigationActive_]]"
           icons="pdf:fullscreen-exit cr:fullscreen">
       </viewer-zoom-button>
+      <!-- TODO(crbug.com/51472): Change icons for two-up-view-button -->
+      <!-- once they are finalized. -->
+      <viewer-zoom-button id="two-up-view-button" delay="100"
+          disabled="[[annotationMode]]" hidden$="[[!twoUpViewEnabled]]"
+          on-fabclick="twoUpViewToggle_"
+          keyboard-navigation-active="[[keyboardNavigationActive_]]"
+          icons="pdf:create pdf:eraser"></viewer-zoom-button>
       <viewer-zoom-button id="zoom-in-button" icons="pdf:add"
           keyboard-navigation-active="[[keyboardNavigationActive_]]"
           on-fabclick="zoomIn" delay="50"></viewer-zoom-button>

chrome/browser/resources/pdf/elements/viewer-zoom-toolbar.js

@@ -8,7 +8,7 @@ import './viewer-zoom-button.js';
 
 import {assert} from 'chrome://resources/js/assert.m.js';
 import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
-import {FittingType} from '../constants.js';
+import {FittingType, TwoUpViewAction} from '../constants.js';
 
 /**
  * @typedef {{
@@ -21,14 +21,31 @@ export let FitToChangedEvent;
 const FIT_TO_PAGE_BUTTON_STATE = 0;
 const FIT_TO_WIDTH_BUTTON_STATE = 1;
 
+const TWO_UP_VIEW_DISABLED_STATE = 0;
+const TWO_UP_VIEW_ENABLED_STATE = 1;
+
 Polymer({
   is: 'viewer-zoom-toolbar',
 
   _template: html`{__html_template__}`,
 
   properties: {
+    /** Whether the viewer is currently in annotation mode. */
+    annotationMode: {
+      type: Boolean,
+      value: false,
+    },
+
     isPrintPreview: Boolean,
 
+    twoUpViewEnabled: Boolean,
+
+    /** @private */
+    fitButtonDelay_: {
+      type: Number,
+      computed: 'computeFitButtonDelay_(twoUpViewEnabled)',
+    },
+
     /** @private */
     keyboardNavigationActive_: {
       type: Boolean,
@@ -36,6 +53,15 @@ Polymer({
     },
   },
 
+  /**
+   * @param {boolean} twoUpViewEnabled Whether two-up view button is enabled.
+   * @return {number} Delay for :qthe fit button.
+   * @private
+   */
+  computeFitButtonDelay_(twoUpViewEnabled) {
+    return twoUpViewEnabled ? 150 : 100;
+  },
+
   listeners: {
     'focus': 'onFocus_',
     'keyup': 'onKeyUp_',
@@ -81,12 +107,16 @@ Polymer({
    * Change button tooltips to match any changes to localized strings.
    * @param {!{tooltipFitToPage: string,
    *           tooltipFitToWidth: string,
+   *           tooltipTwoUpViewEnable: string,
+   *           tooltipTwoUpViewDisable: string,
    *           tooltipZoomIn: string,
    *           tooltipZoomOut: string}} strings
    */
   setStrings(strings) {
     this.$['fit-button'].tooltips =
         [strings.tooltipFitToPage, strings.tooltipFitToWidth];
+    this.$['two-up-view-button'].tooltips =
+        [strings.tooltipTwoUpViewEnable, strings.tooltipTwoUpViewDisable];
     this.$['zoom-in-button'].tooltips = [strings.tooltipZoomIn];
     this.$['zoom-out-button'].tooltips = [strings.tooltipZoomOut];
   },
@@ -139,6 +169,20 @@ Polymer({
         {fittingType: fittingType, userInitiated: userInitiated});
   },
 
+  /**
+   * Handle clicks of the two-up-view button.
+   * @private
+   */
+  twoUpViewToggle_: function() {
+    assert(this.twoUpViewEnabled);
+    const twoUpViewAction = this.$['two-up-view-button'].activeIndex ===
+            TWO_UP_VIEW_DISABLED_STATE ?
+        TwoUpViewAction.TWO_UP_VIEW_ENABLE :
+        TwoUpViewAction.TWO_UP_VIEW_DISABLE;
+
+    this.fire('two-up-view-changed', twoUpViewAction);
+  },
+
   /**
    * Handle clicks of the zoom-in-button.
    */
@@ -157,6 +201,7 @@ Polymer({
     if (!this.visible_) {
       this.visible_ = true;
       this.$['fit-button'].show();
+      this.$['two-up-view-button'].show();
       this.$['zoom-in-button'].show();
       this.$['zoom-out-button'].show();
     }
@@ -166,6 +211,7 @@ Polymer({
     if (this.visible_) {
       this.visible_ = false;
       this.$['fit-button'].hide();
+      this.$['two-up-view-button'].hide();
       this.$['zoom-in-button'].hide();
       this.$['zoom-out-button'].hide();
     }

chrome/browser/resources/pdf/pdf_viewer.js

@@ -9,7 +9,7 @@ import {PromiseResolver} from 'chrome://resources/js/promise_resolver.m.js';
 import {$, hasKeyModifiers, isRTL} from 'chrome://resources/js/util.m.js';
 
 import {BrowserApi} from './browser_api.js';
-import {FittingType} from './constants.js';
+import {FittingType, TwoUpViewAction} from './constants.js';
 import {ContentController, InkController, MessageData, PluginController, PrintPreviewParams} from './controller.js';
 import {Bookmark} from './elements/viewer-bookmark.js';
 import {FitToChangedEvent} from './elements/viewer-zoom-toolbar.js';
@@ -333,6 +333,10 @@ export class PDFViewer {
         'fit-to-changed',
         e => this.fitToChanged_(
             /** @type {!CustomEvent<FitToChangedEvent>} */ (e)));
+    this.zoomToolbar_.addEventListener(
+        'two-up-view-changed',
+        e => this.twoUpViewChanged_(
+            /** @type {!CustomEvent<!TwoUpViewAction>} */ (e)));
     this.zoomToolbar_.addEventListener(
         'zoom-in', () => this.viewport_.zoomIn());
     this.zoomToolbar_.addEventListener(
@@ -641,6 +645,7 @@ export class PDFViewer {
    */
   async annotationModeToggled_(e) {
     const annotationMode = e.detail.value;
+    this.zoomToolbar_.annotationMode = annotationMode;
     if (annotationMode) {
       // Enter annotation mode.
       assert(this.currentController_ === this.pluginController_);
@@ -657,6 +662,7 @@ export class PDFViewer {
         } catch (e) {
           // The user aborted entering annotation mode. Revert to the plugin.
           this.toolbar_.annotationMode = false;
+          this.zoomToolbar_.annotationMode = false;
           this.updateProgress_(100);
           return;
         }
@@ -702,6 +708,7 @@ export class PDFViewer {
       return;
     }
     this.toolbar_.toggleAnnotation();
+    this.zoomToolbar_.annotationMode = false;
     await this.loaded;
   }
 
@@ -726,6 +733,22 @@ export class PDFViewer {
     }
   }
 
+  /**
+   * Changes two up view mode for the controller. Controller will trigger
+   * layout update later, which will update the viewport accordingly.
+   * @param {!CustomEvent<!TwoUpViewAction>} e
+   * @private
+   */
+  twoUpViewChanged_(e) {
+    this.currentController_.setTwoUpView(
+        e.detail === TwoUpViewAction.TWO_UP_VIEW_ENABLE);
+    this.toolbarManager_.forceHideTopToolbar();
+    this.toolbar_.annotationAvailable =
+        (e.detail !== TwoUpViewAction.TWO_UP_VIEW_ENABLE);
+
+    // TODO(crbug.com/51472): Record to metrics.
+  }
+
   /**
    * Sends a 'documentLoaded' message to the PDFScriptingAPI if the document has
    * finished loading.
@@ -918,6 +941,8 @@ export class PDFViewer {
         loadTimeData.getBoolean('pdfAnnotationsEnabled');
     $('toolbar').printingEnabled = loadTimeData.getBoolean('printingEnabled');
     $('zoom-toolbar').setStrings(strings);
+    $('zoom-toolbar').twoUpViewEnabled =
+        loadTimeData.getBoolean('pdfTwoUpViewEnabled') && !this.isPrintPreview_;
     // Display the zoom toolbar after the UI text direction is set, to ensure it
     // appears on the correct side of the PDF viewer.
     $('zoom-toolbar').hidden = false;

chrome/browser/resources/pdf/viewport.js

@@ -19,7 +19,12 @@ import {InactiveZoomManager, ZoomManager} from './zoom_manager.js';
  */
 let DocumentDimensions;
 
-/** @typedef {{defaultPageOrientation: number}} */
+/**
+ * @typedef {{
+ *   defaultPageOrientation: number,
+ *   twoUpViewEnabled: boolean,
+ * }}
+ */
 export let LayoutOptions;
 
 /** @typedef {{x: number, y: number}} */
@@ -136,12 +141,6 @@ export class Viewport {
     /** @private {!FittingType} */
     this.fittingType_ = FittingType.NONE;
 
-    /**
-     * |twoUpView_| should be in sync with |two_up_view_| in PDFiumEngine.
-     * @private {boolean}
-     */
-    this.twoUpView_ = false;
-
     /** @private {number} */
     this.prevScale_ = 1;
 
@@ -220,9 +219,13 @@ export class Viewport {
     return this.rotations_;
   }
 
-  /** @param {boolean} twoUpView The new two up view state to set. */
-  setTwoUpView(twoUpView) {
-    this.twoUpView_ = twoUpView;
+  /** @return {boolean} Whether viewport is in two-up view mode. */
+  twoUpViewEnabled() {
+    const options = this.getLayoutOptions();
+    if (options === undefined) {
+      return false;
+    }
+    return options.twoUpViewEnabled;
   }
 
   /**
@@ -677,7 +680,7 @@ export class Viewport {
   getLastPageInViewport_(viewportRect) {
     const pageAtY = this.getPageAtY_(viewportRect.y + viewportRect.height);
 
-    if (!this.twoUpView_ || pageAtY % 2 === 1 ||
+    if (!this.twoUpViewEnabled() || pageAtY % 2 === 1 ||
         pageAtY + 1 >= this.pageDimensions_.length) {
       return pageAtY;
     }
@@ -1070,7 +1073,8 @@ export class Viewport {
    */
   goToNextPage() {
     const currentPage = this.getMostVisiblePage();
-    const nextPageOffset = (this.twoUpView_ && currentPage % 2 === 0) ? 2 : 1;
+    const nextPageOffset =
+        (this.twoUpViewEnabled() && currentPage % 2 === 0) ? 2 : 1;
     this.goToPage(currentPage + nextPageOffset);
   }
 
@@ -1082,7 +1086,7 @@ export class Viewport {
     const currentPage = this.getMostVisiblePage();
     let previousPageOffset = -1;
 
-    if (this.twoUpView_) {
+    if (this.twoUpViewEnabled()) {
       previousPageOffset = (currentPage % 2 === 0) ? -2 : -3;
     }
 

chrome/test/data/pdf/two_up_view_feature_test.js

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+chrome.test.runTests([
+  function testTwoUpViewFeatureDisabled() {
+    const toolbar = document.body.querySelector('#zoom-toolbar');
+    toolbar.twoUpViewEnabled = false;
+
+    const twoUpButton = toolbar.shadowRoot.querySelector('#two-up-view-button');
+    chrome.test.assertTrue(!!twoUpButton);
+    chrome.test.assertTrue(twoUpButton.hidden);
+
+    const fitButton = toolbar.shadowRoot.querySelector('#fit-button');
+    chrome.test.assertTrue(!!fitButton);
+    chrome.test.assertEq(100, fitButton.delay);
+
+    chrome.test.succeed();
+  },
+
+
+  function testTwoUpViewFeatureEnabled() {
+    const toolbar = document.body.querySelector('#zoom-toolbar');
+    toolbar.twoUpViewEnabled = true;
+
+    const twoUpButton = toolbar.shadowRoot.querySelector('#two-up-view-button');
+    chrome.test.assertTrue(!!twoUpButton);
+    chrome.test.assertFalse(twoUpButton.hidden);
+
+    const fitButton = toolbar.shadowRoot.querySelector('#fit-button');
+    chrome.test.assertTrue(!!fitButton);
+    chrome.test.assertEq(150, fitButton.delay);
+
+    chrome.test.succeed();
+  },
+]);

chrome/test/data/pdf/viewport_test.js

@@ -240,9 +240,9 @@ const tests = [
   function testGetMostVisiblePageForTwoUpView() {
     const mockWindow = new MockWindow(400, 500);
     const viewport = getZoomableViewport(mockWindow, new MockSizer(), 0, 1, 0);
-    viewport.setTwoUpView(true);
 
-    const documentDimensions = new MockDocumentDimensions(100, 100);
+    const documentDimensions = new MockDocumentDimensions(
+        100, 100, {defaultPageOrientation: 0, twoUpViewEnabled: true});
     documentDimensions.addPageForTwoUpView(100, 0, 300, 400);
     documentDimensions.addPageForTwoUpView(400, 0, 400, 300);
     documentDimensions.addPageForTwoUpView(0, 400, 400, 250);
@@ -601,9 +601,9 @@ const tests = [
     const mockCallback = new MockViewportChangedCallback();
     const viewport = getZoomableViewport(mockWindow, mockSizer, 0, 1, 0);
     viewport.setViewportChangedCallback(mockCallback.callback);
-    viewport.setTwoUpView(true);
 
-    const documentDimensions = new MockDocumentDimensions(800, 750);
+    const documentDimensions = new MockDocumentDimensions(
+        800, 750, {defaultPageOrientation: 0, twoUpViewEnabled: true});
     documentDimensions.addPageForTwoUpView(200, 0, 200, 150);
     documentDimensions.addPageForTwoUpView(400, 0, 400, 200);
     documentDimensions.addPageForTwoUpView(100, 200, 300, 250);
@@ -706,9 +706,9 @@ const tests = [
     const mockCallback = new MockViewportChangedCallback();
     const viewport = getZoomableViewport(mockWindow, mockSizer, 0, 1, 0);
     viewport.setViewportChangedCallback(mockCallback.callback);
-    viewport.setTwoUpView(true);
 
-    const documentDimensions = new MockDocumentDimensions(800, 750);
+    const documentDimensions = new MockDocumentDimensions(
+        800, 750, {defaultPageOrientation: 0, twoUpViewEnabled: true});
     documentDimensions.addPageForTwoUpView(200, 0, 200, 150);
     documentDimensions.addPageForTwoUpView(400, 0, 400, 200);
     documentDimensions.addPageForTwoUpView(100, 200, 300, 250);
@@ -1053,10 +1053,11 @@ const tests = [
 
     chrome.test.assertEq(undefined, viewport.getLayoutOptions());
 
-    viewport.setDocumentDimensions(
-        new MockDocumentDimensions(50, 50, {defaultPageOrientation: 1}));
+    viewport.setDocumentDimensions(new MockDocumentDimensions(
+        50, 50, {defaultPageOrientation: 1, twoUpViewEnabled: true}));
     chrome.test.assertEq(
-        {defaultPageOrientation: 1}, viewport.getLayoutOptions());
+        {defaultPageOrientation: 1, twoUpViewEnabled: true},
+        viewport.getLayoutOptions());
 
     viewport.setDocumentDimensions(new MockDocumentDimensions(50, 50));
     chrome.test.assertEq(undefined, viewport.getLayoutOptions());

components/pdf_strings.grdp

@@ -46,6 +46,15 @@
     <message name="IDS_PDF_TOOLTIP_FIT_WIDTH" desc="Button tooltip for the button which zooms a PDF so that the width of a single page fills the window horizontally">
       Fit to width
     </message>
+    <!-- TODO(nigi): Remove the translateable="false" attributes for messages
+    IDS_PDF_TOOLTIP_TWO_UP_VIEW_ENABLE and IDS_PDF_TOOLTIP_TWO_UP_VIEW_DISABLE
+    once their tooltips are finalized. -->
+    <message name="IDS_PDF_TOOLTIP_TWO_UP_VIEW_ENABLE" desc="Button tooltip for the button which enables displaying two pages of a PDF on the screen horizontally" translateable="false">
+      Enable two-up view
+    </message>
+    <message name="IDS_PDF_TOOLTIP_TWO_UP_VIEW_DISABLE" desc="Button tooltip for the button which stops displaying two pages of a PDF on the screen horizontally" translateable="false">
+      Disable two-up view
+    </message>
     <message name="IDS_PDF_TOOLTIP_ZOOM_IN" desc="Button tooltip for the button which zooms in a PDF, so that the document appears larger">
       Zoom in
     </message>

pdf/out_of_process_instance.cc

@@ -1740,6 +1740,7 @@ void OutOfProcessInstance::RotateCounterclockwise() {
 }
 
 void OutOfProcessInstance::SetTwoUpView(bool enable_two_up_view) {
+  DCHECK(base::FeatureList::IsEnabled(features::kPDFTwoUpView));
   engine_->SetTwoUpView(enable_two_up_view);
 }