Skip to content

Commit

Permalink
[Chromecast] Don't add icui18n dep for Cast
Browse files Browse the repository at this point in the history
This dep increases the OTA size by 600kB, which exceeds the size
allowance for debug builds on some devices. Cast likely needs a
different mitigation for Y2038 as a proper long-term fix.

Bug: internal b/167763382
Test: CQ
Change-Id: I71d1834be09e070a6816731b17e0ae8fb7831676
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2399452
Commit-Queue: Sean Topping <seantopping@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814440}
  • Loading branch information
Sean Topping authored and Commit Bot committed Oct 6, 2020
1 parent f3dc4bd commit 2d5cd23
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 8 deletions.
22 changes: 14 additions & 8 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1261,6 +1261,7 @@ component("base") {
"//base/third_party/double_conversion",
"//base/third_party/dynamic_annotations",
"//build:branding_buildflags",
"//build:chromecast_buildflags",
"//build:chromeos_buildflags",
"//third_party/modp_b64",
]
Expand Down Expand Up @@ -2062,18 +2063,23 @@ component("base") {
if (is_posix && !is_apple) {
sources += [
"time/time_conversion_posix.cc",
"time/time_exploded_icu.cc", # See note below.
"time/time_exploded_posix.cc",
"time/time_now_posix.cc",
]

# The ICU dependency is only needed on systems with a 32-bit time_t.
# However, that cannot be determined from build variables, like
# |current_cpu|, since some 32-bit systems have a 64-bit time_t (and vice
# versa). Thus, the dependency is taken here for all POSIX platforms and the
# compiler+linker should be able to easily detect when the ICU routines will
# not be called and delete them in the final linking.
deps += [ "//third_party/icu:icui18n" ]
# TODO(b/167763382) Find an alternate solution for Chromecast devices, since
# adding the icui18n dep significantly increases the binary size.
if (!is_chromecast) {
sources += [ "time/time_exploded_icu.cc" ]

# The ICU dependency is only needed on systems with a 32-bit time_t.
# However, that cannot be determined from build variables, like
# |current_cpu|, since some 32-bit systems have a 64-bit time_t (and vice
# versa). Thus, the dependency is taken here for all POSIX platforms and
# the compiler+linker should be able to easily detect when the ICU
# routines will not be called and delete them in the final linking.
deps += [ "//third_party/icu:icui18n" ]
}
}

if (is_posix && !is_apple && !is_nacl) {
Expand Down
5 changes: 5 additions & 0 deletions base/time/time_exploded_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "base/numerics/safe_math.h"
#include "base/synchronization/lock.h"
#include "build/build_config.h"
#include "build/chromecast_buildflags.h"

#if defined(OS_NACL)
#include "base/os_compat_nacl.h"
Expand Down Expand Up @@ -126,8 +127,12 @@ void Time::Explode(bool is_local, Exploded* exploded) const {

// For systems with a Y2038 problem, use ICU as the Explode() implementation.
if (sizeof(SysTime) < 8) {
// TODO(b/167763382) Find an alternate solution for Chromecast devices, since
// adding the icui18n dep significantly increases the binary size.
#if !BUILDFLAG(IS_CHROMECAST)
ExplodeUsingIcu(millis_since_unix_epoch, is_local, exploded);
return;
#endif // !BUILDFLAG(IS_CHROMECAST)
}

// Split the |millis_since_unix_epoch| into separate seconds and millisecond
Expand Down

0 comments on commit 2d5cd23

Please sign in to comment.