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

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);