Skip to content

GitLab

T
tangled
Commit e73a63c1 authored by David Dorwin's avatar David Dorwin Committed by Commit Bot
Browse files
Options

[i18n] Remove test-related logic from InitializeTimeZoneData() 

Tests now override a variable used by this function rather than the
function checking whether a test has overridden an environment variable.
This also allowed simplification of the function.

Also made "time zone" consistently two words.

Bug: fuchsia:37487
Change-Id: Ib040dd07dee239faa5f41f9e5c5070309a265a11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2102077
Commit-Queue: David Dorwin <ddorwin@chromium.org>
Reviewed-by: default avatarWez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751701}
parent 84e74dfa
Hide whitespace changes
Inline Side-by-side
Showing with 38 additions and 33 deletions
base/fuchsia/time_zone_data_unittest.cc
View file @ e73a63c1
......@@ -4,7 +4,6 @@
#include "base/i18n/icu_util.h"
#include "base/environment.h"
#include "base/files/file_util.h"
#include "base/strings/string_util.h"
#include "build/build_config.h"
......@@ -28,8 +27,6 @@ const char kRevisionFilePath[] = "/config/data/tzdata/revision.txt";
class TimeZoneDataTest : public testing::Test {
protected:
TimeZoneDataTest() : env_(base::Environment::Create()) {}
void SetUp() override { ResetIcu(); }
void TearDown() override { ResetIcu(); }
......@@ -48,8 +45,6 @@ class TimeZoneDataTest : public testing::Test {
*icu_version = icu::TimeZone::getTZDataVersion(err);
ASSERT_EQ(U_ZERO_ERROR, err) << u_errorName(err);
}
std::unique_ptr<base::Environment> env_;
};
// Loads a file revision.txt from the actual underlying filesystem, which
......@@ -67,8 +62,7 @@ TEST_F(TimeZoneDataTest, CompareSystemRevisionWithExpected) {
return;
}
// Ensure that timezone data is loaded from the default location.
ASSERT_TRUE(env_->UnSetVar("ICU_TIMEZONE_FILES_DIR"));
// ResetIcu() ensures that time zone data is loaded from the default location.
ASSERT_TRUE(InitializeICU());
std::string expected;
......@@ -87,7 +81,7 @@ TEST_F(TimeZoneDataTest, CompareSystemRevisionWithExpected) {
// ICU library versions.
TEST_F(TimeZoneDataTest, TestLoadingTimeZoneDataFromKnownConfigs) {
ASSERT_TRUE(base::DirectoryExists(base::FilePath(kTzDataDirPath)));
ASSERT_TRUE(env_->SetVar("ICU_TIMEZONE_FILES_DIR", kTzDataDirPath));
SetIcuTimeZoneDataDirForTesting(kTzDataDirPath);
EXPECT_TRUE(InitializeICU());
std::string actual;
......@@ -97,7 +91,7 @@ TEST_F(TimeZoneDataTest, TestLoadingTimeZoneDataFromKnownConfigs) {
}
TEST_F(TimeZoneDataTest, DoesNotCrashWithInvalidPath) {
ASSERT_TRUE(env_->SetVar("ICU_TIMEZONE_FILES_DIR", "/some/nonexistent/path"));
SetIcuTimeZoneDataDirForTesting("/some/nonexistent/path");
EXPECT_TRUE(InitializeICU());
std::string actual;
......
base/i18n/icu_util.cc
View file @ e73a63c1
......@@ -89,7 +89,7 @@ const char kIcuExtraDataFileName[] = "icudtl_extra.dat";
// only implemented for OS_FUCHSIA.
#if defined(OS_FUCHSIA)
// The environment variable used to point the ICU data loader to the directory
// containing timezone data. This is available from ICU version 54. The env
// containing time zone data. This is available from ICU version 54. The env
// variable approach is antiquated by today's standards (2019), but is the
// recommended way to configure ICU.
//
......@@ -120,6 +120,12 @@ PlatformFile g_icudtl_extra_pf = kInvalidPlatformFile;
MemoryMappedFile* g_icudtl_extra_mapped_file = nullptr;
MemoryMappedFile::Region g_icudtl_extra_region;
#if defined(OS_FUCHSIA)
// The directory from which the ICU data loader will be configured to load time
// zone data. It is only changed by SetIcuTimeZoneDataDirForTesting().
const char* g_icu_time_zone_data_dir = kIcuTimeZoneDataDir;
#endif // defined(OS_FUCHSIA)
struct PfRegion {
public:
PlatformFile pf;
......@@ -208,30 +214,20 @@ void LazyOpenIcuDataFile() {
g_icudtl_region = pf_region->region;
}
// Loads external timezone data, for configurations where it makes sense.
// If the timezone data directory is not set up properly, we continue but log
// appropriate information for debugging.
// Configures ICU to load external time zone data, if appropriate.
void InitializeExternalTimeZoneData() {
#if defined(OS_FUCHSIA)
std::unique_ptr<base::Environment> env = base::Environment::Create();
// We only set the variable if it was not already set by a test. Fuchsia
// normally does not otherwise set env variables in production code.
if (!env->HasVar(kIcuTimeZoneEnvVariable)) {
env->SetVar(kIcuTimeZoneEnvVariable, kIcuTimeZoneDataDir);
}
std::string tzdata_dir;
env->GetVar(kIcuTimeZoneEnvVariable, &tzdata_dir);
// Try opening the path to check if present. No need to verify that it is a
// directory since ICU loading will return an error if the TimeZone data is
// wrong.
if (!base::DirectoryExists(base::FilePath(tzdata_dir))) {
if (!base::DirectoryExists(base::FilePath(g_icu_time_zone_data_dir))) {
// TODO(https://crbug.com/1061262): Make this FATAL unless expected.
PLOG(WARNING) << "Could not open: '" << tzdata_dir
PLOG(WARNING) << "Could not open: '" << g_icu_time_zone_data_dir
<< "'. Using built-in timezone database";
return;
}
// Set the environment variable to override the location used by ICU.
// Loading can still fail if the directory is empty or its data is invalid.
std::unique_ptr<base::Environment> env = base::Environment::Create();
env->SetVar(kIcuTimeZoneEnvVariable, g_icu_time_zone_data_dir);
#endif // defined(OS_FUCHSIA)
}
......@@ -323,10 +319,10 @@ bool InitializeICUFromDataFile() {
// than relying on ICU's internal initialization.
void InitializeIcuTimeZone() {
#if defined(OS_ANDROID)
// On Android, we can't leave it up to ICU to set the default timezone
// because ICU's timezone detection does not work in many timezones (e.g.
// On Android, we can't leave it up to ICU to set the default time zone
// because ICU's time zone detection does not work in many time zones (e.g.
// Australia/Sydney, Asia/Seoul, Europe/Paris ). Use JNI to detect the host
// timezone and set the ICU default timezone accordingly in advance of
// time zone and set the ICU default time zone accordingly in advance of
// actual use. See crbug.com/722821 and
// https://ssl.icu-project.org/trac/ticket/13208 .
string16 zone_id = android::GetDefaultTimeZoneId();
......@@ -334,7 +330,7 @@ void InitializeIcuTimeZone() {
icu::UnicodeString(FALSE, zone_id.data(), zone_id.length())));
#elif defined(OS_FUCHSIA)
// The platform-specific mechanisms used by ICU's detectHostTimeZone() to
// determine the default timezone will not work on Fuchsia. Therefore,
// determine the default time zone will not work on Fuchsia. Therefore,
// proactively set the default system.
// This is also required by TimeZoneMonitorFuchsia::ProfileMayHaveChanged(),
// which uses the current default to detect whether the time zone changed in
......@@ -346,7 +342,7 @@ void InitializeIcuTimeZone() {
icu::TimeZone::adoptDefault(
icu::TimeZone::createTimeZone(icu::UnicodeString::fromUTF8(zone_id)));
#elif defined(OS_LINUX) && !BUILDFLAG(IS_CHROMECAST)
// To respond to the timezone change properly, the default timezone
// To respond to the time zone change properly, the default time zone
// cache in ICU has to be populated on starting up.
// See TimeZoneMonitorLinux::NotifyClientsFromImpl().
std::unique_ptr<icu::TimeZone> zone(icu::TimeZone::createDefault());
......@@ -471,7 +467,17 @@ void ResetGlobalsForTesting() {
g_icudtl_mapped_file = nullptr;
g_icudtl_extra_pf = kInvalidPlatformFile;
g_icudtl_extra_mapped_file = nullptr;
#if defined(OS_FUCHSIA)
g_icu_time_zone_data_dir = kIcuTimeZoneDataDir;
#endif // defined(OS_FUCHSIA)
}
#if defined(OS_FUCHSIA)
// |dir| must remain valid until ResetGlobalsForTesting() is called.
void SetIcuTimeZoneDataDirForTesting(const char* dir) {
g_icu_time_zone_data_dir = dir;
}
#endif // defined(OS_FUCHSIA)
#endif // (ICU_UTIL_DATA_IMPL == ICU_UTIL_DATA_FILE)
bool InitializeICU() {
......
base/i18n/icu_util.h
View file @ e73a63c1
......@@ -49,6 +49,11 @@ BASE_I18N_EXPORT bool InitializeExtraICUWithFileDescriptor(
const MemoryMappedFile::Region& data_region);
BASE_I18N_EXPORT void ResetGlobalsForTesting();
#if defined(OS_FUCHSIA)
// Overrides the directory used by ICU for external time zone data.
BASE_I18N_EXPORT void SetIcuTimeZoneDataDirForTesting(const char* dir);
#endif // defined(OS_FUCHSIA)
#endif // ICU_UTIL_DATA_IMPL == ICU_UTIL_DATA_FILE
// In a test binary, initialize functions might be called twice.
......
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