PDF Viewer: Move InkController to its own CrOS-only file.

Previously the InkController class was defined in controller.js,
and moreover the code was not excluded for non-CrOS builds. This CL
fixes the issue by addressing a TODO in the code.

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

chrome/browser/resources/component_extension_resources.grd

@@ -124,6 +124,7 @@
         <include name="IDR_PDF_VIEWER_BOOKMARK_JS" file="${root_gen_dir}/chrome/browser/resources/pdf/elements/viewer-bookmark.js" use_base_dir="false" type="BINDATA" />
         <include name="IDR_PDF_VIEWER_ERROR_SCREEN_JS" file="${root_gen_dir}/chrome/browser/resources/pdf/elements/viewer-error-screen.js" use_base_dir="false" type="BINDATA" />
         <if expr="chromeos">
+          <include name="IDR_PDF_VIEWER_INK_CONTROLLER_JS" file="pdf/ink_controller.js" type="BINDATA" />
           <include name="IDR_PDF_VIEWER_INK_INDEX_HTML" file="pdf/ink/index.html" type="BINDATA" />
           <include name="IDR_PDF_VIEWER_INK_INK_API_JS" file="pdf/ink/ink_api.js" type="BINDATA" />
           <include name="IDR_PDF_VIEWER_INK_HOST_JS" file="${root_gen_dir}/chrome/browser/resources/pdf/elements/viewer-ink-host.js" use_base_dir="false" type="BINDATA" />

chrome/browser/resources/pdf/BUILD.gn

@@ -101,16 +101,22 @@ js_library("toolbar_manager") {
   ]
 }
 
-js_library("controller") {
+js_library("ink_controller") {
   deps = [
     ":annotation_tool",
-    ":constants",
+    ":controller",
+    ":viewport",
+    "//ui/webui/resources/js/cr:event_target.m",
+  ]
+}
+
+js_library("controller") {
+  deps = [
     ":viewport",
     "elements:viewer-pdf-toolbar",
     "//ui/webui/resources/js:assert.m",
     "//ui/webui/resources/js:load_time_data.m",
     "//ui/webui/resources/js:promise_resolver.m",
-    "//ui/webui/resources/js:util.m",
     "//ui/webui/resources/js/cr:event_target.m",
   ]
 }
@@ -141,6 +147,7 @@ js_library("pdf_viewer") {
     ":browser_api",
     ":constants",
     ":controller",
+    ":ink_controller",
     ":metrics",
     ":navigator",
     ":pdf_scripting_api",
@@ -204,6 +211,7 @@ js_type_check("pdf_resources") {
     ":constants",
     ":controller",
     ":gesture_detector",
+    ":ink_controller",
     ":main",
     ":metrics",
     ":navigator",

chrome/browser/resources/pdf/controller.js

@@ -6,7 +6,6 @@ import {assert} from 'chrome://resources/js/assert.m.js';
 import {NativeEventTarget as EventTarget} from 'chrome://resources/js/cr/event_target.m.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {PromiseResolver} from 'chrome://resources/js/promise_resolver.m.js';
-import {$} from 'chrome://resources/js/util.m.js';
 
 import {SaveRequestType} from './constants.js';
 import {PartialPoint, Point, Viewport} from './viewport.js';
@@ -46,25 +45,6 @@ let EmailMessageData;
  */
 export let PrintPreviewParams;
 
-// Note: Redefining this type here, to work around the fact that ink externs
-// are only available on Chrome OS, so the targets that contain them cannot be
-// built on other platforms.
-// TODO (rbpotter): Break InkController into its own file that is only included
-// on Chrome OS.
-
-/**
- * @typedef {{
- *   setAnnotationTool: function(AnnotationTool):void,
- *   viewportChanged: function():void,
- *   saveDocument: function():!Promise,
- *   undo: function():void,
- *   redo: function():void,
- *   load: function(string, !ArrayBuffer):!Promise,
- *   viewport: !Viewport,
- * }}
- */
-let ViewerInkHostElement;
-
 /**
  * Creates a cryptographically secure pseudorandom 128-bit token.
  * @return {string} The generated token as a hex string.
@@ -130,109 +110,6 @@ export class ContentController {
   unload() {}
 }
 
-// Controller for annotation mode, on Chrome OS only. Fires the following events
-// from its event target:
-// has-unsaved-changes: Fired to indicate there are ink annotations that have
-//     not been saved.
-// set-annotation-undo-state: Contains information about whether undo or redo
-//     options are available.
-export class InkController extends ContentController {
-  /**
-   * @param {!Viewport} viewport
-   * @param {!HTMLDivElement} contentElement
-   */
-  constructor(viewport, contentElement) {
-    super();
-
-    /** @private {!Viewport} */
-    this.viewport_ = viewport;
-
-    /** @private {!HTMLDivElement} */
-    this.contentElement_ = contentElement;
-
-    /** @private {?ViewerInkHostElement} */
-    this.inkHost_ = null;
-
-    /** @private {!EventTarget} */
-    this.eventTarget_ = new EventTarget();
-
-    /** @type {?AnnotationTool} */
-    this.tool_ = null;
-  }
-
-  /** @return {!EventTarget} */
-  getEventTarget() {
-    return this.eventTarget_;
-  }
-
-  /** @param {AnnotationTool} tool */
-  setAnnotationTool(tool) {
-    this.tool_ = tool;
-    if (this.inkHost_) {
-      this.inkHost_.setAnnotationTool(tool);
-    }
-  }
-
-  /** @override */
-  rotateClockwise() {
-    // TODO(dstockwell): implement rotation
-  }
-
-  /** @override */
-  rotateCounterclockwise() {
-    // TODO(dstockwell): implement rotation
-  }
-
-  /** @override */
-  setTwoUpView(enableTwoUpView) {
-    // TODO(dstockwell): Implement two up view.
-  }
-
-  /** @override */
-  viewportChanged() {
-    this.inkHost_.viewportChanged();
-  }
-
-  /** @override */
-  save(requestType) {
-    return this.inkHost_.saveDocument();
-  }
-
-  /** @override */
-  undo() {
-    this.inkHost_.undo();
-  }
-
-  /** @override */
-  redo() {
-    this.inkHost_.redo();
-  }
-
-  /** @override */
-  load(filename, data) {
-    if (!this.inkHost_) {
-      const inkHost = document.createElement('viewer-ink-host');
-      this.contentElement_.appendChild(inkHost);
-      this.inkHost_ = /** @type {!ViewerInkHostElement} */ (inkHost);
-      this.inkHost_.viewport = this.viewport_;
-      inkHost.addEventListener('stroke-added', e => {
-        this.eventTarget_.dispatchEvent(new CustomEvent('has-unsaved-changes'));
-      });
-      inkHost.addEventListener('undo-state-changed', e => {
-        this.eventTarget_.dispatchEvent(
-            new CustomEvent('set-annotation-undo-state', {detail: e.detail}));
-      });
-    }
-    return this.inkHost_.load(filename, data);
-  }
-
-  /** @override */
-  unload() {
-    this.inkHost_.remove();
-    this.inkHost_ = null;
-  }
-}
-
 // PDF plugin controller, responsible for communicating with the embedded plugin
 // element. Dispatches a 'plugin-message' event containing the message from the
 // plugin, if a message type not handled by this controller is received.

chrome/browser/resources/pdf/ink_controller.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.
+
+import {NativeEventTarget as EventTarget} from 'chrome://resources/js/cr/event_target.m.js';
+
+import {ContentController} from './controller.js';
+import {Viewport} from './viewport.js';
+
+// Note: Redefining this type here, to work around the fact that ink externs
+// are only available on Chrome OS, so the targets that contain them cannot be
+// built on other platforms.
+
+/**
+ * @typedef {{
+ *   setAnnotationTool: function(AnnotationTool):void,
+ *   viewportChanged: function():void,
+ *   saveDocument: function():!Promise,
+ *   undo: function():void,
+ *   redo: function():void,
+ *   load: function(string, !ArrayBuffer):!Promise,
+ *   viewport: !Viewport,
+ * }}
+ */
+let ViewerInkHostElement;
+
+// Controller for annotation mode, on Chrome OS only. Fires the following events
+// from its event target:
+// has-unsaved-changes: Fired to indicate there are ink annotations that have
+//     not been saved.
+// set-annotation-undo-state: Contains information about whether undo or redo
+//     options are available.
+export class InkController extends ContentController {
+  /**
+   * @param {!Viewport} viewport
+   * @param {!HTMLDivElement} contentElement
+   */
+  constructor(viewport, contentElement) {
+    super();
+
+    /** @private {!Viewport} */
+    this.viewport_ = viewport;
+
+    /** @private {!HTMLDivElement} */
+    this.contentElement_ = contentElement;
+
+    /** @private {?ViewerInkHostElement} */
+    this.inkHost_ = null;
+
+    /** @private {!EventTarget} */
+    this.eventTarget_ = new EventTarget();
+
+    /** @type {?AnnotationTool} */
+    this.tool_ = null;
+  }
+
+  /** @return {!EventTarget} */
+  getEventTarget() {
+    return this.eventTarget_;
+  }
+
+  /** @param {AnnotationTool} tool */
+  setAnnotationTool(tool) {
+    this.tool_ = tool;
+    if (this.inkHost_) {
+      this.inkHost_.setAnnotationTool(tool);
+    }
+  }
+
+  /** @override */
+  rotateClockwise() {
+    // TODO(dstockwell): implement rotation
+  }
+
+  /** @override */
+  rotateCounterclockwise() {
+    // TODO(dstockwell): implement rotation
+  }
+
+  /** @override */
+  setTwoUpView(enableTwoUpView) {
+    // TODO(dstockwell): Implement two up view.
+  }
+
+  /** @override */
+  viewportChanged() {
+    this.inkHost_.viewportChanged();
+  }
+
+  /** @override */
+  save(requestType) {
+    return this.inkHost_.saveDocument();
+  }
+
+  /** @override */
+  undo() {
+    this.inkHost_.undo();
+  }
+
+  /** @override */
+  redo() {
+    this.inkHost_.redo();
+  }
+
+  /** @override */
+  load(filename, data) {
+    if (!this.inkHost_) {
+      const inkHost = document.createElement('viewer-ink-host');
+      this.contentElement_.appendChild(inkHost);
+      this.inkHost_ = /** @type {!ViewerInkHostElement} */ (inkHost);
+      this.inkHost_.viewport = this.viewport_;
+      inkHost.addEventListener('stroke-added', e => {
+        this.eventTarget_.dispatchEvent(new CustomEvent('has-unsaved-changes'));
+      });
+      inkHost.addEventListener('undo-state-changed', e => {
+        this.eventTarget_.dispatchEvent(
+            new CustomEvent('set-annotation-undo-state', {detail: e.detail}));
+      });
+    }
+    return this.inkHost_.load(filename, data);
+  }
+
+  /** @override */
+  unload() {
+    this.inkHost_.remove();
+    this.inkHost_ = null;
+  }
+}

chrome/browser/resources/pdf/pdf_viewer.js

@@ -20,7 +20,6 @@ import {html} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.
 import {Bookmark} from './bookmark_type.js';
 import {BrowserApi} from './browser_api.js';
 import {FittingType, SaveRequestType, TwoUpViewAction} from './constants.js';
-import {InkController, MessageData} from './controller.js';
 import {PDFMetrics} from './metrics.js';
 import {NavigatorDelegate, PdfNavigator} from './navigator.js';
 import {OpenPdfParamsParser} from './open_pdf_params_parser.js';
@@ -30,6 +29,11 @@ import {DestinationMessageData, DocumentDimensionsMessageData, shouldIgnoreKeyEv
 import {ToolbarManager} from './toolbar_manager.js';
 import {Point} from './viewport.js';
 
+// <if expr="chromeos">
+import {InkController} from './ink_controller.js';
+// </if>
+
+
 /**
  * @typedef {{
  *   type: string,
@@ -170,8 +174,10 @@ class PDFViewerElement extends PDFViewerBaseElement {
     /** @private {boolean} */
     this.toolbarEnabled_ = false;
 
+    // <if expr="chromeos">
     /** @private {?InkController} */
     this.inkController_ = null;
+    // </if>
 
     /** @private {?ToolbarManager} */
     this.toolbarManager_ = null;
@@ -228,6 +234,7 @@ class PDFViewerElement extends PDFViewerBaseElement {
   init(browserApi) {
     super.init(browserApi);
 
+    // <if expr="chromeos">
     this.inkController_ = new InkController(
         this.viewport, /** @type {!HTMLDivElement} */ (this.getContent()));
     this.tracker.add(
@@ -236,6 +243,7 @@ class PDFViewerElement extends PDFViewerBaseElement {
     this.tracker.add(
         this.inkController_.getEventTarget(), 'set-annotation-undo-state',
         e => this.setAnnotationUndoState_(e));
+    // </if>
 
     this.title_ = getFilenameFromURL(this.originalUrl);
     if (this.toolbarEnabled_) {
@@ -828,6 +836,7 @@ class PDFViewerElement extends PDFViewerBaseElement {
     this.inkController_.setAnnotationTool(e.detail.value);
   }
 
+  // <if expr="chromeos">
   /**
    * @param {!CustomEvent<{canUndo: boolean, canRedo: boolean}>} e
    * @private
@@ -836,6 +845,7 @@ class PDFViewerElement extends PDFViewerBaseElement {
     this.getToolbar_().canUndoAnnotation = e.detail.canUndo;
     this.getToolbar_().canRedoAnnotation = e.detail.canRedo;
   }
+  // </if>
 
   /** @override */
   rotateClockwise() {