Skip to content

Commit

Permalink
Cleanup unconditional build usage of //printing
Browse files Browse the repository at this point in the history
Printing is not supported on Fuchsia yet it has been making use of parts
of //printing.  This should not be happening, targets in //printing
should not be used when `enable_basic_printing` is not enabled.

Update printing build to assert that printing is enabled to identify all
possible locations that are doing this and also avoid repeating this
issue in the future.

Update build files to condition dependencies upon printing targets as
necessary.  Similarly wrap some utility code with build flag checks
for `ENABLE_PRINTING` to avoid printing-specific code.

Content web test makes use of printing code to print to a bitmap.  This
is the scenario which was causing Fuchsia to make use of //printing.
Change this to just return an empty bitmap when printing isn't enabled
so that the dependency isn't relied upon for tests which aren't even
relevant for such a condition.

Bug: 1200443
Change-Id: I0c45e280a4c561a7918b9b103b0b4409fdfeb442
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2844307
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Michael Bai <michaelbai@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#876807}
  • Loading branch information
Alan Screen authored and Chromium LUCI CQ committed Apr 27, 2021
1 parent b137b59 commit 53a2813
Show file tree
Hide file tree
Showing 28 changed files with 130 additions and 23 deletions.
14 changes: 10 additions & 4 deletions android_webview/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import("//build/config/android/rules.gni")
import("//components/spellcheck/spellcheck_build_features.gni")
import("//printing/buildflags/buildflags.gni")

java_cpp_enum("browser_enums") {
sources = [
Expand Down Expand Up @@ -233,9 +234,6 @@ source_set("browser") {
"//components/policy/core/browser",
"//components/pref_registry",
"//components/prefs",
"//components/printing/browser",
"//components/printing/common",
"//components/printing/common:mojo_interfaces",
"//components/safe_browsing/android:remote_database_manager",
"//components/safe_browsing/content",
"//components/safe_browsing/content/browser",
Expand Down Expand Up @@ -264,7 +262,6 @@ source_set("browser") {
"//components/webdata/common",
"//content/public/browser",
"//media/mojo:buildflags",
"//printing",
"//services/cert_verifier/public/mojom",
"//services/network/public/mojom",
"//services/proxy_resolver:lib",
Expand All @@ -278,6 +275,15 @@ source_set("browser") {
"//url:gurl_android",
]

if (enable_basic_printing) {
deps += [
"//components/printing/browser",
"//components/printing/common",
"//components/printing/common:mojo_interfaces",
"//printing",
]
}

if (enable_spellcheck) {
deps += [ "//components/spellcheck/browser" ]
}
Expand Down
11 changes: 9 additions & 2 deletions android_webview/renderer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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("//printing/buildflags/buildflags.gni")

source_set("renderer") {
sources = [
"aw_content_renderer_client.cc",
Expand Down Expand Up @@ -43,8 +45,6 @@ source_set("renderer") {
"//components/js_injection/renderer",
"//components/page_load_metrics/renderer",
"//components/power_scheduler",
"//components/printing/common",
"//components/printing/renderer",
"//components/resources",
"//components/safe_browsing/content/common:interfaces",
"//components/safe_browsing/content/renderer:throttles",
Expand All @@ -64,4 +64,11 @@ source_set("renderer") {
"//ui/base",
"//url",
]

if (enable_basic_printing) {
deps += [
"//components/printing/common",
"//components/printing/renderer",
]
}
}
11 changes: 9 additions & 2 deletions chrome/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import("//chrome/browser/share/android/java_sources.gni")
import("//chrome/chrome_paks.gni")
import("//chrome/common/features.gni")
import("//device/vr/buildflags/buildflags.gni")
import("//printing/buildflags/buildflags.gni")
import("//testing/test.gni")
import("//third_party/icu/config.gni")
import("//third_party/protobuf/proto_library.gni")
Expand Down Expand Up @@ -516,7 +517,6 @@ android_library("chrome_java") {
"//mojo/public/java/system:system_impl_java",
"//mojo/public/mojom/base:base_java",
"//net/android:net_java",
"//printing:printing_java",
"//services/data_decoder/public/cpp/android:safe_json_java",
"//services/device/public/java:device_feature_list_java",
"//services/device/public/mojom:mojom_java",
Expand Down Expand Up @@ -654,6 +654,10 @@ android_library("chrome_java") {
# TODO(crbug/1186003): Instead of adding source files, add it as a separate
# dependency when circular deps is resolved.
sources += price_tracking_java_sources

if (enable_basic_printing) {
deps += [ "//printing:printing_java" ]
}
}

# Template for strict mode detection enabling/disabling so that proguard strips out
Expand Down Expand Up @@ -1359,7 +1363,6 @@ android_library("chrome_test_java") {
"//mojo/public/mojom/base:base_java",
"//net/android:net_java",
"//net/android:net_java_test_support",
"//printing:printing_java",
"//services:services_javatests",
"//services/device/public/java:geolocation_java",
"//services/device/public/java:geolocation_java_test_support",
Expand Down Expand Up @@ -1412,6 +1415,10 @@ android_library("chrome_test_java") {

deps += feed_test_deps

if (enable_basic_printing) {
deps += [ "//printing:printing_java" ]
}

srcjar_deps = [ "//chrome/browser:tos_dialog_behavior_generated_enum" ]

data = [
Expand Down
5 changes: 1 addition & 4 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1988,7 +1988,6 @@ static_library("browser") {
"//chrome/common:version_header",
"//chrome/common/net",
"//chrome/common/performance_manager/mojom",
"//chrome/common/printing:printing",
"//chrome/installer/util:with_no_strings",
"//components/assist_ranker",
"//components/autofill/content/browser",
Expand Down Expand Up @@ -2133,7 +2132,6 @@ static_library("browser") {
"//components/policy/core/browser",
"//components/policy/proto",
"//components/prefs",
"//components/printing/browser",
"//components/privacy_sandbox:privacy_sandbox_prefs",
"//components/profile_metrics",
"//components/proxy_config",
Expand Down Expand Up @@ -2294,7 +2292,6 @@ static_library("browser") {
"//net",
"//net:extras",
"//ppapi/buildflags",
"//printing",
"//printing/buildflags",
"//rlz/buildflags",
"//services/audio/public/cpp",
Expand Down Expand Up @@ -4198,7 +4195,6 @@ static_library("browser") {
"//components/keep_alive_registry",
"//components/live_caption:constants",
"//components/pref_registry",
"//components/printing/common:mojo_interfaces",
"//components/services/app_service:lib",
"//components/services/app_service/public/cpp:app_file_handling",
"//components/services/app_service/public/cpp:app_share_target",
Expand Down Expand Up @@ -5510,6 +5506,7 @@ static_library("browser") {
"printing/printing_init.h",
]
deps += [
"//chrome/common/printing",
"//components/printing/browser",
"//components/printing/common:mojo_interfaces",
"//components/services/print_compositor/public/cpp",
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3813,7 +3813,9 @@ std::wstring ChromeContentBrowserClient::GetAppContainerSidForSandboxType(
case sandbox::policy::SandboxType::kXrCompositing:
case sandbox::policy::SandboxType::kNetwork:
case sandbox::policy::SandboxType::kCdm:
#if BUILDFLAG(ENABLE_PRINTING)
case sandbox::policy::SandboxType::kPrintBackend:
#endif
case sandbox::policy::SandboxType::kPrintCompositor:
case sandbox::policy::SandboxType::kAudio:
case sandbox::policy::SandboxType::kSpeechRecognition:
Expand Down Expand Up @@ -3860,7 +3862,9 @@ bool ChromeContentBrowserClient::PreSpawnChild(
case sandbox::policy::SandboxType::kNoSandboxAndElevatedPrivileges:
case sandbox::policy::SandboxType::kXrCompositing:
case sandbox::policy::SandboxType::kCdm:
#if BUILDFLAG(ENABLE_PRINTING)
case sandbox::policy::SandboxType::kPrintBackend:
#endif
case sandbox::policy::SandboxType::kPrintCompositor:
case sandbox::policy::SandboxType::kAudio:
case sandbox::policy::SandboxType::kSpeechRecognition:
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1576,7 +1576,6 @@ static_library("ui") {
"//components/network_session_configurator/common",
"//components/page_load_metrics/browser",
"//components/performance_manager:site_data_proto",
"//components/printing/browser",
"//components/reading_list/features:flags",
"//components/safe_browsing/core/common:safe_browsing_policy_handler",
"//components/safety_check",
Expand Down Expand Up @@ -4719,7 +4718,7 @@ static_library("ui") {
"webui/print_preview/printer_handler.h",
]
deps += [
"//chrome/common/printing:printing",
"//chrome/common/printing",
"//chrome/services/printing/public/mojom",
"//components/printing/common:mojo_interfaces",
"//components/services/print_compositor/public/mojom",
Expand Down
1 change: 1 addition & 0 deletions chrome/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import("//media/media_options.gni")
import("//mojo/public/tools/bindings/mojom.gni")
import("//pdf/features.gni")
import("//ppapi/buildflags/buildflags.gni")
import("//printing/buildflags/buildflags.gni")
import("//third_party/widevine/cdm/widevine.gni")
import("//tools/grit/grit_rule.gni")

Expand Down
2 changes: 1 addition & 1 deletion chrome/service/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import("//build/config/chromeos/ui_mode.gni")
import("//build/config/features.gni")
import("//printing/buildflags/buildflags.gni")

assert(!is_chromeos_ash)
assert(enable_print_preview)

static_library("service") {
sources = [
Expand Down
1 change: 1 addition & 0 deletions content/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ if (is_component_build) {
"//base",
"//build:chromeos_buildflags",
"//media:media_buildflags",
"//printing/buildflags",
"//sandbox:sandbox_buildflags",
"//sandbox/linux:sandbox",
"//sandbox/policy",
Expand Down
3 changes: 3 additions & 0 deletions content/browser/utility_process_sandbox_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "content/public/test/test_service.mojom.h"
#include "content/test/sandbox_status.test-mojom.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "printing/buildflags/buildflags.h"
#include "sandbox/policy/linux/sandbox_linux.h"
#include "sandbox/policy/switches.h"

Expand Down Expand Up @@ -134,7 +135,9 @@ class UtilityProcessSandboxBrowserTest
case SandboxType::kTts:
#endif
case SandboxType::kNetwork:
#if BUILDFLAG(ENABLE_PRINTING)
case SandboxType::kPrintBackend:
#endif
case SandboxType::kSpeechRecognition: {
constexpr int kExpectedPartialSandboxFlags =
SandboxLinux::kSeccompBPF | SandboxLinux::kYama |
Expand Down
5 changes: 5 additions & 0 deletions content/browser/utility_sandbox_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "build/chromeos_buildflags.h"
#include "content/public/common/sandboxed_process_launcher_delegate.h"
#include "content/public/common/zygote/zygote_buildflags.h"
#include "printing/buildflags/buildflags.h"
#include "sandbox/policy/sandbox_type.h"

#if BUILDFLAG(USE_ZYGOTE_HANDLE)
Expand Down Expand Up @@ -47,7 +48,9 @@ UtilitySandboxedProcessLauncherDelegate::
sandbox_type_ == sandbox::policy::SandboxType::kUtility ||
sandbox_type_ == sandbox::policy::SandboxType::kNetwork ||
sandbox_type_ == sandbox::policy::SandboxType::kCdm ||
#if BUILDFLAG(ENABLE_PRINTING)
sandbox_type_ == sandbox::policy::SandboxType::kPrintBackend ||
#endif
sandbox_type_ == sandbox::policy::SandboxType::kPrintCompositor ||
sandbox_type_ == sandbox::policy::SandboxType::kPpapi ||
sandbox_type_ == sandbox::policy::SandboxType::kVideoCapture ||
Expand Down Expand Up @@ -99,7 +102,9 @@ ZygoteHandle UtilitySandboxedProcessLauncherDelegate::GetZygote() {
#endif // BUILDFLAG(ENABLE_LIBASSISTANT_SANDBOX)
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
sandbox_type_ == sandbox::policy::SandboxType::kAudio ||
#if BUILDFLAG(ENABLE_PRINTING)
sandbox_type_ == sandbox::policy::SandboxType::kPrintBackend ||
#endif
sandbox_type_ == sandbox::policy::SandboxType::kSpeechRecognition) {
return GetUnsandboxedZygote();
}
Expand Down
3 changes: 3 additions & 0 deletions content/browser/utility_sandbox_delegate_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "content/public/common/content_switches.h"
#include "content/public/common/sandboxed_process_launcher_delegate.h"
#include "media/mojo/mojom/cdm_service.mojom.h"
#include "printing/buildflags/buildflags.h"
#include "sandbox/policy/features.h"
#include "sandbox/policy/sandbox_type.h"
#include "sandbox/policy/win/sandbox_win.h"
Expand Down Expand Up @@ -263,10 +264,12 @@ bool UtilitySandboxedProcessLauncherDelegate::PreSpawnTarget(
return false;
}

#if BUILDFLAG(ENABLE_PRINTING)
if (sandbox_type_ == sandbox::policy::SandboxType::kPrintBackend) {
if (!PrintBackendPreSpawnTarget(policy))
return false;
}
#endif

return GetContentClient()->browser()->PreSpawnChild(
policy, sandbox_type_, ContentBrowserClient::ChildSpawnFlags::NONE);
Expand Down
11 changes: 9 additions & 2 deletions content/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import("//media/media_options.gni")
import("//mojo/public/tools/bindings/mojom.gni")
import("//net/features.gni")
import("//ppapi/buildflags/buildflags.gni")
import("//printing/buildflags/buildflags.gni")
import("//testing/test.gni")
import("//third_party/blink/public/public_features.gni")
import("//third_party/closure_compiler/closure_args.gni")
Expand Down Expand Up @@ -1518,7 +1519,10 @@ test("content_browsertests") {
"../browser/utility_process_sandbox_browsertest.cc",
"../browser/zygote_host/zygote_browsertest.cc",
]
deps += [ "//ui/gfx:test_support" ]
deps += [
"//printing/buildflags",
"//ui/gfx:test_support",
]

if (use_atk) {
sources += [
Expand Down Expand Up @@ -2336,7 +2340,6 @@ test("content_unittests") {
"//net:test_support",
"//ppapi/buildflags",
"//ppapi/c",
"//printing",
"//services/audio/public/cpp:test_support",
"//services/audio/public/mojom",
"//services/data_decoder/public/cpp:test_support",
Expand Down Expand Up @@ -2411,6 +2414,10 @@ test("content_unittests") {
deps += [ "//components/crash/content/browser/error_reporting" ]
}

if (enable_basic_printing) {
deps += [ "//printing" ]
}

if (enable_plugins) {
sources += [
"../browser/renderer_host/pepper/browser_ppapi_host_test.cc",
Expand Down
6 changes: 5 additions & 1 deletion content/utility/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import("//build/config/chromeos/ui_mode.gni")
import("//chromeos/assistant/assistant.gni")
import("//device/vr/buildflags/buildflags.gni")
import("//media/media_options.gni")
import("//printing/buildflags/buildflags.gni")

source_set("utility") {
# Only the public target should depend on this. All other targets (even
Expand Down Expand Up @@ -49,6 +50,7 @@ source_set("utility") {
"//media:media_buildflags",
"//mojo/public/cpp/bindings",
"//net",
"//printing/buildflags",
"//sandbox",
"//services/audio",
"//services/data_decoder:lib",
Expand Down Expand Up @@ -103,9 +105,11 @@ source_set("utility") {
if (is_linux || is_chromeos) {
deps += [
"//content/utility/speech:speech_recognition_sandbox_hook",
"//printing:printing_sandbox_hook",
"//services/network:network_sandbox_hook",
]
if (enable_basic_printing) {
deps += [ "//printing:printing_sandbox_hook" ]
}
}

if (enable_vr && !is_android) {
Expand Down
7 changes: 7 additions & 0 deletions content/utility/utility_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,17 @@
#include "content/public/common/sandbox_init.h"
#include "content/public/utility/content_utility_client.h"
#include "content/utility/utility_thread_impl.h"
#include "printing/buildflags/buildflags.h"
#include "sandbox/policy/sandbox.h"
#include "services/tracing/public/cpp/trace_startup.h"
#include "third_party/icu/source/common/unicode/unistr.h"
#include "third_party/icu/source/i18n/unicode/timezone.h"

#if defined(OS_LINUX) || defined(OS_CHROMEOS)
#include "content/utility/speech/speech_recognition_sandbox_hook_linux.h"
#if BUILDFLAG(ENABLE_PRINTING)
#include "printing/sandbox/print_backend_sandbox_hook_linux.h"
#endif
#include "sandbox/policy/linux/sandbox_linux.h"
#include "services/audio/audio_sandbox_hook_linux.h"
#include "services/network/network_sandbox_hook_linux.h"
Expand Down Expand Up @@ -118,14 +121,18 @@ int UtilityMain(const MainFunctionParams& parameters) {
sandbox_type == sandbox::policy::SandboxType::kLibassistant ||
#endif // BUILDFLAG(ENABLE_LIBASSISTANT_SANDBOX)
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
#if BUILDFLAG(ENABLE_PRINTING)
sandbox_type == sandbox::policy::SandboxType::kPrintBackend ||
#endif
sandbox_type == sandbox::policy::SandboxType::kAudio ||
sandbox_type == sandbox::policy::SandboxType::kSpeechRecognition) {
sandbox::policy::SandboxLinux::PreSandboxHook pre_sandbox_hook;
if (sandbox_type == sandbox::policy::SandboxType::kNetwork)
pre_sandbox_hook = base::BindOnce(&network::NetworkPreSandboxHook);
#if BUILDFLAG(ENABLE_PRINTING)
else if (sandbox_type == sandbox::policy::SandboxType::kPrintBackend)
pre_sandbox_hook = base::BindOnce(&printing::PrintBackendPreSandboxHook);
#endif // BUILDFLAG(ENABLE_PRINTING)
else if (sandbox_type == sandbox::policy::SandboxType::kAudio)
pre_sandbox_hook = base::BindOnce(&audio::AudioPreSandboxHook);
else if (sandbox_type == sandbox::policy::SandboxType::kSpeechRecognition)
Expand Down
Loading

0 comments on commit 53a2813

Please sign in to comment.