[PDF Ink Signatures] Send messages to set text annotation mode and font

Adding messages to the plugin when turning on or off text annotation
mode and when updating font parameters.

Sends an initial message to the plugin to get the fonts list the first
time fonts are needed.

Bug: 402546154
Change-Id: I3f538e75aef835aee3fcabbe1540bdd3933b37a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6434865
Reviewed-by: Alan Screen <awscreen@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: John Lee <johntlee@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1445322}

chrome/browser/resources/pdf/controller.ts

@@ -6,7 +6,7 @@ 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, AnnotationBrushType} from './constants.js';
+import type {AnnotationBrush, AnnotationBrushType, AnnotationMode, Color, TextAlignment, TextStyles} from './constants.js';
 // </if>
 import type {NamedDestinationMessageData, Rect, SaveRequestType} from './constants.js';
 import type {PdfPluginElement} from './internal_plugin.js';
@@ -51,6 +51,27 @@ interface AnnotationBrushMessage {
   type: string;
   data: AnnotationBrush;
 }
+
+interface AnnotationTextMessageData {
+  typeface: string;
+  fontSize: number;
+  alignment: TextAlignment;
+  style: TextStyles;
+  color: Color;
+}
+
+// setTextAnnotationFont goes from the viewer to the plugin.
+// updateTextAnnotationFont goes from the plugin to the viewer.
+// They contain the same data fields.
+interface AnnotationTextMessage {
+  type: 'setTextAnnotationFont'|'updateTextAnnotationFont';
+  data: AnnotationTextMessageData;
+}
+
+interface AnnotationFontsMessage {
+  type: 'getTextAnnotFontNames';
+  data: string[];
+}
 // </if>
 
 /**
@@ -197,10 +218,10 @@ export class PluginController implements ContentController {
   viewportChanged() {}
 
   // <if expr="enable_pdf_ink2">
-  setAnnotationMode(enable: boolean) {
+  setAnnotationMode(mode: AnnotationMode) {
     this.postMessage_({
       type: 'setAnnotationMode',
-      enable,
+      mode,
     });
   }
 
@@ -212,6 +233,23 @@ export class PluginController implements ContentController {
     });
   }
 
+  getTextAnnotFontNames(): Promise<AnnotationFontsMessage> {
+    // TODO(crbug.com/402546154): Use backend reply instead of dropping it
+    // on the ground and returning dummy data once the backend is in place.
+    this.postMessageWithReply_({
+      type: 'getTextAnnotFontNames',
+    });
+    return Promise.resolve({
+      type: 'getTextAnnotFontNames',
+      data: [
+        'Roboto',
+        'Serif',
+        'Sans',
+        'Monospace',
+      ],
+    });
+  }
+
   setAnnotationBrush(brush: AnnotationBrush) {
     const message: AnnotationBrushMessage = {
       type: 'setAnnotationBrush',
@@ -220,6 +258,15 @@ export class PluginController implements ContentController {
 
     this.postMessage_(message);
   }
+
+  setTextAnnotationFont(textData: AnnotationTextMessageData) {
+    const message: AnnotationTextMessage = {
+      type: 'setTextAnnotationFont',
+      data: textData,
+    };
+
+    this.postMessage_(message);
+  }
   // </if>
 
   redo() {

chrome/browser/resources/pdf/elements/ink_annotation_text_mixin.ts

@@ -66,14 +66,17 @@ export const InkAnnotationTextMixin =
         accessor currentFont: string = '';
         accessor currentSize: number = TEXT_SIZES[0]!;
         accessor colors: ColorOption[] = TEXT_COLORS;
-        accessor fonts: string[] = [
-          'Roboto',
-          'Serif',
-          'Sans',
-          'Monospace',
-        ];
+        accessor fonts: string[] = [];
         accessor sizes: number[] = TEXT_SIZES;
 
+        override firstUpdated() {
+          Ink2Manager.getInstance().getTextAnnotationFonts().then(fonts => {
+            // Set the fonts and then update the text.
+            this.fonts = fonts;
+            this.onTextChanged(Ink2Manager.getInstance().getCurrentText());
+          });
+        }
+
         isSelectedFont(font: string): boolean {
           return font === this.currentFont;
         }

chrome/browser/resources/pdf/ink2_manager.ts

@@ -12,7 +12,7 @@ import {PluginController} from './controller.js';
 export class Ink2Manager extends EventTarget {
   private brush_: AnnotationBrush = {type: AnnotationBrushType.PEN};
   private text_: AnnotationText = {
-    font: 'Roboto',
+    font: '',
     size: 12,
     color: {r: 0, g: 0, b: 0},
     alignment: TextAlignment.LEFT,
@@ -23,8 +23,8 @@ export class Ink2Manager extends EventTarget {
       [TextStyle.STRIKETHROUGH]: false,
     },
   };
-
   private brushResolver_: PromiseResolver<void>|null = null;
+  private fontsResolver_: PromiseResolver<string[]>|null = null;
   private pluginController_: PluginController = PluginController.getInstance();
 
   isInitializationStarted(): boolean {
@@ -86,6 +86,19 @@ export class Ink2Manager extends EventTarget {
     this.setAnnotationBrushInPlugin_();
   }
 
+  getTextAnnotationFonts(): Promise<string[]> {
+    if (this.fontsResolver_ === null) {
+      this.fontsResolver_ = new PromiseResolver();
+      this.pluginController_.getTextAnnotFontNames().then(fontsMessage => {
+        assert(this.fontsResolver_);
+        this.fontsResolver_.resolve(fontsMessage.data);
+        assert(fontsMessage.data.length > 0);
+        this.setTextFont(fontsMessage.data[0]!);
+      });
+    }
+    return this.fontsResolver_.promise;
+  }
+
   setTextFont(font: string) {
     if (this.text_.font === font) {
       return;
@@ -162,9 +175,13 @@ export class Ink2Manager extends EventTarget {
   }
 
   private setAnnotationTextInPlugin_(): void {
-    // TODO (crbug.com/402547554): Replace this with a real call to the plugin,
-    // once the backend has been built.
-    console.info('Send plugin text information ' + JSON.stringify(this.text_));
+    this.pluginController_.setTextAnnotationFont({
+      typeface: this.text_.font,
+      fontSize: this.text_.size,
+      alignment: this.text_.alignment,
+      style: this.text_.styles,
+      color: this.text_.color,
+    });
   }
 
   private fireTextChanged_() {

chrome/browser/resources/pdf/pdf_viewer.ts

@@ -585,8 +585,7 @@ export class PdfViewerElement extends PdfViewerBaseElement {
       if (this.restoreAnnotationMode_ === AnnotationMode.NONE) {
         this.recordEnterExitAnnotationModeMetrics_(newAnnotationMode);
       }
-      this.pluginController_.setAnnotationMode(
-          newAnnotationMode !== AnnotationMode.NONE);
+      this.pluginController_.setAnnotationMode(newAnnotationMode);
       if (newAnnotationMode === AnnotationMode.DRAW &&
           !Ink2Manager.getInstance().isInitializationStarted()) {
         await Ink2Manager.getInstance().initializeBrush();

chrome/test/data/pdf/ink2_annotation_text_mixin_test.ts

@@ -32,11 +32,27 @@ class TestElement extends TestElementBase {
   }
 }
 
+const initializationPromise = eventToPromise('text-changed', manager);
 customElements.define(TestElement.is, TestElement);
 const testElement = document.createElement('test-element') as TestElement;
 document.body.appendChild(testElement);
 
 chrome.test.runTests([
+  async function testInitialization() {
+    // The mixin should request the fonts from the backend initially, and
+    // update its text parameters based on the values in the manager. This
+    // is asynchronous (eventually, the fonts will be requested from the
+    // plugin).
+    const initEvent = await initializationPromise;
+    const expectedFonts = ['Roboto', 'Serif', 'Sans', 'Monospace'];
+    chrome.test.checkDeepEq(expectedFonts, testElement.fonts);
+    chrome.test.checkDeepEq(testElement.currentColor, initEvent.detail.color);
+    chrome.test.assertEq(testElement.currentFont, initEvent.detail.font);
+    chrome.test.assertEq(testElement.currentSize, initEvent.detail.size);
+
+    chrome.test.succeed();
+  },
+
   async function testSetProperties() {
     // Verify that calling onCurrentColorChanged with a new color calls the
     // manager and results in an event firing.
@@ -46,9 +62,7 @@ chrome.test.runTests([
     let whenChanged = eventToPromise('text-changed', manager);
     testElement.onCurrentColorChanged(colorEvent);
     let changedEvent = await whenChanged;
-    chrome.test.assertEq(newColor.r, changedEvent.detail.color.r);
-    chrome.test.assertEq(newColor.g, changedEvent.detail.color.g);
-    chrome.test.assertEq(newColor.b, changedEvent.detail.color.b);
+    chrome.test.checkDeepEq(newColor, changedEvent.detail.color);
 
     // Test firing a change event from a <select> with onFontSelected
     // registered as the listener calls the manager and results in an event.
@@ -78,11 +92,10 @@ chrome.test.runTests([
   function testOnTextChanged() {
     // Initial state
     const initialColor = hexToColor(TEXT_COLORS[0]!.color);
-    chrome.test.assertEq(initialColor.r, testElement.currentColor.r);
-    chrome.test.assertEq(initialColor.g, testElement.currentColor.g);
-    chrome.test.assertEq(initialColor.b, testElement.currentColor.b);
-    chrome.test.assertEq(TEXT_SIZES[0], testElement.currentSize);
-    chrome.test.assertEq('', testElement.currentFont);
+    chrome.test.checkDeepEq(initialColor, testElement.currentColor);
+    chrome.test.assertEq(12, testElement.currentSize);
+    // First font returned by the current dummy code.
+    chrome.test.assertEq('Roboto', testElement.currentFont);
 
     const newColor = hexToColor(TEXT_COLORS[1]!.color);
     testElement.onTextChanged({
@@ -97,9 +110,7 @@ chrome.test.runTests([
         [TextStyle.STRIKETHROUGH]: false,
       },
     });
-    chrome.test.assertEq(newColor.r, testElement.currentColor.r);
-    chrome.test.assertEq(newColor.g, testElement.currentColor.g);
-    chrome.test.assertEq(newColor.b, testElement.currentColor.b);
+    chrome.test.checkDeepEq(newColor, testElement.currentColor);
     chrome.test.assertEq(TEXT_SIZES[1]!, testElement.currentSize);
     chrome.test.assertEq('Serif', testElement.currentFont);
 

chrome/test/data/pdf/ink2_manager_test.ts

@@ -5,12 +5,42 @@
 import type {AnnotationBrush, AnnotationText} from 'chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/pdf_viewer_wrapper.js';
 import {AnnotationBrushType, Ink2Manager, TextAlignment} from 'chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/pdf_viewer_wrapper.js';
 import {assert} from 'chrome://resources/js/assert.js';
+import {eventToPromise} from 'chrome://webui-test/test_util.js';
 
 import {assertAnnotationBrush, setGetAnnotationBrushReply, setupTestMockPluginForInk} from './test_util.js';
+import type {MockPdfPluginElement} from './test_util.js';
 
 const mockPlugin = setupTestMockPluginForInk();
 const manager = Ink2Manager.getInstance();
 
+/**
+ * Tests that the current annotation text matches `expectedText`. Clears all
+ * messages from `mockPlugin` after, otherwise subsequent calls would continue
+ * to find and use the same message.
+ * @param mockPlugin The mock plugin receiving messages.
+ * @param expectedText The expected text that the current annotation text
+ * should match.
+ */
+export function assertAnnotationText(
+    mockPlugin: MockPdfPluginElement, expectedText: AnnotationText) {
+  const setAnnotationTextMessage =
+      mockPlugin.findMessage('setTextAnnotationFont');
+  chrome.test.assertTrue(setAnnotationTextMessage !== undefined);
+  chrome.test.assertEq('setTextAnnotationFont', setAnnotationTextMessage.type);
+  chrome.test.assertEq(
+      expectedText.font, setAnnotationTextMessage.data.typeface);
+  chrome.test.assertEq(
+      expectedText.size, setAnnotationTextMessage.data.fontSize);
+  chrome.test.assertEq(
+      expectedText.alignment, setAnnotationTextMessage.data.alignment);
+  chrome.test.checkDeepEq(
+      expectedText.color, setAnnotationTextMessage.data.color);
+  chrome.test.checkDeepEq(
+      expectedText.styles, setAnnotationTextMessage.data.style);
+
+  mockPlugin.clearMessages();
+}
+
 chrome.test.runTests([
   async function testInitializeBrush() {
     chrome.test.assertFalse(manager.isInitializationStarted());
@@ -52,10 +82,7 @@ chrome.test.runTests([
       chrome.test.assertEq(index + 1, brushUpdates.length);
       chrome.test.assertEq(expected.type, brushUpdates[index]!.type);
       if (expected.color) {
-        assert(brushUpdates[index]!.color);
-        chrome.test.assertEq(expected.color.r, brushUpdates[index]!.color.r);
-        chrome.test.assertEq(expected.color.g, brushUpdates[index]!.color.g);
-        chrome.test.assertEq(expected.color.b, brushUpdates[index]!.color.b);
+        chrome.test.checkDeepEq(expected.color, brushUpdates[index]!.color);
       }
       chrome.test.assertEq(expected.size, brushUpdates[index]!.size);
     }
@@ -98,6 +125,47 @@ chrome.test.runTests([
     chrome.test.succeed();
   },
 
+  async function testGetTextAnnotationFonts() {
+    // Checks that requesting the fonts for the first time retrieves the
+    // fonts from the plugin and fires a text-changed event with the font
+    // set to the first font returned.
+    const whenChanged = eventToPromise('text-changed', manager);
+    const fonts = await manager.getTextAnnotationFonts();
+
+    // For now, these are hardcoded in controller.ts.
+    const expectedFonts = ['Roboto', 'Serif', 'Sans', 'Monospace'];
+    chrome.test.assertEq(fonts.length, expectedFonts.length);
+    for (let i = 0; i < expectedFonts.length; i++) {
+      chrome.test.assertEq(fonts[i], expectedFonts[i]);
+    }
+
+    // Check that the manager requested the fonts.
+    const getTextAnnotFontNamesMessage =
+        mockPlugin.findMessage('getTextAnnotFontNames');
+    chrome.test.assertTrue(getTextAnnotFontNamesMessage !== undefined);
+    chrome.test.assertEq(
+        'getTextAnnotFontNames', getTextAnnotFontNamesMessage.type);
+
+    // Check that an event was fired.
+    const changedEvent = await whenChanged;
+    chrome.test.assertEq('Roboto', changedEvent.detail.font);
+    // Manager should also have sent the initial text settings to the plugin.
+    const expectedText = {
+      font: 'Roboto',
+      size: 12,
+      color: {r: 0, g: 0, b: 0},
+      alignment: TextAlignment.LEFT,
+      styles: {
+        bold: false,
+        italic: false,
+        underline: false,
+        strikethrough: false,
+      },
+    };
+    assertAnnotationText(mockPlugin, expectedText);
+    chrome.test.succeed();
+  },
+
   function testSetTextProperties() {
     const textUpdates: AnnotationText[] = [];
     manager.addEventListener('text-changed', e => {
@@ -108,19 +176,9 @@ chrome.test.runTests([
       chrome.test.assertEq(index + 1, textUpdates.length);
       chrome.test.assertEq(expected.font, textUpdates[index]!.font);
       chrome.test.assertEq(expected.size, textUpdates[index]!.size);
-      chrome.test.assertEq(expected.color.r, textUpdates[index]!.color.r);
-      chrome.test.assertEq(expected.color.g, textUpdates[index]!.color.g);
-      chrome.test.assertEq(expected.color.b, textUpdates[index]!.color.b);
+      chrome.test.checkDeepEq(expected.color, textUpdates[index]!.color);
       chrome.test.assertEq(expected.alignment, textUpdates[index]!.alignment);
-      chrome.test.assertEq(
-          expected.styles.bold, textUpdates[index]!.styles.bold);
-      chrome.test.assertEq(
-          expected.styles.italic, textUpdates[index]!.styles.italic);
-      chrome.test.assertEq(
-          expected.styles.underline, textUpdates[index]!.styles.underline);
-      chrome.test.assertEq(
-          expected.styles.strikethrough,
-          textUpdates[index]!.styles.strikethrough);
+      chrome.test.checkDeepEq(expected.styles, textUpdates[index]!.styles);
     }
 
     // Update font. Note the other `expectedText` values come from the defaults
@@ -138,22 +196,26 @@ chrome.test.runTests([
         strikethrough: false,
       },
     };
+    assertAnnotationText(mockPlugin, expectedText);
     assertTextUpdate(0, expectedText);
 
     // Set size to 10.
     manager.setTextSize(10);
     expectedText.size = 10;
+    assertAnnotationText(mockPlugin, expectedText);
     assertTextUpdate(1, expectedText);
 
     // Set alignment to CENTER.
     manager.setTextAlignment(TextAlignment.CENTER);
     expectedText.alignment = TextAlignment.CENTER;
+    assertAnnotationText(mockPlugin, expectedText);
     assertTextUpdate(2, expectedText);
 
     // Set color to blue.
     const blue = {r: 0, b: 100, g: 0};
     manager.setTextColor(blue);
     expectedText.color = blue;
+    assertAnnotationText(mockPlugin, expectedText);
     assertTextUpdate(3, expectedText);
 
     // Set style to bold + italic.
@@ -161,6 +223,7 @@ chrome.test.runTests([
         {bold: true, italic: true, underline: false, strikethrough: false};
     manager.setTextStyles(boldItalic);
     expectedText.styles = boldItalic;
+    assertAnnotationText(mockPlugin, expectedText);
     assertTextUpdate(4, expectedText);
 
     chrome.test.succeed();

chrome/test/data/pdf/ink2_viewer_toolbar_test.ts

@@ -177,9 +177,19 @@ chrome.test.runTests([
     await microtasksFinished();
     chrome.test.assertEq(AnnotationMode.DRAW, viewerToolbar.annotationMode);
 
-    const enableMessage = mockPlugin.findMessage('setAnnotationMode');
+    let enableMessage = mockPlugin.findMessage('setAnnotationMode');
     chrome.test.assertTrue(enableMessage !== null);
-    chrome.test.assertEq(enableMessage!.enable, true);
+    chrome.test.assertEq(enableMessage!.mode, AnnotationMode.DRAW);
+
+    mockPlugin.clearMessages();
+
+    viewerToolbar.setAnnotationMode(AnnotationMode.TEXT);
+    await microtasksFinished();
+    chrome.test.assertEq(AnnotationMode.TEXT, viewerToolbar.annotationMode);
+
+    enableMessage = mockPlugin.findMessage('setAnnotationMode');
+    chrome.test.assertTrue(enableMessage !== null);
+    chrome.test.assertEq(enableMessage!.mode, AnnotationMode.TEXT);
 
     mockPlugin.clearMessages();
 
@@ -189,7 +199,7 @@ chrome.test.runTests([
 
     const disableMessage = mockPlugin.findMessage('setAnnotationMode');
     chrome.test.assertTrue(disableMessage !== null);
-    chrome.test.assertEq(disableMessage!.enable, false);
+    chrome.test.assertEq(disableMessage!.mode, AnnotationMode.NONE);
     chrome.test.succeed();
   },
   // Test that entering presentation mode exits annotation mode, and exiting

pdf/pdf_ink_module.cc

@@ -314,6 +314,10 @@ bool PdfInkModule::OnMessage(const base::Value::Dict& message) {
           {"setAnnotationBrush",
            &PdfInkModule::HandleSetAnnotationBrushMessage},
           {"setAnnotationMode", &PdfInkModule::HandleSetAnnotationModeMessage},
+          {"getTextAnnotFontNames",
+           &PdfInkModule::HandleGetTextAnnotFontNamesMessage},
+          {"setTextAnnotationFont",
+           &PdfInkModule::HandleSetTextAnnotationFontMessage},
       });
 
   auto it = kMessageHandlers.find(*message.FindString("type"));
@@ -1026,7 +1030,9 @@ void PdfInkModule::HandleSetAnnotationBrushMessage(
 
 void PdfInkModule::HandleSetAnnotationModeMessage(
     const base::Value::Dict& message) {
-  enabled_ = message.FindBool("enable").value();
+  const std::string* mode = message.FindString("mode");
+  CHECK(mode);
+  enabled_ = *mode == "draw";
   client_->OnAnnotationModeToggled(enabled_);
   if (enabled_ && !loaded_data_from_pdf_) {
     loaded_data_from_pdf_ = true;
@@ -1043,6 +1049,18 @@ void PdfInkModule::HandleSetAnnotationModeMessage(
   MaybeSetCursor();
 }
 
+void PdfInkModule::HandleGetTextAnnotFontNamesMessage(
+    const base::Value::Dict& message) {
+  // TODO(crbug.com/409439509): Fill in this method. For now, just create it
+  // so the backend doesn't CHECK when it's sent from the frontend.
+}
+
+void PdfInkModule::HandleSetTextAnnotationFontMessage(
+    const base::Value::Dict& message) {
+  // TODO(crbug.com/409439509): Fill in this method. For now, just create it
+  // so the backend doesn't CHECK when it's sent from the frontend.
+}
+
 PdfInkBrush& PdfInkModule::GetDrawingBrush() {
   // Use the const PdfInkBrush getter and remove the const qualifier to avoid
   // duplicate getter logic.

pdf/pdf_ink_module.h

@@ -334,6 +334,8 @@ class PdfInkModule {
   void HandleGetAnnotationBrushMessage(const base::Value::Dict& message);
   void HandleSetAnnotationBrushMessage(const base::Value::Dict& message);
   void HandleSetAnnotationModeMessage(const base::Value::Dict& message);
+  void HandleGetTextAnnotFontNamesMessage(const base::Value::Dict& message);
+  void HandleSetTextAnnotationFontMessage(const base::Value::Dict& message);
 
   bool is_drawing_stroke() const {
     return std::holds_alternative<DrawingStrokeState>(current_tool_state_);

pdf/pdf_ink_module_unittest.cc

@@ -675,12 +675,12 @@ TEST_P(PdfInkModuleTest, HandleSetAnnotationModeMessage) {
   EXPECT_FALSE(ink_module().enabled());
   EXPECT_TRUE(ink_module().loaded_v2_shapes_.empty());
 
-  message.Set("enable", true);
+  message.Set("mode", "draw");
   EXPECT_TRUE(ink_module().OnMessage(message));
   EXPECT_TRUE(ink_module().enabled());
   EXPECT_THAT(ink_module().loaded_v2_shapes_, kShapeMapMatcher);
 
-  message.Set("enable", false);
+  message.Set("mode", "none");
   EXPECT_TRUE(ink_module().OnMessage(message));
   EXPECT_FALSE(ink_module().enabled());
   EXPECT_THAT(ink_module().loaded_v2_shapes_, kShapeMapMatcher);
@@ -698,7 +698,7 @@ TEST_P(PdfInkModuleTest, MaybeSetCursorWhenTogglingAnnotationMode) {
   EXPECT_TRUE(ink_module().OnMessage(message));
   EXPECT_TRUE(ink_module().enabled());
 
-  message.Set("enable", false);
+  message.Set("mode", "none");
   EXPECT_TRUE(ink_module().OnMessage(message));
   EXPECT_FALSE(ink_module().enabled());
 }

pdf/test/pdf_ink_test_helpers.cc

@@ -49,7 +49,7 @@ base::Value::Dict CreateSetAnnotationBrushMessageForTesting(
 base::Value::Dict CreateSetAnnotationModeMessageForTesting(bool enable) {
   base::Value::Dict message;
   message.Set("type", "setAnnotationMode");
-  message.Set("enable", enable);
+  message.Set("mode", enable ? "draw" : "none");
   return message;
 }