diff options
author | estade@chromium.org <estade@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-04 22:05:04 +0000 |
---|---|---|
committer | estade@chromium.org <estade@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-04 22:05:04 +0000 |
commit | 0376e364459cc52fd417eeda5d071a012bb10ee7 (patch) | |
tree | 470f61a8620dc1922cca95a7bb56fcac7b518236 | |
parent | a7552342506985bc90b33ab54cb62df1bfb190a6 (diff) | |
download | chromium_src-0376e364459cc52fd417eeda5d071a012bb10ee7.zip chromium_src-0376e364459cc52fd417eeda5d071a012bb10ee7.tar.gz chromium_src-0376e364459cc52fd417eeda5d071a012bb10ee7.tar.bz2 |
Add DCHECK for misused string resources.
When replacing a placeholder in a localized string, make sure the correct number of placeholders were found (i.e. the same number of strings that the calling code passed).
BUG=none
TEST=running/testing the browser in debug doesn't cause errors
Review URL: http://codereview.chromium.org/3396025
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@61420 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | app/l10n_util.cc | 110 | ||||
-rw-r--r-- | app/l10n_util.h | 6 | ||||
-rw-r--r-- | base/string_util.cc | 11 | ||||
-rw-r--r-- | chrome/browser/cocoa/page_info_bubble_controller_unittest.mm | 8 | ||||
-rw-r--r-- | chrome/browser/cocoa/page_info_window_mac_unittest.mm | 8 | ||||
-rw-r--r-- | chrome/browser/content_setting_bubble_model.cc | 3 | ||||
-rw-r--r-- | chrome/browser/dom_ui/options/personal_options_handler.cc | 11 | ||||
-rw-r--r-- | chrome/browser/gtk/extension_installed_bubble_gtk.cc | 3 |
8 files changed, 95 insertions, 65 deletions
diff --git a/app/l10n_util.cc b/app/l10n_util.cc index c6530a0..00e8a61 100644 --- a/app/l10n_util.cc +++ b/app/l10n_util.cc @@ -523,10 +523,7 @@ string16 GetStringUTF16(int message_id) { } static string16 GetStringF(int message_id, - const string16& a, - const string16& b, - const string16& c, - const string16& d, + const std::vector<string16>& replacements, std::vector<size_t>* offsets) { // TODO(tc): We could save a string copy if we got the raw string as // a StringPiece and were able to call ReplaceStringPlaceholders with @@ -534,12 +531,35 @@ static string16 GetStringF(int message_id, // practice, the strings should be relatively short. ResourceBundle& rb = ResourceBundle::GetSharedInstance(); const string16& format_string = rb.GetLocalizedString(message_id); - std::vector<string16> subst; - subst.push_back(a); - subst.push_back(b); - subst.push_back(c); - subst.push_back(d); - string16 formatted = ReplaceStringPlaceholders(format_string, subst, + +#ifndef NDEBUG + // Make sure every replacement string is being used, so we don't just + // silently fail to insert one. If |offsets| is non-NULL, then don't do this + // check as the code may simply want to find the placeholders rather than + // actually replacing them. + if (!offsets) { + std::string utf8_string = UTF16ToUTF8(format_string); + + // $9 is the highest allowed placeholder. + for (size_t i = 0; i < 9; ++i) { + bool placeholder_should_exist = replacements.size() > i; + + std::string placeholder = StringPrintf("$%d", static_cast<int>(i + 1)); + size_t pos = utf8_string.find(placeholder.c_str()); + if (placeholder_should_exist) { + DCHECK_NE(std::string::npos, pos) << + " Didn't find a " << placeholder << " placeholder in " << + utf8_string; + } else { + DCHECK_EQ(std::string::npos, pos) << + " Unexpectedly found a " << placeholder << " placeholder in " << + utf8_string; + } + } + } +#endif + + string16 formatted = ReplaceStringPlaceholders(format_string, replacements, offsets); AdjustParagraphDirectionality(&formatted); @@ -548,23 +568,22 @@ static string16 GetStringF(int message_id, #if !defined(WCHAR_T_IS_UTF16) std::wstring GetStringF(int message_id, const std::wstring& a) { - return UTF16ToWide(GetStringF(message_id, WideToUTF16(a), string16(), - string16(), string16(), NULL)); + return UTF16ToWide(GetStringFUTF16(message_id, WideToUTF16(a))); } std::wstring GetStringF(int message_id, const std::wstring& a, const std::wstring& b) { - return UTF16ToWide(GetStringF(message_id, WideToUTF16(a), WideToUTF16(b), - string16(), string16(), NULL)); + return UTF16ToWide(GetStringFUTF16(message_id, WideToUTF16(a), + WideToUTF16(b))); } std::wstring GetStringF(int message_id, const std::wstring& a, const std::wstring& b, const std::wstring& c) { - return UTF16ToWide(GetStringF(message_id, WideToUTF16(a), WideToUTF16(b), - WideToUTF16(c), string16(), NULL)); + return UTF16ToWide(GetStringFUTF16(message_id, WideToUTF16(a), + WideToUTF16(b), WideToUTF16(c))); } std::wstring GetStringF(int message_id, @@ -572,29 +591,27 @@ std::wstring GetStringF(int message_id, const std::wstring& b, const std::wstring& c, const std::wstring& d) { - return UTF16ToWide(GetStringF(message_id, WideToUTF16(a), WideToUTF16(b), - WideToUTF16(c), WideToUTF16(d), NULL)); + return UTF16ToWide(GetStringFUTF16(message_id, WideToUTF16(a), WideToUTF16(b), + WideToUTF16(c), WideToUTF16(d))); } #endif std::string GetStringFUTF8(int message_id, const string16& a) { - return UTF16ToUTF8(GetStringF(message_id, a, string16(), string16(), - string16(), NULL)); + return UTF16ToUTF8(GetStringFUTF16(message_id, a)); } std::string GetStringFUTF8(int message_id, const string16& a, const string16& b) { - return UTF16ToUTF8(GetStringF(message_id, a, b, string16(), string16(), - NULL)); + return UTF16ToUTF8(GetStringFUTF16(message_id, a, b)); } std::string GetStringFUTF8(int message_id, const string16& a, const string16& b, const string16& c) { - return UTF16ToUTF8(GetStringF(message_id, a, b, c, string16(), NULL)); + return UTF16ToUTF8(GetStringFUTF16(message_id, a, b, c)); } std::string GetStringFUTF8(int message_id, @@ -602,25 +619,31 @@ std::string GetStringFUTF8(int message_id, const string16& b, const string16& c, const string16& d) { - return UTF16ToUTF8(GetStringF(message_id, a, b, c, d, NULL)); + return UTF16ToUTF8(GetStringFUTF16(message_id, a, b, c, d)); } string16 GetStringFUTF16(int message_id, const string16& a) { - return GetStringF(message_id, a, string16(), string16(), string16(), NULL); + std::vector<string16> replacements; + replacements.push_back(a); + return GetStringF(message_id, replacements, NULL); } string16 GetStringFUTF16(int message_id, const string16& a, const string16& b) { - return GetStringF(message_id, a, b, string16(), string16(), NULL); + return GetStringFUTF16(message_id, a, b, NULL); } string16 GetStringFUTF16(int message_id, const string16& a, const string16& b, const string16& c) { - return GetStringF(message_id, a, b, c, string16(), NULL); + std::vector<string16> replacements; + replacements.push_back(a); + replacements.push_back(b); + replacements.push_back(c); + return GetStringF(message_id, replacements, NULL); } string16 GetStringFUTF16(int message_id, @@ -628,14 +651,20 @@ string16 GetStringFUTF16(int message_id, const string16& b, const string16& c, const string16& d) { - return GetStringF(message_id, a, b, c, d, NULL); + std::vector<string16> replacements; + replacements.push_back(a); + replacements.push_back(b); + replacements.push_back(c); + replacements.push_back(d); + return GetStringF(message_id, replacements, NULL); } std::wstring GetStringF(int message_id, const std::wstring& a, size_t* offset) { DCHECK(offset); std::vector<size_t> offsets; - string16 result = GetStringF(message_id, WideToUTF16(a), string16(), - string16(), string16(), &offsets); + std::vector<string16> replacements; + replacements.push_back(WideToUTF16(a)); + string16 result = GetStringF(message_id, replacements, &offsets); DCHECK(offsets.size() == 1); *offset = offsets[0]; return UTF16ToWide(result); @@ -645,24 +674,31 @@ std::wstring GetStringF(int message_id, const std::wstring& a, const std::wstring& b, std::vector<size_t>* offsets) { - return UTF16ToWide(GetStringF(message_id, WideToUTF16(a), WideToUTF16(b), - string16(), string16(), offsets)); + std::vector<string16> replacements; + replacements.push_back(WideToUTF16(a)); + replacements.push_back(WideToUTF16(b)); + return UTF16ToWide(GetStringF(message_id, replacements, offsets)); } string16 GetStringFUTF16(int message_id, const string16& a, size_t* offset) { DCHECK(offset); std::vector<size_t> offsets; - string16 result = GetStringFUTF16(message_id, a, string16(), &offsets); + std::vector<string16> replacements; + replacements.push_back(a); + string16 result = GetStringF(message_id, replacements, &offsets); DCHECK(offsets.size() == 1); *offset = offsets[0]; return result; } string16 GetStringFUTF16(int message_id, - const string16& a, - const string16& b, - std::vector<size_t>* offsets) { - return GetStringF(message_id, a, b, string16(), string16(), offsets); + const string16& a, + const string16& b, + std::vector<size_t>* offsets) { + std::vector<string16> replacements; + replacements.push_back(a); + replacements.push_back(b); + return GetStringF(message_id, replacements, offsets); } std::wstring GetStringF(int message_id, int a) { diff --git a/app/l10n_util.h b/app/l10n_util.h index 10dc0a1..54328e7 100644 --- a/app/l10n_util.h +++ b/app/l10n_util.h @@ -150,9 +150,9 @@ string16 GetStringFUTF16(int message_id, const string16& a, size_t* offset); string16 GetStringFUTF16(int message_id, - const string16& a, - const string16& b, - std::vector<size_t>* offsets); + const string16& a, + const string16& b, + std::vector<size_t>* offsets); // Convenience formatters for a single number. std::wstring GetStringF(int message_id, int a); diff --git a/base/string_util.cc b/base/string_util.cc index c7268dc..83ba148 100644 --- a/base/string_util.cc +++ b/base/string_util.cc @@ -902,7 +902,7 @@ OutStringType DoReplaceStringPlaceholders(const FormatStringType& format_string, size_t sub_length = 0; for (typename std::vector<OutStringType>::const_iterator iter = subst.begin(); iter != subst.end(); ++iter) { - sub_length += (*iter).length(); + sub_length += iter->length(); } OutStringType formatted; @@ -927,9 +927,10 @@ OutStringType DoReplaceStringPlaceholders(const FormatStringType& format_string, ReplacementOffset r_offset(index, static_cast<int>(formatted.size())); r_offsets.insert(std::lower_bound(r_offsets.begin(), - r_offsets.end(), r_offset, - &CompareParameter), - r_offset); + r_offsets.end(), + r_offset, + &CompareParameter), + r_offset); } if (index < substitutions) formatted.append(subst.at(index)); @@ -941,7 +942,7 @@ OutStringType DoReplaceStringPlaceholders(const FormatStringType& format_string, } if (offsets) { for (std::vector<ReplacementOffset>::const_iterator i = r_offsets.begin(); - i != r_offsets.end(); ++i) { + i != r_offsets.end(); ++i) { offsets->push_back(i->offset); } } diff --git a/chrome/browser/cocoa/page_info_bubble_controller_unittest.mm b/chrome/browser/cocoa/page_info_bubble_controller_unittest.mm index 387ba44..11c3208 100644 --- a/chrome/browser/cocoa/page_info_bubble_controller_unittest.mm +++ b/chrome/browser/cocoa/page_info_bubble_controller_unittest.mm @@ -101,9 +101,7 @@ class PageInfoBubbleControllerTest : public CocoaTest { TEST_F(PageInfoBubbleControllerTest, NoHistoryNoSecurity) { model_->AddSection(PageInfoModel::ICON_STATE_ERROR, l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE), - l10n_util::GetStringFUTF16( - IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY, - ASCIIToUTF16("google.com")), + l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY), PageInfoModel::SECTION_INFO_IDENTITY); model_->AddSection(PageInfoModel::ICON_STATE_ERROR, l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE), @@ -120,9 +118,7 @@ TEST_F(PageInfoBubbleControllerTest, NoHistoryNoSecurity) { TEST_F(PageInfoBubbleControllerTest, HistoryNoSecurity) { model_->AddSection(PageInfoModel::ICON_STATE_ERROR, l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE), - l10n_util::GetStringFUTF16( - IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY, - ASCIIToUTF16("google.com")), + l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY), PageInfoModel::SECTION_INFO_IDENTITY); model_->AddSection(PageInfoModel::ICON_STATE_ERROR, l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE), diff --git a/chrome/browser/cocoa/page_info_window_mac_unittest.mm b/chrome/browser/cocoa/page_info_window_mac_unittest.mm index 7ef5908..d3db57e 100644 --- a/chrome/browser/cocoa/page_info_window_mac_unittest.mm +++ b/chrome/browser/cocoa/page_info_window_mac_unittest.mm @@ -114,9 +114,7 @@ class PageInfoWindowMacTest : public CocoaTest { TEST_F(PageInfoWindowMacTest, NoHistoryNoSecurity) { model_->AddSection(PageInfoModel::ICON_STATE_ERROR, l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE), - l10n_util::GetStringFUTF16( - IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY, - ASCIIToUTF16("google.com")), + l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY), PageInfoModel::SECTION_INFO_IDENTITY); model_->AddSection(PageInfoModel::ICON_STATE_ERROR, l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE), @@ -134,9 +132,7 @@ TEST_F(PageInfoWindowMacTest, NoHistoryNoSecurity) { TEST_F(PageInfoWindowMacTest, HistoryNoSecurity) { model_->AddSection(PageInfoModel::ICON_STATE_ERROR, l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE), - l10n_util::GetStringFUTF16( - IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY, - ASCIIToUTF16("google.com")), + l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY), PageInfoModel::SECTION_INFO_IDENTITY); model_->AddSection(PageInfoModel::ICON_STATE_ERROR, l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE), diff --git a/chrome/browser/content_setting_bubble_model.cc b/chrome/browser/content_setting_bubble_model.cc index 71055a6..d77f2ca 100644 --- a/chrome/browser/content_setting_bubble_model.cc +++ b/chrome/browser/content_setting_bubble_model.cc @@ -224,8 +224,7 @@ class ContentSettingSingleRadioGroup : public ContentSettingTitleAndLinkModel { COMPILE_ASSERT(arraysize(kBlockIDs) == CONTENT_SETTINGS_NUM_TYPES, Need_a_setting_for_every_content_settings_type); std::string radio_block_label; - radio_block_label = l10n_util::GetStringFUTF8( - kBlockIDs[content_type()], UTF8ToUTF16(display_host)); + radio_block_label = l10n_util::GetStringUTF8(kBlockIDs[content_type()]); radio_group.radio_items.push_back(radio_allow_label); radio_group.radio_items.push_back(radio_block_label); diff --git a/chrome/browser/dom_ui/options/personal_options_handler.cc b/chrome/browser/dom_ui/options/personal_options_handler.cc index 9e4f398..d5de71c 100644 --- a/chrome/browser/dom_ui/options/personal_options_handler.cc +++ b/chrome/browser/dom_ui/options/personal_options_handler.cc @@ -23,6 +23,7 @@ #include "chrome/browser/profile.h" #include "chrome/browser/profile_manager.h" #include "chrome/browser/sync/profile_sync_service.h" +#include "chrome/browser/sync/sync_ui_util.h" #include "chrome/browser/themes/browser_theme_provider.h" #include "chrome/common/notification_service.h" #include "chrome/common/chrome_paths.h" @@ -167,10 +168,12 @@ void PersonalOptionsHandler::Initialize() { void PersonalOptionsHandler::SetSyncStatusUIString(const ListValue* args) { ProfileSyncService* service = dom_ui_->GetProfile()->GetProfileSyncService(); if (service != NULL && ProfileSyncService::IsSyncEnabled()) { - scoped_ptr<Value> status_string(Value::CreateStringValue( - l10n_util::GetStringFUTF16(IDS_SYNC_ACCOUNT_SYNCED_TO_USER_WITH_TIME, - service->GetAuthenticatedUsername(), - service->GetLastSyncedTimeString()))); + string16 status_label; + // TODO(estade): use |link_label|. + string16 link_label; + sync_ui_util::GetStatusLabels(service, &status_label, &link_label); + + scoped_ptr<Value> status_string(Value::CreateStringValue(status_label)); dom_ui_->CallJavascriptFunction( L"PersonalOptions.syncStatusCallback", diff --git a/chrome/browser/gtk/extension_installed_bubble_gtk.cc b/chrome/browser/gtk/extension_installed_bubble_gtk.cc index 7d97f9a..f5a6f5f 100644 --- a/chrome/browser/gtk/extension_installed_bubble_gtk.cc +++ b/chrome/browser/gtk/extension_installed_bubble_gtk.cc @@ -201,8 +201,7 @@ void ExtensionInstalledBubbleGtk::ShowInternal() { // Manage label GtkWidget* manage_label = gtk_label_new( - l10n_util::GetStringFUTF8(IDS_EXTENSION_INSTALLED_MANAGE_INFO, - UTF8ToUTF16(extension_->name())).c_str()); + l10n_util::GetStringUTF8(IDS_EXTENSION_INSTALLED_MANAGE_INFO).c_str()); gtk_label_set_line_wrap(GTK_LABEL(manage_label), TRUE); gtk_widget_set_size_request(manage_label, kTextColumnWidth, -1); gtk_box_pack_start(GTK_BOX(text_column), manage_label, FALSE, FALSE, 0); |