chromium/src
0
Fork 0
Code Activity

[unseasoned-pdf] Move buildflag ENABLE_INK to //pdf/.

Browse Source 
The annotation feature is only allowed when `ENABLE_INK` is true in
//chrome/browser/resources/pdf/, but it is allowed behind flag
`IS_CHROMEOS_ASH` in //pdf/.

To fix this discrepancy, this CL moves `ENABLE_INK` to //pdf/BUILD.gn
so that this flag can be used in //pdf/ as well, and makes sure that
the annotation feature is behind this flag instead of `IS_CHROMEOS_ASH`
in //pdf/.

This CL also makes sure OutOfProcessInstance::HandleSaveMessage()
can perform saving annotations only when `ENABLE_INK` is true.

Bug: 1213294
Change-Id: I325597f10f83c84aca827c8f3c5c3b8a3e6e314b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2931438
Commit-Queue: Hui Yingst <nigi@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#889468}
This commit is contained in:
Hui Yingst
2021-06-04 21:45:39 +00:00
committed by Chromium LUCI CQ
parent 68050dcd21
commit 6dd6918161
13 changed files with 37 additions and 38 deletions

3
chrome/browser/extensions/BUILD.gn

@ -6,7 +6,6 @@ import("//build/config/chromeos/ui_mode.gni")
import("//build/config/features.gni")
import("//build/config/ozone.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")
@ -804,7 +803,6 @@ static_library("extensions") {
"//chrome/browser/profiles:profile",
"//chrome/browser/resource_coordinator:intervention_policy_database_proto",
"//chrome/browser/resource_coordinator:mojo_bindings",
"//chrome/browser/resources/pdf/ink:buildflags",
"//chrome/browser/safe_browsing",
"//chrome/browser/safe_browsing:metrics_collector",
"//chrome/browser/web_applications",
@ -917,6 +915,7 @@ static_library("extensions") {
"//google_apis",
"//media:media_buildflags",
"//net",
"//pdf:buildflags",
"//ppapi/buildflags",
"//printing/buildflags",
"//rlz/buildflags",

1
chrome/browser/extensions/chrome_component_extension_resource_manager.cc

@ -29,7 +29,6 @@
#include "base/command_line.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chromeos/file_manager/file_manager_string_util.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"

2
chrome/browser/pdf/pdf_extension_test.cc

@ -47,7 +47,6 @@
#include "chrome/browser/plugins/plugin_test_utils.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/renderer_context_menu/render_view_context_menu_browsertest_util.h"
#include "chrome/browser/resources/pdf/ink/buildflags.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
@ -97,6 +96,7 @@
#include "extensions/test/result_catcher.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "pdf/buildflags.h"
#include "pdf/pdf_features.h"
#include "services/network/public/cpp/features.h"
#include "testing/gmock/include/gmock/gmock.h"

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

@ -2,7 +2,6 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import("//chrome/browser/resources/pdf/ink/ink.gni")
import("//chrome/common/features.gni")
import("//pdf/features.gni")
import("//third_party/closure_compiler/compile_js.gni")

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

@ -2,7 +2,7 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import("//chrome/browser/resources/pdf/ink/ink.gni")
import("//pdf/features.gni")
import("//third_party/closure_compiler/compile_js.gni")
import("//tools/polymer/html_to_js.gni")

30
chrome/browser/resources/pdf/ink/BUILD.gn

@ -3,28 +3,20 @@
# found in the LICENSE file.
import("//build/buildflag_header.gni")
import("//chrome/browser/resources/pdf/ink/ink.gni")
import("//pdf/features.gni")
import("//third_party/closure_compiler/compile_js.gni")
# Guard the typepchecking targets so the buildflag header can be provided
# without coupling to enable_pdf.
assert(enable_pdf, "enable_pdf check failed")
assert(enable_ink, "ink_api dependencies should be guarded by enable_ink")
if (enable_ink) {
js_type_check("closure_compile") {
deps = [ ":ink_api" ]
}
js_library("ink_api") {
deps = [ "..:annotation_tool" ]
externs_list = [
"$externs_path/pending.js",
"drawing_canvas_externs.js",
]
}
js_type_check("closure_compile") {
deps = [ ":ink_api" ]
}
buildflag_header("buildflags") {
header = "buildflags.h"
flags = [ "ENABLE_INK=$enable_ink" ]
js_library("ink_api") {
deps = [ "..:annotation_tool" ]
externs_list = [
"$externs_path/pending.js",
"drawing_canvas_externs.js",
]
}

7
chrome/browser/resources/pdf/ink/ink.gni

@ -1,7 +0,0 @@
import("//build/config/chrome_build.gni")
import("//chromeos/components/media_app_ui/media_app_ui.gni")
declare_args() {
# Ink libraries are provided by the ChromeOS media app dependency.
enable_ink = enable_cros_media_app
}

2
chrome/test/BUILD.gn

@ -977,7 +977,6 @@ if (!is_android) {
"//chrome/browser/profiling_host:profiling_browsertests",
"//chrome/browser/resource_coordinator:tab_manager_features",
"//chrome/browser/resource_coordinator:tab_metrics_event_proto",
"//chrome/browser/resources/pdf/ink:buildflags",
"//chrome/browser/safe_browsing:verdict_cache_manager_factory",
"//chrome/browser/sharing/proto",
"//chrome/browser/web_applications:browser_tests",
@ -1182,6 +1181,7 @@ if (!is_android) {
"//net",
"//net:extras",
"//net:test_support",
"//pdf:buildflags",
"//pdf:features",
"//ppapi/buildflags",
"//ppapi/shared_impl:test_support",

2
chrome/test/data/pdf/BUILD.gn

@ -2,7 +2,7 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import("//chrome/browser/resources/pdf/ink/ink.gni")
import("//pdf/features.gni")
import("//third_party/closure_compiler/compile_js.gni")
js_type_check("closure_compile") {

3
pdf/BUILD.gn

@ -14,6 +14,7 @@ import("//v8/gni/v8.gni")
buildflag_header("buildflags") {
header = "buildflags.h"
flags = [
"ENABLE_INK=$enable_ink",
"ENABLE_PDF=$enable_pdf",
"ENABLE_PDF_UNSEASONED=$enable_pdf_unseasoned",
]
@ -181,6 +182,7 @@ if (enable_pdf) {
"//components/strings",
"//gin",
"//net",
"//pdf:buildflags",
"//pdf/pdfium/fuzzers",
"//ppapi/cpp:objects",
"//ppapi/cpp/private:internal_module",
@ -312,6 +314,7 @@ if (enable_pdf) {
"//base:i18n",
"//build:chromeos_buildflags",
"//net",
"//pdf:buildflags",
"//ppapi/cpp:objects",
"//ppapi/cpp/private:internal_module",
"//skia",

8
pdf/features.gni

@ -3,12 +3,20 @@
# found in the LICENSE file.
import("//build/config/chromecast_build.gni")
import("//chromeos/components/media_app_ui/media_app_ui.gni")
# Most build code won't need to include this file. Instead you can
# unconditionally depend on "//pdf" which will be a no-op when PDF support is
# disabled.
declare_args() {
# Enable ink libraries provided by the ChromeOS media app dependency.
#
# This argument indicates whether the ink libraries provided by the ChromeOS
# media app dependency is enabled. It also determines whether the annotation
# feature is enabled for the PDF viewer.
enable_ink = enable_cros_media_app
# TODO(crbug.com/702993): Currently disabled on Fuchsia because the PDF Viewer
# currently depends on PPAPI. It does not make sense to port PPAPI, which is
# being deprecated, to Fuchsia. Once the PDF Viewer no longer uses PPAPI, the

9
pdf/out_of_process_instance.cc

@ -28,6 +28,7 @@
#include "net/base/escape.h"
#include "pdf/accessibility.h"
#include "pdf/accessibility_structs.h"
#include "pdf/buildflags.h"
#include "pdf/document_attachment_info.h"
#include "pdf/document_metadata.h"
#include "pdf/pdfium/pdfium_engine.h"
@ -565,9 +566,9 @@ bool OutOfProcessInstance::Init(uint32_t argc,
// can be restarted by exiting annotation mode on ChromeOS, which can set the
// document to an edited state.
set_edit_mode(has_edits);
#if !BUILDFLAG(IS_CHROMEOS_ASH)
#if !BUILDFLAG(ENABLE_INK)
DCHECK(!edit_mode());
#endif // !BUILDFLAG(IS_CHROMEOS_ASH)
#endif // !BUILDFLAG(ENABLE_INK)
pp::PDF::SetCrashData(GetPluginInstance(), original_url, top_level_url);
return engine()->New(original_url, headers);
@ -1095,10 +1096,14 @@ void OutOfProcessInstance::HandleSaveMessage(const pp::VarDictionary& dict) {
dict.Get(pp::Var(kJSSaveRequestType)).AsInt());
switch (request_type) {
case SaveRequestType::kAnnotation:
#if BUILDFLAG(ENABLE_INK)
// In annotation mode, assume the user will make edits and prefer saving
// using the plugin data.
SetPluginCanSave(true);
SaveToBuffer(dict.Get(pp::Var(kJSToken)).AsString());
#else
NOTREACHED();
#endif // BUILDFLAG(ENABLE_INK)
break;
case SaveRequestType::kOriginal:
SetPluginCanSave(false);

5
pdf/pdf_view_plugin_base.cc

@ -34,6 +34,7 @@
#include "net/base/escape.h"
#include "pdf/accessibility.h"
#include "pdf/accessibility_structs.h"
#include "pdf/buildflags.h"
#include "pdf/content_restriction.h"
#include "pdf/document_layout.h"
#include "pdf/document_metadata.h"
@ -455,7 +456,7 @@ void PdfViewPluginBase::SaveToBuffer(const std::string& token) {
if (IsSaveDataSizeValid(data.size()))
data_to_save = base::Value(std::move(data));
} else {
#if BUILDFLAG(IS_CHROMEOS_ASH)
#if BUILDFLAG(ENABLE_INK)
uint32_t length = engine()->GetLoadedByteSize();
if (IsSaveDataSizeValid(length)) {
base::Value::BlobStorage data(length);
@ -464,7 +465,7 @@ void PdfViewPluginBase::SaveToBuffer(const std::string& token) {
}
#else
NOTREACHED();
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
#endif // BUILDFLAG(ENABLE_INK)
}
message.SetKey("dataToSave", std::move(data_to_save));