Commit 375da054 authored by Bruce Dawson's avatar Bruce Dawson Committed by Commit Bot

Revert "Remove code that was added to debug bug 445616"

This reverts commit ef1fe502.

Reason for revert: It's not clear that the underlying bug was fixed
and this change may have made ICU initialization failures happen
silently, leading to later misbehavior.

To be clear, this adds back code to help debug bug 445616.

I'm also taking ownership of the TODO items.

Original change's description:
> Remove code that was added to debug bug 445616
>
> Looks like the bug was fixed, so we don't need this.
>
> Bug: 445616
> Change-Id: Id33ecc6249b1e68337650c733d0877e3326732bf
> Reviewed-on: https://chromium-review.googlesource.com/885021
> Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Jungshik Shin <jshin@chromium.org>
> Reviewed-by: Scott Graham <scottmg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#532612}

TBR=dcheng@chromium.org,sergeyu@chromium.org,scottmg@chromium.org,jshin@chromium.org

Bug: 445616
Change-Id: I2f5dad591791dea8d1427fdceddbf873efbf5f22
Reviewed-on: https://chromium-review.googlesource.com/1099980Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarBruce Dawson <brucedawson@chromium.org>
Reviewed-by: default avatarScott Graham <scottmg@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567485}
parent 93cd24be
......@@ -63,6 +63,14 @@ bool g_called_once = false;
#if ICU_UTIL_DATA_IMPL == ICU_UTIL_DATA_FILE
// To debug http://crbug.com/445616.
int g_debug_icu_last_error;
int g_debug_icu_load;
int g_debug_icu_pf_error_details;
int g_debug_icu_pf_last_error;
#if defined(OS_WIN)
wchar_t g_debug_icu_pf_filename[_MAX_PATH];
#endif // OS_WIN
// Use an unversioned file name to simplify a icu version update down the road.
// No need to change the filename in multiple places (gyp files, windows
// build pkg configurations, etc). 'l' stands for Little Endian.
......@@ -98,8 +106,22 @@ void LazyInitIcuDataFile() {
LOG(ERROR) << "Can't find " << kIcuDataFileName;
return;
}
#if defined(OS_WIN)
// TODO(brucedawson): http://crbug.com/445616
wchar_t tmp_buffer[_MAX_PATH] = {0};
wcscpy_s(tmp_buffer, data_path.value().c_str());
debug::Alias(tmp_buffer);
#endif
data_path = data_path.AppendASCII(kIcuDataFileName);
#else
#if defined(OS_WIN)
// TODO(brucedawson): http://crbug.com/445616
wchar_t tmp_buffer2[_MAX_PATH] = {0};
wcscpy_s(tmp_buffer2, data_path.value().c_str());
debug::Alias(tmp_buffer2);
#endif
#else // !defined(OS_MACOSX)
// Assume it is in the framework bundle's Resources directory.
ScopedCFTypeRef<CFStringRef> data_file_name(
SysUTF8ToCFStringRef(kIcuDataFileName));
......@@ -117,9 +139,24 @@ void LazyInitIcuDataFile() {
#endif // !defined(OS_MACOSX)
File file(data_path, File::FLAG_OPEN | File::FLAG_READ);
if (file.IsValid()) {
// TODO(brucedawson): http://crbug.com/445616.
g_debug_icu_pf_last_error = 0;
g_debug_icu_pf_error_details = 0;
#if defined(OS_WIN)
g_debug_icu_pf_filename[0] = 0;
#endif // OS_WIN
g_icudtl_pf = file.TakePlatformFile();
g_icudtl_region = MemoryMappedFile::Region::kWholeFile;
}
#if defined(OS_WIN)
else {
// TODO(brucedawson): http://crbug.com/445616.
g_debug_icu_pf_last_error = ::GetLastError();
g_debug_icu_pf_error_details = file.error_details();
wcscpy_s(g_debug_icu_pf_filename, data_path.value().c_str());
}
#endif // OS_WIN
}
bool InitializeICUWithFileDescriptorInternal(
......@@ -127,15 +164,18 @@ bool InitializeICUWithFileDescriptorInternal(
const MemoryMappedFile::Region& data_region) {
// This can be called multiple times in tests.
if (g_icudtl_mapped_file) {
g_debug_icu_load = 0; // To debug http://crbug.com/445616.
return true;
}
if (data_fd == kInvalidPlatformFile) {
g_debug_icu_load = 1; // To debug http://crbug.com/445616.
LOG(ERROR) << "Invalid file descriptor to ICU data received.";
return false;
}
std::unique_ptr<MemoryMappedFile> icudtl_mapped_file(new MemoryMappedFile());
if (!icudtl_mapped_file->Initialize(File(data_fd), data_region)) {
g_debug_icu_load = 2; // To debug http://crbug.com/445616.
LOG(ERROR) << "Couldn't mmap icu data file";
return false;
}
......@@ -143,8 +183,12 @@ bool InitializeICUWithFileDescriptorInternal(
UErrorCode err = U_ZERO_ERROR;
udata_setCommonData(const_cast<uint8_t*>(g_icudtl_mapped_file->data()), &err);
if (err != U_ZERO_ERROR) {
g_debug_icu_load = 3; // To debug http://crbug.com/445616.
g_debug_icu_last_error = err;
}
#if defined(OS_ANDROID)
if (err == U_ZERO_ERROR) {
else {
// 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.
// Australia/Sydney, Asia/Seoul, Europe/Paris ). Use JNI to detect the host
......@@ -250,6 +294,20 @@ bool InitializeICU() {
LazyInitIcuDataFile();
result =
InitializeICUWithFileDescriptorInternal(g_icudtl_pf, g_icudtl_region);
#if defined(OS_WIN)
int debug_icu_load = g_debug_icu_load;
debug::Alias(&debug_icu_load);
int debug_icu_last_error = g_debug_icu_last_error;
debug::Alias(&debug_icu_last_error);
int debug_icu_pf_last_error = g_debug_icu_pf_last_error;
debug::Alias(&debug_icu_pf_last_error);
int debug_icu_pf_error_details = g_debug_icu_pf_error_details;
debug::Alias(&debug_icu_pf_error_details);
wchar_t debug_icu_pf_filename[_MAX_PATH] = {0};
wcscpy_s(debug_icu_pf_filename, g_debug_icu_pf_filename);
debug::Alias(&debug_icu_pf_filename);
CHECK(result); // TODO(brucedawson): http://crbug.com/445616
#endif
#endif
// To respond to the timezone change properly, the default timezone
......
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