PDF Viewer: Disable two up button in annotation mode

Two up mode is implemented in the plugin but not the ink controller,
and triggering it causes a crash if the plugin is not on the page.

Also fixing the following case:
1. Rotate PDF. Annotation is disabled.
2. Set 2 page view.
3. Rotate PDF back to upright.

Currently, annotation is re-enabled, despite still being in 2 page view.
After this CL, annotation remains disabled, until two page view is
turned off.

Bug: 1048812
Change-Id: I26c0cb43645c82905e1fe5b53398332573d8a774
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2411284Reviewed-by: dpapad <dpapad@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807496}

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

@@ -167,7 +167,10 @@
 <div id="toolbar">
   <div id="start">
     <cr-icon-button id="sidenavToggle" iron-icon="cr20:menu"
-        disabled="[[annotationMode]]" on-click="onSidenavToggleClick_">
+<if expr="chromeos">
+        disabled="[[annotationMode]]"
+</if>
+        on-click="onSidenavToggleClick_">
     </cr-icon-button>
     <span id="title">[[docTitle]]</span>
   </div>
@@ -193,8 +196,11 @@
         on-click="onFitToButtonClick_">
     </cr-icon-button>
     <cr-icon-button iron-icon="pdf:rotate-left"
-        disabled="[[annotationMode]]" aria-label$="$i18n{tooltipRotateCCW}"
-        title="$i18n{tooltipRotateCCW}" on-click="onRotateClick_">
+<if expr="chromeos">
+        disabled="[[annotationMode]]"
+</if>
+        aria-label$="$i18n{tooltipRotateCCW}" title="$i18n{tooltipRotateCCW}"
+        on-click="onRotateClick_">
     </cr-icon-button>
   </div>
   <div id="end">
@@ -224,8 +230,10 @@
 
 <cr-action-menu>
   <button id="two-page-view-button"
-      class="dropdown-item" on-click="toggleTwoPageViewClick_"
-      role="checkbox"
+      class="dropdown-item" on-click="toggleTwoPageViewClick_" role="checkbox"
+<if expr="chromeos">
+      disabled="[[annotationMode]]"
+</if>
       aria-checked="[[getTwoPageViewAriaChecked_(twoUpViewEnabled_)]]">
     <span class="check-container">
       <iron-icon icon="pdf:check" hidden="[[!twoUpViewEnabled_]]"></iron-icon>

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

@@ -37,12 +37,12 @@ export class ViewerPdfToolbarNewElement extends PolymerElement {
     return {
       // <if expr="chromeos">
       annotationAvailable: Boolean,
-      // </if>
       annotationMode: {
         type: Boolean,
         value: false,
         reflectToAttribute: true,
       },
+      // </if>
       docTitle: String,
       docLength: Number,
       hasEdits: Boolean,

chrome/browser/resources/pdf/pdf_viewer.js

@@ -105,7 +105,7 @@ export class PDFViewerElement extends PDFViewerBaseElement {
       annotationAvailable_: {
         type: Boolean,
         computed: 'computeAnnotationAvailable_(' +
-            'hadPassword_, rotated_, canSerializeDocument_)',
+            'hadPassword_, rotated_, canSerializeDocument_, twoUpViewEnabled_)',
       },
 
       annotationMode_: {
@@ -140,6 +140,8 @@ export class PDFViewerElement extends PDFViewerBaseElement {
 
       title_: String,
 
+      twoUpViewEnabled_: Boolean,
+
       isFormFieldFocused_: Boolean,
 
       pdfViewerUpdateEnabled_: Boolean,
@@ -194,6 +196,9 @@ export class PDFViewerElement extends PDFViewerBaseElement {
     /** @private {string} */
     this.title_ = '';
 
+    /** @private {boolean} */
+    this.twoUpViewEnabled_ = false;
+
     /** @private {boolean} */
     this.isFormFieldFocused_ = false;
 
@@ -620,13 +625,12 @@ export class PDFViewerElement extends PDFViewerBaseElement {
    * @private
    */
   onTwoUpViewChanged_(e) {
-    const twoUpViewEnabled = e.detail;
-    this.currentController.setTwoUpView(twoUpViewEnabled);
+    this.twoUpViewEnabled_ = e.detail;
+    this.currentController.setTwoUpView(this.twoUpViewEnabled_);
     if (!this.pdfViewerUpdateEnabled_) {
       this.toolbarManager_.forceHideTopToolbar();
     }
-    this.getToolbar_().annotationAvailable = !twoUpViewEnabled;
-    PDFMetrics.recordTwoUpViewEnabled(twoUpViewEnabled);
+    PDFMetrics.recordTwoUpViewEnabled(this.twoUpViewEnabled_);
   }
 
   /**
@@ -1042,7 +1046,8 @@ export class PDFViewerElement extends PDFViewerBaseElement {
    * @private
    */
   computeAnnotationAvailable_() {
-    return this.canSerializeDocument_ && !this.rotated_ && !this.hadPassword_;
+    return this.canSerializeDocument_ && !this.rotated_ && !this.hadPassword_ &&
+        !this.twoUpViewEnabled_;
   }
 
   /** @override */

chrome/test/data/pdf/annotations_feature_enabled_test.js

@@ -386,9 +386,12 @@ chrome.test.runTests([
   },
   function testHidingAnnotationsExitsAnnotationsMode() {
     testAsync(async () => {
+      document.body.innerHTML = '';
       const toolbar = document.createElement('viewer-pdf-toolbar-new');
       document.body.appendChild(toolbar);
       toolbar.toggleAnnotation();
+      // This is normally done by the parent in response to the event fired by
+      // toggleAnnotation().
       toolbar.annotationMode = true;
 
       await toolbar.addEventListener('display-annotations-changed', async e => {
@@ -400,6 +403,7 @@ chrome.test.runTests([
     });
   },
   function testEnteringAnnotationsModeShowsAnnotations() {
+    document.body.innerHTML = '';
     const toolbar = document.createElement('viewer-pdf-toolbar-new');
     document.body.appendChild(toolbar);
     chrome.test.assertFalse(toolbar.annotationMode);
@@ -412,5 +416,19 @@ chrome.test.runTests([
       chrome.test.succeed();
     });
     toolbar.toggleAnnotation();
+  },
+  function testEnteringAnnotationsModeDisablesTwoUp() {
+    document.body.innerHTML = '';
+    const toolbar = document.createElement('viewer-pdf-toolbar-new');
+    document.body.appendChild(toolbar);
+    chrome.test.assertFalse(toolbar.annotationMode);
+
+    toolbar.toggleAnnotation();
+    // This is normally done by the parent in response to the event fired by
+    // toggleAnnotation().
+    toolbar.annotationMode = true;
+    chrome.test.assertTrue(
+        toolbar.shadowRoot.querySelector('#two-page-view-button').disabled);
+    chrome.test.succeed();
   }
 ]);