Skip to content

Commit

Permalink
chromecast: Migrate IS_CHROMECAST to common buildflag
Browse files Browse the repository at this point in the history
The IS_CHROMECAST define is declared in 16 different places, which is
risky especially if the define is used in a header file.

Move this to a common buildflag header in //build, alongside the chrome
branding flags. This flag needs to be in a widely scoped location, since
we don't know if we're allowed to use //chromecast until we check the
flag's value.

OS_CHROMEOS uses a global #define for this, and that would work here, but
global ifdefs are discouraged. There are few enough uses of IS_CHROMECAST
that having explicit dependencies on a buildflag header isn't too onerous.

No functional change.

Bug: 1030969
TBR=kylechar@chromium.org, dgozman@chromium.org, benwells@chromium.org, kbr@chromium.org, dalecurtis@chromium.org, reillyg@chromium.org, foolip@chromium.org

Change-Id: I48d9ffccde7e7a680ce572f15a8ba1ef11adcc6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1951981
Commit-Queue: Michael Spang <spang@chromium.org>
Reviewed-by: Luke Halliwell (slow) <halliwell@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722665}
  • Loading branch information
mspang authored and Commit Bot committed Dec 6, 2019
1 parent 6c75381 commit 8a06334
Show file tree
Hide file tree
Showing 41 changed files with 86 additions and 94 deletions.
1 change: 1 addition & 0 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2243,6 +2243,7 @@ component("i18n") {
deps = [
":base",
"//base/third_party/dynamic_annotations",
"//build:chromecast_buildflags",
]

if (!is_debug) {
Expand Down
5 changes: 3 additions & 2 deletions base/i18n/icu_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "base/strings/string_util.h"
#include "base/strings/sys_string_conversions.h"
#include "build/build_config.h"
#include "build/chromecast_buildflags.h"
#include "third_party/icu/source/common/unicode/putil.h"
#include "third_party/icu/source/common/unicode/udata.h"

Expand All @@ -43,7 +44,7 @@
#endif

#if defined(OS_ANDROID) || defined(OS_FUCHSIA) || \
(defined(OS_LINUX) && !defined(IS_CHROMECAST))
(defined(OS_LINUX) && !BUILDFLAG(IS_CHROMECAST))
#include "third_party/icu/source/i18n/unicode/timezone.h"
#endif

Expand Down Expand Up @@ -288,7 +289,7 @@ void InitializeIcuTimeZone() {
fuchsia::IntlProfileWatcher::GetPrimaryTimeZoneIdForIcuInitialization();
icu::TimeZone::adoptDefault(
icu::TimeZone::createTimeZone(icu::UnicodeString::fromUTF8(zone_id)));
#elif defined(OS_LINUX) && !defined(IS_CHROMECAST)
#elif defined(OS_LINUX) && !BUILDFLAG(IS_CHROMECAST)
// To respond to the timezone change properly, the default timezone
// cache in ICU has to be populated on starting up.
// See TimeZoneMonitorLinux::NotifyClientsFromImpl().
Expand Down
7 changes: 7 additions & 0 deletions build/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import("//build/buildflag_header.gni")
import("//build/config/chrome_build.gni")
import("//build/config/chromecast_build.gni")

source_set("buildflag_header_h") {
sources = [
Expand All @@ -26,3 +27,9 @@ buildflag_header("branding_buildflags") {
]
}
}

buildflag_header("chromecast_buildflags") {
header = "chromecast_buildflags.h"

flags = [ "IS_CHROMECAST=$is_chromecast" ]
}
2 changes: 0 additions & 2 deletions chromecast/gpu/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,4 @@ cast_source_set("gpu") {
"//content/public/child",
"//content/public/gpu",
]

defines = [ "IS_CHROMECAST" ]
}
2 changes: 1 addition & 1 deletion components/viz/service/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ viz_component("service") {
allow_circular_includes_from = [ ":gpu_service_dependencies" ]

deps = [
"//build:chromecast_buildflags",
"//cc/base",
"//cc/paint",
"//components/crash/core/common:crash_key",
Expand Down Expand Up @@ -241,7 +242,6 @@ viz_component("service") {
}

if (is_chromecast) {
defines += [ "IS_CHROMECAST" ]
deps += [ "//chromecast/media/service/mojom" ]
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@
#include "base/containers/adapters.h"
#include "base/lazy_instance.h"
#include "base/unguessable_token.h"
#include "build/chromecast_buildflags.h"
#include "components/viz/common/quads/draw_quad.h"
#include "components/viz/common/quads/solid_color_draw_quad.h"
#include "components/viz/common/quads/video_hole_draw_quad.h"
#include "components/viz/service/display/overlay_candidate_list.h"
#include "ui/gfx/geometry/rect_conversions.h"

#if defined(IS_CHROMECAST)
#if BUILDFLAG(IS_CHROMECAST)
#include "base/no_destructor.h"
#include "mojo/public/cpp/bindings/remote.h"
#endif
Expand All @@ -24,7 +25,7 @@ namespace {
base::LazyInstance<OverlayStrategyUnderlayCast::OverlayCompositedCallback>::
DestructorAtExit g_overlay_composited_callback = LAZY_INSTANCE_INITIALIZER;

#if defined(IS_CHROMECAST)
#if BUILDFLAG(IS_CHROMECAST)
// This persistent mojo::Remote is bound then used by all the instances
// of OverlayStrategyUnderlayCast.
mojo::Remote<chromecast::media::mojom::VideoGeometrySetter>&
Expand Down Expand Up @@ -113,7 +114,7 @@ bool OverlayStrategyUnderlayCast::Attempt(

// TODO(guohuideng): when migration to GPU process complete, remove
// the code that's for the browser process compositor.
#if defined(IS_CHROMECAST)
#if BUILDFLAG(IS_CHROMECAST)
if (g_overlay_composited_callback.Get().is_null()) {
DCHECK(GetVideoGeometrySetter());
GetVideoGeometrySetter()->SetVideoGeometry(
Expand Down Expand Up @@ -149,7 +150,7 @@ void OverlayStrategyUnderlayCast::SetOverlayCompositedCallback(
g_overlay_composited_callback.Get() = cb;
}

#if defined(IS_CHROMECAST)
#if BUILDFLAG(IS_CHROMECAST)
// static
void OverlayStrategyUnderlayCast::ConnectVideoGeometrySetter(
mojo::PendingRemote<chromecast::media::mojom::VideoGeometrySetter>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@

#include "base/callback.h"
#include "base/macros.h"
#include "build/chromecast_buildflags.h"
#include "components/viz/service/display/overlay_strategy_underlay.h"
#include "components/viz/service/viz_service_export.h"
#include "ui/gfx/overlay_transform.h"

#if defined(IS_CHROMECAST)
#if BUILDFLAG(IS_CHROMECAST)
#include "chromecast/media/service/mojom/video_geometry_setter.mojom.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#endif
Expand Down Expand Up @@ -46,7 +47,7 @@ class VIZ_SERVICE_EXPORT OverlayStrategyUnderlayCast
base::RepeatingCallback<void(const gfx::RectF&, gfx::OverlayTransform)>;
static void SetOverlayCompositedCallback(const OverlayCompositedCallback& cb);

#if defined(IS_CHROMECAST)
#if BUILDFLAG(IS_CHROMECAST)
// In Chromecast build, OverlayStrategyUnderlayCast needs a valid mojo
// interface to VideoGeometrySetter Service (shared by all instances of
// OverlaystrategyUnderlayCast). This must be called before compositor starts.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/command_line.h"
#include "base/compiler_specific.h"
#include "base/threading/thread_task_runner_handle.h"
#include "build/chromecast_buildflags.h"
#include "cc/base/switches.h"
#include "components/viz/common/display/renderer_settings.h"
#include "components/viz/common/frame_sinks/begin_frame_source.h"
Expand Down Expand Up @@ -127,7 +128,7 @@ std::unique_ptr<OutputSurface> OutputSurfaceProviderImpl::CreateOutputSurface(
renderer_settings);
}
if (!output_surface) {
#if defined(OS_CHROMEOS) || defined(IS_CHROMECAST)
#if defined(OS_CHROMEOS) || BUILDFLAG(IS_CHROMECAST)
// GPU compositing is expected to always work on Chrome OS so we should
// never encounter fatal context error. This could be an unrecoverable
// hardware error or a bug.
Expand Down Expand Up @@ -165,7 +166,7 @@ std::unique_ptr<OutputSurface> OutputSurfaceProviderImpl::CreateOutputSurface(
#endif

if (IsFatalOrSurfaceFailure(context_result)) {
#if defined(OS_CHROMEOS) || defined(IS_CHROMECAST)
#if defined(OS_CHROMEOS) || BUILDFLAG(IS_CHROMECAST)
// GL compositing is expected to always work on Chrome OS so we should
// never encounter fatal context error. This could be an unrecoverable
// hardware error or a bug.
Expand Down
1 change: 0 additions & 1 deletion components/viz/service/main/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ source_set("main") {
}

if (is_chromecast) {
defines += [ "IS_CHROMECAST" ]
deps += [ "//chromecast/media/service/mojom" ]
}
}
4 changes: 0 additions & 4 deletions content/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1983,10 +1983,6 @@ jumbo_source_set("browser") {
defines += [ "ENABLE_PROTECTED_MEDIA_IDENTIFIER_PERMISSION" ]
}

if (is_chromecast) {
defines += [ "IS_CHROMECAST" ]
}

if (is_chromecast && is_linux) {
sources += [
"tracing/cast_tracing_agent.cc",
Expand Down
3 changes: 2 additions & 1 deletion content/browser/message_port_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <utility>

#include "build/build_config.h"
#include "build/chromecast_buildflags.h"
#include "content/browser/renderer_host/render_process_host_impl.h"
#include "content/browser/renderer_host/render_view_host_impl.h"
#include "content/browser/web_contents/web_contents_impl.h"
Expand Down Expand Up @@ -79,7 +80,7 @@ void MessagePortProvider::PostMessageToFrame(
}
#endif

#if defined(OS_FUCHSIA) || defined(IS_CHROMECAST)
#if defined(OS_FUCHSIA) || BUILDFLAG(IS_CHROMECAST)
// static
void MessagePortProvider::PostMessageToFrame(
WebContents* web_contents,
Expand Down
3 changes: 2 additions & 1 deletion content/browser/tracing/tracing_controller_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "base/threading/thread_restrictions.h"
#include "base/values.h"
#include "build/build_config.h"
#include "build/chromecast_buildflags.h"
#include "content/browser/tracing/tracing_controller_impl.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
Expand Down Expand Up @@ -461,7 +462,7 @@ IN_PROC_BROWSER_TEST_F(TracingControllerTest, DoubleStopTracing) {
}

// Only CrOS and Cast support system tracing.
#if defined(OS_CHROMEOS) || (defined(IS_CHROMECAST) && defined(OS_LINUX))
#if defined(OS_CHROMEOS) || (BUILDFLAG(IS_CHROMECAST) && defined(OS_LINUX))
#define MAYBE_SystemTraceEvents SystemTraceEvents
#else
#define MAYBE_SystemTraceEvents DISABLED_SystemTraceEvents
Expand Down
1 change: 1 addition & 0 deletions content/public/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ jumbo_source_set("browser_sources") {
}

public_deps = [
"//build:chromecast_buildflags",
"//components/download/public/common:public",
"//content/public/common:common_sources",
"//ipc",
Expand Down
7 changes: 4 additions & 3 deletions content/public/browser/message_port_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@
#include "base/optional.h"
#include "base/strings/string16.h"
#include "build/build_config.h"
#include "build/chromecast_buildflags.h"
#include "content/common/content_export.h"

#if defined(OS_ANDROID)
#include "base/android/scoped_java_ref.h"
#endif

#if defined(OS_FUCHSIA) || defined(IS_CHROMECAST)
#if defined(OS_FUCHSIA) || BUILDFLAG(IS_CHROMECAST)
#include "third_party/blink/public/common/messaging/message_port_channel.h"
#endif

Expand Down Expand Up @@ -49,15 +50,15 @@ class CONTENT_EXPORT MessagePortProvider {
const base::android::JavaParamRef<jobjectArray>& ports);
#endif // OS_ANDROID

#if defined(OS_FUCHSIA) || defined(IS_CHROMECAST)
#if defined(OS_FUCHSIA) || BUILDFLAG(IS_CHROMECAST)
// If |target_origin| is unset, then no origin scoping is applied.
static void PostMessageToFrame(
WebContents* web_contents,
const base::string16& source_origin,
const base::Optional<base::string16>& target_origin,
const base::string16& data,
std::vector<mojo::ScopedMessagePipeHandle> channels);
#endif // OS_FUCHSIA || IS_CHROMECAST
#endif // OS_FUCHSIA || BUILDFLAG(IS_CHROMECAST)

private:
DISALLOW_IMPLICIT_CONSTRUCTORS(MessagePortProvider);
Expand Down
4 changes: 0 additions & 4 deletions content/renderer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -664,10 +664,6 @@ target(link_target_type, "renderer") {
if (enable_ipc_fuzzer) {
configs += [ "//tools/ipc_fuzzer:ipc_fuzzer_config" ]
}

if (is_chromecast) {
defines += [ "IS_CHROMECAST" ]
}
}

# See comment at the top of //content/BUILD.gn for how this works.
Expand Down
10 changes: 2 additions & 8 deletions content/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1064,10 +1064,6 @@ test("content_browsertests") {

defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]

if (is_chromecast) {
defines += [ "IS_CHROMECAST" ]
}

configs += [
"//build/config:precompiled_headers",
"//build/config/compiler:no_size_t_to_int_warning",
Expand All @@ -1080,6 +1076,7 @@ test("content_browsertests") {
":web_test_support",
":web_ui_test_mojo_bindings",
"//base/test:test_support",
"//build:chromecast_buildflags",
"//components/discardable_memory/client",
"//components/discardable_memory/common",
"//components/discardable_memory/service",
Expand Down Expand Up @@ -1929,6 +1926,7 @@ test("content_unittests") {
"//base/allocator:buildflags",
"//base/test:test_support",
"//base/third_party/dynamic_annotations",
"//build:chromecast_buildflags",
"//cc",
"//cc:test_support",
"//components/cbor",
Expand Down Expand Up @@ -2089,10 +2087,6 @@ test("content_unittests") {
]
}

if (is_chromecast) {
defines += [ "IS_CHROMECAST" ]
}

# Screen capture unit tests.
if (enable_screen_capture && !is_android) {
deps += [
Expand Down
5 changes: 1 addition & 4 deletions extensions/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,14 @@ import("//testing/libfuzzer/fuzzer_test.gni")
# TODO(crbug.com/731689): Assert that extensions are enabled.

source_set("common_constants") {
if (is_chromecast) {
defines = [ "IS_CHROMECAST" ]
}

sources = [
"constants.cc",
"constants.h",
]

deps = [
"//base",
"//build:chromecast_buildflags",
"//components/services/app_service/public/mojom",
"//components/version_info:channel",
"//ui/base",
Expand Down
3 changes: 2 additions & 1 deletion extensions/common/constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "base/stl_util.h"
#include "base/strings/string_piece.h"
#include "build/chromecast_buildflags.h"

namespace extensions {

Expand Down Expand Up @@ -97,7 +98,7 @@ const char kMimeTypePng[] = "image/png";

namespace extension_misc {

#if defined(OS_CHROMEOS) || defined(IS_CHROMECAST)
#if defined(OS_CHROMEOS) || BUILDFLAG(IS_CHROMECAST)
// The extension id for the built-in component extension.
const char kChromeVoxExtensionId[] = "mndnfokpggljbaajbnioimlmbfngpief";
#else
Expand Down
4 changes: 1 addition & 3 deletions gpu/ipc/service/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,12 @@ jumbo_component("service") {
"webgpu_command_buffer_stub.h",
]
defines = [ "GPU_IPC_SERVICE_IMPLEMENTATION" ]
if (is_chromecast) {
defines += [ "IS_CHROMECAST" ]
}
if (subpixel_font_rendering_disabled) {
defines += [ "SUBPIXEL_FONT_RENDERING_DISABLED" ]
}
public_deps = [
"//base",
"//build:chromecast_buildflags",
"//components/viz/common",
"//gpu/config",
"//ipc",
Expand Down
Loading

0 comments on commit 8a06334

Please sign in to comment.