Commit 31b281ab authored by David's avatar David Committed by Chromium LUCI CQ

Migrate Chromium PDF viewer to use Media App ink resources.

This lets the Chromium PDF viewer get it's ink resources from
chormeos/componeonts/media_app_ui/resources/prod/lib/ink which is
built internally and shipped into CrOS as part of the media app.
The resources themselves are bundled via grit in
chromeos_media_app_bundle_resources.h.

In doing so we add the BUILD flag enable_use_media_app_ink:
* if true, don't build third_party/ink and thus don't create
gen/third_party/ink/ink_resources.pak, instead get resources from
gen/chromeos/chromeos_media_app_bundle_resources.pak
* if false, do what TOT currently does and build third_party/ink

This allows us to only ever build 1 set of ink resources into the
Chrome OS binary and avoid bloating the OS by ~ +2.4MB.
See size increase in
http://b/170161460#comment18 for more context.

This change also updates the externs of the ink library which works
with new ink resources and also the older ink resources
(third_party/ink/build) checked into TOT.

Regression tested with various builds of TOT, new ink resources & TOT
and new ink resources & this cl for the resource plumbing
(grd, flags etc) see regression testing in
http://b/170161460#comment18 for more context.

Note:
* currently enable_use_media_app_ink is just =
enable_cros_media_app but that might change.
* linux-chromeos-chrome trybot will fail until http://cl/347557653
lands and rolls into chromium. There is some time between
http://cl/347557653 rolling in and this cl landing which may mean the
OS is bloated with 2 copies of ink resources but this should be ok as
long as this is done before branch and in reality they will land in
O(day) time.

Bug: b/170161460, 1150244
Change-Id: I8e01ce9863dd00bcbab7b322ad3cf33c1424ec6f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542926Reviewed-by: default avatarDavid Lei <dlei@google.com>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarBugs Nash <bugsnash@chromium.org>
Commit-Queue: David Lei <dlei@google.com>
Cr-Commit-Position: refs/heads/master@{#837450}
parent 93ae228e
......@@ -1372,7 +1372,8 @@ group("extra_resources") {
"//components/autofill/core/browser:autofill_address_rewriter_resources",
]
if (is_chromeos_ash) {
if (is_chromeos_ash && !enable_use_media_app_ink) {
# TODO(crbug/1150244): Remove when enable_use_media_app_ink is default true.
public_deps += [ "//third_party/ink:ink_resources" ]
}
}
......
......@@ -5,6 +5,7 @@
import("//build/config/chromeos/ui_mode.gni")
import("//build/config/features.gni")
import("//build/config/ui.gni")
import("//chrome/browser/resources/pdf/ink/ink.gni")
import("//chrome/common/features.gni")
import("//chromeos/components/chromebox_for_meetings/buildflags/buildflags.gni")
import("//components/nacl/features.gni")
......@@ -926,6 +927,10 @@ static_library("extensions") {
"//url",
]
if (is_chromeos_ash) {
deps += [ "//chromeos/resources:media_app_bundle_resources_grit" ]
}
if (enable_autofill_assistant_api) {
sources += [
"api/autofill_assistant_private/autofill_assistant_private_api.cc",
......@@ -1029,6 +1034,7 @@ static_library("extensions") {
"//chrome/browser/chromeos/crostini:crostini_installer_types_mojom",
"//chrome/browser/devtools",
"//chrome/browser/nearby_sharing/common",
"//chrome/browser/resources/pdf/ink:buildflags",
"//chrome/browser/ui/webui/settings/chromeos/constants:mojom",
"//chromeos",
"//chromeos/attestation",
......
......@@ -28,15 +28,23 @@
#include "base/command_line.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chromeos/file_manager/file_manager_string_util.h"
#include "third_party/ink/grit/ink_resources.h"
#include "chrome/browser/resources/pdf/ink/buildflags.h"
#include "ui/file_manager/grit/file_manager_gen_resources_map.h"
#include "ui/file_manager/grit/file_manager_resources_map.h"
#endif
#if BUILDFLAG(ENABLE_USE_MEDIA_APP_INK)
#include "chromeos/grit/chromeos_media_app_bundle_resources.h"
#else
// TODO(crbug/1150244): Remove when deprecated.
#include "third_party/ink/grit/ink_resources.h"
#endif // BUILDFLAG(ENABLE_USE_MEDIA_APP_INK)
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
#if BUILDFLAG(ENABLE_PDF)
#include <utility>
#include "chrome/browser/pdf/pdf_extension_util.h"
#endif
#endif // BUILDFLAG(ENABLE_PDF)
namespace extensions {
......@@ -82,11 +90,21 @@ ChromeComponentExtensionResourceManager::Data::Data() {
#if BUILDFLAG(IS_CHROMEOS_ASH)
{"chrome_app/chrome_app_icon_32.png", IDR_CHROME_APP_ICON_32},
{"chrome_app/chrome_app_icon_192.png", IDR_CHROME_APP_ICON_192},
#if BUILDFLAG(ENABLE_USE_MEDIA_APP_INK)
// Built in go/bbsrc/lib/BUILD
{"pdf/ink/ink_engine_ink.worker.js",
IDR_MEDIA_APP_INK_ENGINE_INK_WORKER_JS},
{"pdf/ink/ink_engine_ink.wasm", IDR_MEDIA_APP_INK_ENGINE_INK_WASM},
{"pdf/ink/ink_lib_binary.js", IDR_MEDIA_APP_EXPORT_CANVAS_BIN_JS},
{"pdf/ink/ink_loader.js", IDR_MEDIA_APP_INK_JS},
#else
// TODO(crbug/1150244): Remove these when deprecated.
{"pdf/ink/ink_lib_binary.js", IDR_INK_LIB_BINARY_JS},
{"pdf/ink/wasm_ink.worker.js", IDR_INK_WORKER_JS},
{"pdf/ink/wasm_ink.wasm", IDR_INK_WASM},
{"pdf/ink/ink_loader.js", IDR_INK_LOADER_JS},
#endif
#endif // BUILDFLAG(ENABLE_USE_MEDIA_APP_INK)
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
};
AddComponentResourceEntries(kComponentExtensionResources,
......@@ -140,8 +158,8 @@ void ChromeComponentExtensionResourceManager::Data::AddComponentResourceEntries(
gen_folder_path = gen_folder_path.NormalizePathSeparators();
for (size_t i = 0; i < size; ++i) {
base::FilePath resource_path = base::FilePath().AppendASCII(
entries[i].name);
base::FilePath resource_path =
base::FilePath().AppendASCII(entries[i].name);
resource_path = resource_path.NormalizePathSeparators();
if (!gen_folder_path.IsParent(resource_path)) {
......
......@@ -224,7 +224,7 @@ Polymer({
this.ink_.addUndoStateListener(
e => this.dispatchEvent(
new CustomEvent('undo-state-changed', {detail: e})));
this.ink_.setPDF(data);
await this.ink_.setPDF(data);
this.state_ = State.ACTIVE;
this.viewportChanged();
// Wait for the next task to avoid a race where Ink drops the background
......
......@@ -2,6 +2,8 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import("//build/buildflag_header.gni")
import("//chrome/browser/resources/pdf/ink/ink.gni")
import("//third_party/closure_compiler/compile_js.gni")
js_type_check("closure_compile") {
......@@ -10,8 +12,17 @@ js_type_check("closure_compile") {
js_library("ink_api") {
deps = [ "..:annotation_tool" ]
externs_list = [
"//third_party/ink/build/ink_lib_externs.js",
"$externs_path/pending.js",
]
externs_list = [ "$externs_path/pending.js" ]
if (enable_use_media_app_ink) {
externs_list += [ "drawing_canvas_externs.js" ]
} else {
externs_list += [ "//third_party/ink/build/ink_lib_externs.js" ]
}
}
buildflag_header("buildflags") {
header = "buildflags.h"
flags = [ "ENABLE_USE_MEDIA_APP_INK=$enable_use_media_app_ink" ]
}
// 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.
/**
* @fileoverview @externs for export-safe ink.Canvas wrapper.
*
* This file defines types and an interface, drawings.Canvas, that are safe for
* export and satisfy the usage in the Chrome PDF annotation mode.
*/
/** @const Namespace */
const drawings = {};
/**
* A simple rectangle type. The coordinate system depends on context.
*
* @typedef {{left: number, top: number, right: number, bottom: number}}
*/
drawings.Box;
/**
* A PointerEvent-like record type that can be used to pass PointerEvents or an
* equivalent object with adjusted fields to the Ink engine.
*
* Ink depends on the timestamps of events and offsetX/Y, none of which can be
* set in the synthetic PointerEvent constructor.
*
* @typedef {{
* type: string,
* timeStamp: number,
* pointerType: string,
* pointerId: number,
* offsetX: number,
* offsetY: number,
* pressure: number,
* button: number,
* buttons: number,
* }}
*/
drawings.InputEvent;
/**
* Tool types supported in the Ink engine.
*
* See http://go/ink-tools for details on each tool.
*
* Note: These values map to the AnnotationTool definition in Chromium.
*
* @enum {string}
*/
drawings.ToolType = {
MARKER: 'marker',
PEN: 'pen',
HIGHLIGHTER: 'highlighter',
ERASER: 'eraser',
};
/** @typedef {{canUndo: boolean, canRedo: boolean}} */
drawings.UndoState;
/**
* The main interface to Ink.
*
* Many of these functions ultimately delegate to C++ compiled to WebAssembly.
* The WebAssembly module depends on global state, thus it is only safe to have
* one instance at a time.
*
* Use the static execute function to get an instance of the class.
*
* @interface
*/
drawings.Canvas = class {
/**
* Factory function for the Canvas instance.
*
* @param {!Element} parent Element to render the canvas into.
* @return {!Promise<!drawings.Canvas>}
*/
static async execute(parent) {}
/**
* Notifies the attached listener when the undo/redo state changes.
*
* @param {function(!drawings.UndoState):undefined} listener
*/
addUndoRedoListener(listener) {}
/**
* Sets the PDF to edit in the Ink engine. This resets any existing
* annotations, the camera, and any other state in the engine.
*
* @param {!ArrayBuffer|!Uint8Array} buffer
* @return {!Promise<undefined>}
*/
async setPDF(buffer) {}
/**
* Returns a copy of the currently-edited PDF with the Ink annotations
* serialized into it.
*
* @return {!Uint8Array}
*/
getPDF() {}
/**
* Returns the currently-edited PDF with the Ink annotations serialized into
* it. This destructively moves the PDF out of the Ink engine, and the engine
* should not be issued any further strokes or functions calls until setPDF is
* called again.
*
* @return {!Uint8Array}
*/
getPDFDestructive() {}
/**
* Set the camera to the provided box in PDF coordinates.
*
* @param {!drawings.Box} camera
*/
setCamera(camera) {}
/**
* Set the tool parameters in the Ink engine.
*
* See AnnotationTool in Chromium.
*
* @param {{
* tool: (!drawings.ToolType|string),
* color: (string|undefined),
* size: (number|undefined),
* }} toolParams
*/
setTool({tool, color, size}) {}
/**
* Returns a Promise that is resolved when all previous asynchronous engine
* operations are completed. Use this prior to calling getPDF to ensure all
* Ink strokes are serialized.
*
* @return {!Promise<undefined>}
*/
flush() {}
/**
* Set the out of bounds color drawn around the PDF and between pages.
*
* @param {string} color
*/
setOutOfBoundsColor(color) {}
/**
* Set the image used as the page border around each page. Must be a data or
* Object URL to a nine-patch PNG image.
*
* @param {string} url
*/
setBorderImage(url) {}
/**
* Set the page layout to vertical (default) and the given spacing.
*
* @param {number} spacing
*/
setVerticalPageLayout(spacing) {}
/**
* Dispatch the given PointerEvent-like to the Ink engine.
*
* This will cause the engine to draw a line, erase, etc. depending on the
* current tool parameters. Note that the Ink engine depends on the timestamp
* when processing events, but synthetic PointerEvents have the timestamp set
* to the creation time and can't be changed. Instead dispatch an object
* literal with the same fields with the desired timestamp.
*
* @param {!drawings.InputEvent} event
*/
dispatchInput(event) {}
/** Undo the most recent operation. */
undo() {}
/** Redo the last undone operation. */
redo() {}
};
import("//build/config/chrome_build.gni")
import("//chromeos/components/media_app_ui/media_app_ui.gni")
declare_args() {
enable_use_media_app_ink = enable_cros_media_app
}
......@@ -21,11 +21,11 @@ class InkAPI {
/**
* @param {!ArrayBuffer} buffer
*/
setPDF(buffer) {
async setPDF(buffer) {
// We change the type from ArrayBuffer to Uint8Array due to the consequences
// of the buffer being passed across the iframe boundary. This realm has a
// different ArrayBuffer constructor than `buffer`.
this.canvas_.setPDF(new Uint8Array(buffer));
return this.canvas_.setPDF(new Uint8Array(buffer));
}
/**
......
......@@ -5,6 +5,7 @@
import("//build/config/chromeos/ui_mode.gni")
import("//build/config/locales.gni")
import("//chrome/browser/buildflags.gni")
import("//chrome/browser/resources/pdf/ink/ink.gni")
import("//chrome/common/features.gni")
import("//extensions/buildflags/buildflags.gni")
import("//ui/base/ui_features.gni")
......@@ -192,7 +193,6 @@ template("chrome_extra_paks") {
"$root_gen_dir/chromeos/chromeos_resources.pak",
"$root_gen_dir/chromeos/chromeos_scanning_app_resources.pak",
"$root_gen_dir/chromeos/connectivity_diagnostics_resources.pak",
"$root_gen_dir/third_party/ink/ink_resources.pak",
"$root_gen_dir/ui/file_manager/file_manager_gen_resources.pak",
"$root_gen_dir/ui/file_manager/file_manager_resources.pak",
]
......@@ -217,11 +217,17 @@ template("chrome_extra_paks") {
"//chromeos/resources:media_app_resources",
"//chromeos/resources:print_management_resources",
"//chromeos/resources:scanning_app_resources",
"//third_party/ink:ink_resources",
"//ui/file_manager:file_manager_gen_resources",
"//ui/file_manager:resources",
]
if (!enable_use_media_app_ink) {
# TODO(crbug/1150244): Remove when enable_use_media_app_ink is default
# true.
sources += [ "$root_gen_dir/third_party/ink/ink_resources.pak" ]
deps += [ "//third_party/ink:ink_resources" ]
}
if (!is_official_build) {
sources += [
"$root_gen_dir/chromeos/chromeos_file_manager_resources.pak",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment