PDF Viewer: Cleanup in preparation for adding edit option

- Set feature enabled values in the toolbar directly, when strings are
set, rather than doing this from the viewer.
- Simplify/clean up styles in the dropdown to match them to the menu
being added
- Cleanups in viewer-pdf-toolbar.html

Bug: 1078543
Change-Id: I4146ff642d2ea482f343149a52c00253844ed3b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2223153
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773472}

chrome/browser/resources/pdf/elements/BUILD.gn

@@ -76,6 +76,8 @@ js_library("viewer-pdf-toolbar") {
     ":viewer-page-selector",
     ":viewer-toolbar-dropdown",
     "..:annotation_tool",
+    "//ui/webui/resources/js:assert.m",
+    "//ui/webui/resources/js:load_time_data.m",
   ]
   externs_list = [ "$externs_path/pending.js" ]
 }

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

     <style include="cr-hidden-style">
+      :host {
+        --pdf-toolbar-background-color: rgb(50, 54, 57);
+        --pdf-toolbar-text-color: rgb(241, 241, 241);
+      }
+
       :host ::selection {
         background: rgba(255, 255, 255, 0.3);
       }
@@ -36,7 +41,7 @@
       }
 
       cr-icon-button {
-        --cr-icon-button-fill-color: rgb(241, 241, 241);
+        --cr-icon-button-fill-color: var(--pdf-toolbar-text-color);
         margin: 6px;
       }
 
@@ -54,7 +59,7 @@
       }
 
       #toolbar {
-        background-color: rgb(50, 54, 57);
+        background-color: var(--pdf-toolbar-background-color);
         box-shadow: var(--cr-elevation-2);
         position: relative;
       }
@@ -67,7 +72,7 @@
 
       #toolbar,
       #annotations-bar {
-        color: rgb(241, 241, 241);
+        color: var(--pdf-toolbar-text-color);
         display: flex;
         height: 48px;
         padding: 0 16px;
@@ -90,7 +95,7 @@
 
       #pen,
       #highlighter {
-        --dropdown-open-background: rgb(50, 54, 57);
+        --dropdown-open-background: var(--pdf-viewer-background-color);
       }
 
       #eraser {
@@ -171,7 +176,7 @@
         </div>
 
         <div id="buttons" class="invisible">
-          <template is="dom-if" if="[[pdfAnnotationsEnabled]]">
+          <template is="dom-if" if="[[pdfAnnotationsEnabled_]]">
             <cr-icon-button id="annotate" iron-icon="pdf:create"
                 disabled="[[!annotationAvailable]]" on-click="toggleAnnotation"
                 aria-label$="[[strings.tooltipAnnotate]]"
@@ -188,7 +193,7 @@
               title="[[strings.tooltipDownload]]"></cr-icon-button>
 
           <cr-icon-button id="print" iron-icon="cr:print" on-click="print"
-              hidden="[[!printingEnabled]]" title="[[strings.tooltipPrint]]"
+              hidden="[[!printingEnabled_]]" title="[[strings.tooltipPrint]]"
               aria-label$="[[strings.tooltipPrint]]"></cr-icon-button>
 
           <viewer-toolbar-dropdown id="bookmarks"

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

@@ -15,6 +15,8 @@ import './viewer-toolbar-dropdown.js';
 import './viewer-pen-options.js';
 // </if>
 
+import {assert} from 'chrome://resources/js/assert.m.js';
+import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 import {Bookmark} from '../bookmark_type.js';
 
@@ -89,19 +91,28 @@ Polymer({
     /** The number of the page being viewed (1-based). */
     pageNo: Number,
 
-    /** Whether the PDF Annotations feature is enabled. */
-    pdfAnnotationsEnabled: {
+    /**
+     * Whether the PDF Annotations feature is enabled.
+     * @private
+     */
+    pdfAnnotationsEnabled_: {
       type: Boolean,
       value: false,
     },
 
-    /** Whether the Printing feature is enabled. */
-    printingEnabled: {
+    /**
+     * Whether the Printing feature is enabled.
+     * @private
+     */
+    printingEnabled_: {
       type: Boolean,
       value: false,
     },
 
-    strings: Object,
+    strings: {
+      type: Object,
+      observer: 'onStringsSet_',
+    },
   },
 
   /** @type {?Object} */
@@ -278,4 +289,13 @@ Polymer({
   isAnnotationTool_(toolName) {
     return !!this.annotationTool && this.annotationTool.tool === toolName;
   },
+
+  /** @private */
+  onStringsSet_() {
+    assert(this.strings);
+
+    this.pdfAnnotationsEnabled_ =
+        loadTimeData.getBoolean('pdfAnnotationsEnabled');
+    this.printingEnabled_ = loadTimeData.getBoolean('printingEnabled');
+  },
 });

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

@@ -27,23 +27,17 @@
       #dropdown {
         background-color: var(--cr-menu-background-color);
         border-radius: 4px;
+        box-shadow: var(--cr-menu-shadow);
         color: var(--cr-primary-text-color);
         overflow-y: hidden;
         padding-bottom: 2px;
         width: var(--dropdown-width);
       }
 
-      @media (prefers-color-scheme: light) {
-        #dropdown {
-          box-shadow: var(--cr-elevation-2);
-        }
-      }
-
       @media (prefers-color-scheme: dark) {
         #dropdown {
           background-image: linear-gradient(var(--cr-menu-background-sheen),
                                             var(--cr-menu-background-sheen));
-          box-shadow: var(--cr-menu-shadow);
         }
       }
 

chrome/browser/resources/pdf/pdf_viewer.js

@@ -939,9 +939,6 @@ export class PDFViewer {
       this.sendBackgroundColorForPrintPreview_();
     } else {
       $('toolbar').strings = strings;
-      $('toolbar').pdfAnnotationsEnabled =
-          loadTimeData.getBoolean('pdfAnnotationsEnabled');
-      $('toolbar').printingEnabled = loadTimeData.getBoolean('printingEnabled');
     }
     $('zoom-toolbar').setStrings(strings);
     $('zoom-toolbar').twoUpViewEnabled =

chrome/test/data/pdf/annotations_feature_disabled_test.js

@@ -2,10 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
+
 chrome.test.runTests([
   function testAnnotationsDisabled() {
     const toolbar = document.body.querySelector('#toolbar');
-    if (toolbar.pdfAnnotationsEnabled) {
+    if (loadTimeData.getBoolean('pdfAnnotationsEnabled')) {
       chrome.test.assertTrue(!!toolbar.shadowRoot.querySelector('#annotate'));
     } else {
       chrome.test.assertFalse(!!toolbar.shadowRoot.querySelector('#annotate'));

chrome/test/data/pdf/annotations_feature_enabled_test.js

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {$} from 'chrome://resources/js/util.m.js';
 
 window.onerror = e => chrome.test.fail(e.stack);
@@ -41,7 +42,7 @@ async function testAsync(f) {
 chrome.test.runTests([
   function testAnnotationsEnabled() {
     const toolbar = document.body.querySelector('#toolbar');
-    chrome.test.assertTrue(toolbar.pdfAnnotationsEnabled);
+    chrome.test.assertTrue(loadTimeData.getBoolean('pdfAnnotationsEnabled'));
     chrome.test.assertTrue(
         toolbar.shadowRoot.querySelector('#annotate') != null);
     chrome.test.succeed();

chrome/test/data/pdf/printing_icon_test.js

@@ -2,18 +2,24 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
+
 chrome.test.runTests([
   function testPrintingEnabled() {
+    loadTimeData.overrideValues({printingEnabled: true});
     const toolbar = document.body.querySelector('#toolbar');
-    toolbar.printingEnabled = true;
+    // Trigger strings observer.
+    toolbar.strings = Object.assign({}, toolbar.strings);
     const printIcon = toolbar.shadowRoot.querySelector('#print');
     chrome.test.assertTrue(!!printIcon);
     chrome.test.assertFalse(printIcon.hidden);
     chrome.test.succeed();
   },
   function testPrintingDisabled() {
+    loadTimeData.overrideValues({printingEnabled: false});
     const toolbar = document.body.querySelector('#toolbar');
-    toolbar.printingEnabled = false;
+    // Trigger strings observer.
+    toolbar.strings = Object.assign({}, toolbar.strings);
     const printIcon = toolbar.shadowRoot.querySelector('#print');
     chrome.test.assertTrue(!!printIcon);
     chrome.test.assertTrue(printIcon.hidden);