Commit 04d6e508 authored by scr@chromium.org's avatar scr@chromium.org

I tried to add a pak override in the command line but for some reason it is...

I tried to add a pak override in the command line but for some reason it is stripped before it gets to the browser initialization.  I think that adding synchronization to this particular race is the best way to solve the flaky test.

Previous description:

Add DCHECK for possible flake place - will fail with ASSERT rather than crash.

Crashes in comment 15
http://code.google.com/p/chromium/issues/detail?id=95425#c15
Seem to indicate that data has not been loaded/reloaded.  There are two possible places that could happen and logging in non-test code + ASSERTION in test code can at least get us a bit further in determining where to look.

BUG=95425
TEST=out/Debug/browser_tests --gtest_filter=WebUIBidiCheckerBrowserTestRTL.TestSettingsClearBrowserDataPage

Review URL: https://chromiumcodereview.appspot.com/10543015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@141739 0039d316-1c4b-4281-b951-d872f2087c98
parent e6a71d59
...@@ -70,6 +70,10 @@ void WebUIBidiCheckerBrowserTestRTL::RunBidiCheckerOnPage( ...@@ -70,6 +70,10 @@ void WebUIBidiCheckerBrowserTestRTL::RunBidiCheckerOnPage(
} }
void WebUIBidiCheckerBrowserTestRTL::SetUpOnMainThread() { void WebUIBidiCheckerBrowserTestRTL::SetUpOnMainThread() {
// Ensure that no other page (NTP4, home page, e.g.) is loading when we reload
// the locale resources.
ui_test_utils::NavigateToURL(browser(), GURL("about:blank"));
WebUIBidiCheckerBrowserTest::SetUpOnMainThread(); WebUIBidiCheckerBrowserTest::SetUpOnMainThread();
FilePath pak_path; FilePath pak_path;
app_locale_ = base::i18n::GetConfiguredLocale(); app_locale_ = base::i18n::GetConfiguredLocale();
...@@ -79,7 +83,8 @@ void WebUIBidiCheckerBrowserTestRTL::SetUpOnMainThread() { ...@@ -79,7 +83,8 @@ void WebUIBidiCheckerBrowserTestRTL::SetUpOnMainThread() {
pak_path = pak_path.AppendASCII("fake-bidi"); pak_path = pak_path.AppendASCII("fake-bidi");
pak_path = pak_path.ReplaceExtension(FILE_PATH_LITERAL("pak")); pak_path = pak_path.ReplaceExtension(FILE_PATH_LITERAL("pak"));
ResourceBundle::GetSharedInstance().OverrideLocalePakForTest(pak_path); ResourceBundle::GetSharedInstance().OverrideLocalePakForTest(pak_path);
ResourceBundle::GetSharedInstance().ReloadLocaleResources("he"); ASSERT_FALSE(
ResourceBundle::GetSharedInstance().ReloadLocaleResources("he").empty());
base::i18n::SetICUDefaultLocale("he"); base::i18n::SetICUDefaultLocale("he");
#if defined(OS_POSIX) && defined(TOOLKIT_GTK) #if defined(OS_POSIX) && defined(TOOLKIT_GTK)
gtk_widget_set_default_direction(GTK_TEXT_DIR_RTL); gtk_widget_set_default_direction(GTK_TEXT_DIR_RTL);
...@@ -93,7 +98,9 @@ void WebUIBidiCheckerBrowserTestRTL::CleanUpOnMainThread() { ...@@ -93,7 +98,9 @@ void WebUIBidiCheckerBrowserTestRTL::CleanUpOnMainThread() {
#endif #endif
base::i18n::SetICUDefaultLocale(app_locale_); base::i18n::SetICUDefaultLocale(app_locale_);
ResourceBundle::GetSharedInstance().OverrideLocalePakForTest(FilePath()); ResourceBundle::GetSharedInstance().OverrideLocalePakForTest(FilePath());
ResourceBundle::GetSharedInstance().ReloadLocaleResources(app_locale_); ASSERT_EQ(
app_locale_,
ResourceBundle::GetSharedInstance().ReloadLocaleResources(app_locale_));
} }
// Tests // Tests
......
...@@ -199,6 +199,7 @@ std::string ResourceBundle::LoadLocaleResources( ...@@ -199,6 +199,7 @@ std::string ResourceBundle::LoadLocaleResources(
if (locale_file_path.empty()) { if (locale_file_path.empty()) {
// It's possible that there is no locale.pak. // It's possible that there is no locale.pak.
DLOG(WARNING) << "locale_file_path.empty()";
return std::string(); return std::string();
} }
...@@ -341,6 +342,8 @@ base::StringPiece ResourceBundle::GetRawDataResource( ...@@ -341,6 +342,8 @@ base::StringPiece ResourceBundle::GetRawDataResource(
delegate_->GetRawDataResource(resource_id, scale_factor, &data)) delegate_->GetRawDataResource(resource_id, scale_factor, &data))
return data; return data;
// TODO(tony): Firm up locking for or constraints of calling
// ReloadLocaleResources() and how to CHECK for misuse.
DCHECK(locale_resources_data_.get()); DCHECK(locale_resources_data_.get());
if (locale_resources_data_->GetStringPiece(resource_id, &data)) if (locale_resources_data_->GetStringPiece(resource_id, &data))
return data; return data;
......
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