[PDF Ink Signatures] Make annotation brush type selectable

Implement the annotation brush type selection in the frontend of the PDF
viewer. It will send a setAnnotationBrush message to the backend to tell
it what type, color, and size of brush to set the annotation brush to.

This uses the existing annotation bar UI from CrOS's annotation mode.

Some types of brushes shouldn't need a color or size, such as the
eraser. This is reflected in AnnotationBrushParams, where either all
values must be set or the params should be missing completely.

There is a slight difference in behavior from the new annotation mode
and the existing CrOS annotation mode: normally, the annotation tool
would be updated every time the annotation menu was clicked, even if the
values didn't change. The new annotation mode shouldn't do this,
otherwise it will send many messages to the backend.

Bug: 335521186
Change-Id: Ib909927d2d8e6a132c867eafc3d3feadd2cd7c6a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5540210
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Andy Phan <andyphan@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1301653}

chrome/browser/resources/pdf/constants.ts

@@ -2,6 +2,30 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// <if expr="enable_pdf_ink2">
+// Some brushes don't need colors and a size, but the brushes that do should
+// have values for the red, green, and blue colors and a value for the size.
+export interface AnnotationBrush {
+  type: AnnotationBrushType;
+  params?: AnnotationBrushParams;
+}
+
+// The annotation brush params, required for certain brush types.
+export interface AnnotationBrushParams {
+  colorR: number;
+  colorG: number;
+  colorB: number;
+  size: number;
+}
+
+// The different types of annotation brushes.
+export enum AnnotationBrushType {
+  ERASER = 'eraser',
+  HIGHLIGHTER = 'highlighter',
+  PEN = 'pen',
+}
+// </if>
+
 export interface Attachment {
   name: string;
   size: number;

chrome/browser/resources/pdf/controller.ts

@@ -5,6 +5,9 @@
 import {assert} from 'chrome://resources/js/assert.js';
 import {PromiseResolver} from 'chrome://resources/js/promise_resolver.js';
 
+// <if expr="enable_pdf_ink2">
+import type {AnnotationBrush, AnnotationBrushParams, AnnotationBrushType} from './constants.js';
+// </if>
 import type {NamedDestinationMessageData, Rect, SaveRequestType} from './constants.js';
 import type {PdfPluginElement} from './internal_plugin.js';
 import type {Viewport} from './viewport.js';
@@ -41,6 +44,14 @@ interface ThumbnailMessageData {
   height: number;
 }
 
+// <if expr="enable_pdf_ink2">
+// The message sent to the backend to set the annotation brush.
+interface AnnotationBrushMessage extends Partial<AnnotationBrushParams> {
+  type: string;
+  brushType: AnnotationBrushType;
+}
+// </if>
+
 /**
  * Creates a cryptographically secure pseudorandom 128-bit token.
  * @return The generated token as a hex string.
@@ -184,6 +195,16 @@ export class PluginController implements ContentController {
       enable,
     });
   }
+
+  setAnnotationBrush(brush: AnnotationBrush) {
+    const message: AnnotationBrushMessage = {
+      type: 'setAnnotationBrush',
+      brushType: brush.type,
+      ...brush.params,
+    };
+
+    this.postMessage_(message);
+  }
   // </if>
 
   redo() {}

chrome/browser/resources/pdf/elements/viewer-annotations-bar.ts

@@ -17,7 +17,10 @@ import {EventTracker} from 'chrome://resources/js/event_tracker.js';
 import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
 import type {AnnotationTool} from '../annotation_tool.js';
-
+// <if expr="enable_pdf_ink2">
+import type {AnnotationBrush, AnnotationBrushType} from '../constants.js';
+import {PluginController} from '../controller.js';
+// </if>
 // <if expr="enable_ink">
 import {InkController, InkControllerEventType} from '../ink_controller.js';
 // </if>
@@ -63,6 +66,10 @@ export class ViewerAnnotationsBarElement extends PolymerElement {
         type: Boolean,
         value: false,
       },
+
+      // <if expr="enable_pdf_ink2">
+      pdfInk2Enabled: Boolean,
+      // </if>
     };
   }
 
@@ -74,6 +81,10 @@ export class ViewerAnnotationsBarElement extends PolymerElement {
   private inkController_: InkController = InkController.getInstance();
   private tracker_: EventTracker = new EventTracker();
   // </if>
+  // <if expr="enable_pdf_ink2">
+  pdfInk2Enabled: boolean;
+  private pluginController_: PluginController = PluginController.getInstance();
+  // </if>
 
   constructor() {
     super();
@@ -146,16 +157,75 @@ export class ViewerAnnotationsBarElement extends PolymerElement {
         '--pen-tip-border',
         options.selectedColor === '#000000' ? 'currentcolor' :
                                               options.selectedColor);
-    this.annotationTool_ = {
+    const newAnnotationTool: AnnotationTool = {
       tool: tool,
       size: options.selectedSize,
       color: options.selectedColor ? options.selectedColor : undefined,
     };
+    // <if expr="enable_pdf_ink2">
+    if (this.pdfInk2Enabled) {
+      // Only set the annotation brush and `this.annotationTool_` if the values
+      // have changed.
+      if (this.isNewAnnotationTool_(newAnnotationTool)) {
+        this.pluginController_.setAnnotationBrush(
+            this.getAnnotationBrush_(newAnnotationTool));
+        this.annotationTool_ = newAnnotationTool;
+      }
+      return;
+    }
+    // </if>
+
+    this.annotationTool_ = newAnnotationTool;
+
     // <if expr="enable_ink">
     this.inkController_.setAnnotationTool(this.annotationTool_);
     // </if>
   }
 
+  // <if expr="enable_pdf_ink2">
+  /**
+   * Returns whether `annotationTool` is different from `this.annotationTool_`.
+   * @param annotationTool The `AnnotationTool` to check.
+   * @returns True if `this.annotationTool_` is null or doesn't have the same
+   *     values as `annotationTool`, false otherwise.
+   */
+  private isNewAnnotationTool_(annotationTool: AnnotationTool): boolean {
+    if (!this.annotationTool_) {
+      return true;
+    }
+
+    return this.annotationTool_.tool !== annotationTool.tool ||
+        this.annotationTool_.size !== annotationTool.size ||
+        this.annotationTool_.color !== annotationTool.color;
+  }
+
+  /**
+   * @return The `AnnotationBrush` constructed using the values in
+   *     `annotationTool`.
+   */
+  private getAnnotationBrush_(annotationTool: AnnotationTool): AnnotationBrush {
+    const brush: AnnotationBrush = {
+      type: annotationTool.tool as AnnotationBrushType,
+    };
+
+    if (annotationTool.color !== undefined) {
+      // `AnnotationTool`'s color is a hex-coded color string, formatted as
+      // '#ffffff'.
+      const hexColor = annotationTool.color;
+      assert(/^#[0-9a-f]{6}$/.test(hexColor));
+
+      brush.params = {
+        colorR: parseInt(hexColor.substring(1, 3), 16),
+        colorG: parseInt(hexColor.substring(3, 5), 16),
+        colorB: parseInt(hexColor.substring(5, 7), 16),
+        size: annotationTool.size,
+      };
+    }
+
+    return brush;
+  }
+  // </if>
+
   /** @return Whether the annotation tool is using tool `toolName`. */
   private isAnnotationTool_(toolName: string): boolean {
     return !!this.annotationTool_ && this.annotationTool_.tool === toolName;

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

@@ -342,7 +342,11 @@
 </if>
 <if expr="enable_ink or enable_pdf_ink2">
   <template is="dom-if" if="[[showAnnotationsBar_]]">
-    <viewer-annotations-bar annotation-mode="[[annotationMode]]">
+    <viewer-annotations-bar
+<if expr="enable_pdf_ink2">
+      pdf-ink2-enabled="[[pdfInk2Enabled]]"
+</if>
+      annotation-mode="[[annotationMode]]">
     </viewer-annotations-bar>
   </template>
 </if>

chrome/browser/resources/pdf/pdf_viewer_wrapper.ts

@@ -9,6 +9,9 @@ export {AnnotationTool} from './annotation_tool.js';
 // </if>
 export {Bookmark} from './bookmark_type.js';
 export {BrowserApi, ZoomBehavior} from './browser_api.js';
+// <if expr="enable_pdf_ink2">
+export {AnnotationBrush, AnnotationBrushParams, AnnotationBrushType} from './constants.js';
+// </if>
 export {Attachment, FittingType, Point, Rect, SaveRequestType} from './constants.js';
 export {PluginController} from './controller.js';
 export {ViewerAttachmentBarElement} from './elements/viewer-attachment-bar.js';

chrome/test/data/pdf/ink2_annotation_bar_test.ts

@@ -2,11 +2,59 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import type {AnnotationBrushParams} from 'chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/pdf_viewer_wrapper.js';
+import {AnnotationBrushType, PluginController} from 'chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/pdf_viewer_wrapper.js';
+import {assert} from 'chrome://resources/js/assert.js';
 import {waitAfterNextRender} from 'chrome://webui-test/polymer_test_util.js';
 import {isVisible} from 'chrome://webui-test/test_util.js';
 
+import {createMockPdfPluginForTest} from './test_util.js';
+
 const viewer = document.body.querySelector('pdf-viewer')!;
 const viewerToolbar = viewer.$.toolbar;
+const controller = PluginController.getInstance();
+const mockPlugin = createMockPdfPluginForTest();
+controller.setPluginForTesting(mockPlugin);
+
+/**
+ * Test that the annotation brush is of type `type`, and has the `params`, if
+ * necessary. Clears all messages from `mockPlugin` after, otherwise subsequent
+ * calls would continue to find and use the same message.
+ * @param type The type of the annotation brush.
+ * @param params The params for the annotation brush, if necessary for the type
+ *     of brush.
+ */
+function assertAnnotationBrush(
+    type: AnnotationBrushType, params?: AnnotationBrushParams) {
+  const setAnnotationBrushMessage =
+      mockPlugin.findMessage('setAnnotationBrush');
+  chrome.test.assertTrue(setAnnotationBrushMessage !== undefined);
+  chrome.test.assertEq('setAnnotationBrush', setAnnotationBrushMessage.type);
+  chrome.test.assertEq(type, setAnnotationBrushMessage.brushType);
+  const hasParams = params !== undefined;
+  chrome.test.assertEq(
+      hasParams ? params.colorR : undefined, setAnnotationBrushMessage.colorR);
+  chrome.test.assertEq(
+      hasParams ? params.colorG : undefined, setAnnotationBrushMessage.colorG);
+  chrome.test.assertEq(
+      hasParams ? params.colorB : undefined, setAnnotationBrushMessage.colorB);
+  chrome.test.assertEq(
+      hasParams ? params.size : undefined, setAnnotationBrushMessage.size);
+
+  mockPlugin.clearMessages();
+}
+
+/**
+ * Helper to always got a non-null annotation bar. The annotation bar must
+ * exist.
+ * @returns The annotation bar.
+ */
+function getAnnotationsBar() {
+  const annotationsBar =
+      viewerToolbar.shadowRoot!.querySelector('viewer-annotations-bar');
+  assert(annotationsBar);
+  return annotationsBar;
+}
 
 chrome.test.runTests([
   // Test that the annotations bar is shown when annotation mode is enabled and
@@ -16,8 +64,7 @@ chrome.test.runTests([
 
     viewerToolbar.toggleAnnotation();
     await waitAfterNextRender(viewerToolbar);
-    const annotationsBar =
-        viewerToolbar.shadowRoot!.querySelector('viewer-annotations-bar');
+    const annotationsBar = getAnnotationsBar();
 
     // Annotations bar should be visible when annotation mode is enabled.
     chrome.test.assertTrue(viewerToolbar.annotationMode);
@@ -31,4 +78,115 @@ chrome.test.runTests([
     chrome.test.assertFalse(isVisible(annotationsBar));
     chrome.test.succeed();
   },
+  // Test that the pen can be selected. Test that its size and color can be
+  // selected.
+  async function testSelectPen() {
+    viewerToolbar.toggleAnnotation();
+    await waitAfterNextRender(viewerToolbar);
+    chrome.test.assertTrue(viewerToolbar.annotationMode);
+
+    const annotationBar = getAnnotationsBar();
+
+    // Default to pen.
+    assertAnnotationBrush(
+        AnnotationBrushType.PEN,
+        {colorR: 0, colorG: 0, colorB: 0, size: 0.1429});
+
+    // Change the pen size.
+    const penOptions =
+        annotationBar.shadowRoot!.querySelector('#pen viewer-pen-options');
+    assert(penOptions);
+    const sizeButton =
+        penOptions.shadowRoot!.querySelector<HTMLElement>('#sizes [value="1"]');
+    assert(sizeButton);
+    sizeButton.click();
+
+    assertAnnotationBrush(
+        AnnotationBrushType.PEN, {colorR: 0, colorG: 0, colorB: 0, size: 1});
+
+    // Change the pen color.
+    const colorButton = penOptions.shadowRoot!.querySelector<HTMLElement>(
+        '#colors [value="#00b0ff"]');
+    assert(colorButton);
+    colorButton.click();
+
+    assertAnnotationBrush(
+        AnnotationBrushType.PEN,
+        {colorR: 0, colorG: 176, colorB: 255, size: 1});
+    chrome.test.succeed();
+  },
+  // Test that the eraser can be selected.
+  function testSelectEraser() {
+    const annotationBar = getAnnotationsBar();
+
+    // Switch to eraser. It shouldn't have any params.
+    annotationBar.$.eraser.click();
+
+    assertAnnotationBrush(AnnotationBrushType.ERASER);
+    chrome.test.succeed();
+  },
+  // Test that the pen can be selected again, and should have the same settings
+  // as last set in `testSelectPen()`.
+  function testGoBackToPenWithPreviousSettings() {
+    const annotationBar = getAnnotationsBar();
+
+    // Switch back to pen. It should have the previous settings.
+    annotationBar.$.pen.click();
+
+    assertAnnotationBrush(
+        AnnotationBrushType.PEN,
+        {colorR: 0, colorG: 176, colorB: 255, size: 1});
+    chrome.test.succeed();
+  },
+  // Test that the highlighter can be selected. Test that the dropdown menu can
+  // be opened to select a color. Test that the size can be selected.
+  function testSelectHighlighterWithDropdownColor() {
+    const annotationBar = getAnnotationsBar();
+
+    // Switch to highlighter.
+    const highlighter = annotationBar.$.highlighter;
+    highlighter.click();
+
+    assertAnnotationBrush(
+        AnnotationBrushType.HIGHLIGHTER,
+        {colorR: 255, colorG: 188, colorB: 0, size: 0.7143});
+
+    // Fail to change the highlighter color to a hidden color. The
+    // highlighter dropdown needs to be expanded to change to the color, so
+    // without the dropdown, changing the color should fail.
+    const highlighterOptions = highlighter.querySelector('viewer-pen-options');
+    assert(highlighterOptions);
+    const collapsedColorButton =
+        highlighterOptions.shadowRoot!.querySelector<HTMLInputElement>(
+            '#colors [value="#d1c4e9"]');
+    assert(collapsedColorButton);
+    chrome.test.assertTrue(collapsedColorButton.disabled);
+    collapsedColorButton.click();
+
+    // Open the dropdown menu and change the highlighter color.
+    const dropdownMenu =
+        highlighterOptions.shadowRoot!.querySelector<HTMLElement>(
+            '#colors #expand');
+    assert(dropdownMenu);
+    dropdownMenu.click();
+
+    chrome.test.assertFalse(collapsedColorButton.disabled);
+    collapsedColorButton.click();
+
+    assertAnnotationBrush(
+        AnnotationBrushType.HIGHLIGHTER,
+        {colorR: 209, colorG: 196, colorB: 233, size: 0.7143});
+
+    // Change the highlighter size.
+    const sizeButton =
+        highlighterOptions.shadowRoot!.querySelector<HTMLElement>(
+            '#sizes [value="1"]');
+    assert(sizeButton);
+    sizeButton.click();
+
+    assertAnnotationBrush(
+        AnnotationBrushType.HIGHLIGHTER,
+        {colorR: 209, colorG: 196, colorB: 233, size: 1});
+    chrome.test.succeed();
+  },
 ]);

pdf/ink_module.cc

@@ -120,6 +120,7 @@ bool InkModule::OnMessage(const base::Value::Dict& message) {
 
   static constexpr auto kMessageHandlers =
       base::MakeFixedFlatMap<std::string_view, MessageHandler>({
+          {"setAnnotationBrush", &InkModule::HandleSetAnnotationBrushMessage},
           {"setAnnotationMode", &InkModule::HandleSetAnnotationModeMessage},
       });
 
@@ -206,6 +207,11 @@ bool InkModule::OnMouseMove(const blink::WebMouseEvent& event) {
   return true;
 }
 
+void InkModule::HandleSetAnnotationBrushMessage(
+    const base::Value::Dict& message) {
+  // TODO(crbug.com/335524382): Implement setting the brush for Ink2.
+}
+
 void InkModule::HandleSetAnnotationModeMessage(
     const base::Value::Dict& message) {
   enabled_ = message.FindBool("enable").value();

pdf/ink_module.h

@@ -50,6 +50,7 @@ class InkModule {
   bool OnMouseUp(const blink::WebMouseEvent& event);
   bool OnMouseMove(const blink::WebMouseEvent& event);
 
+  void HandleSetAnnotationBrushMessage(const base::Value::Dict& message);
   void HandleSetAnnotationModeMessage(const base::Value::Dict& message);
 
   bool enabled_ = false;