Cleanup AdjustStringForLocaleDirection() to modify input parameter in place.

As described in the bug, the current behavior is confusing and bug-prone.

BUG=47194
TEST=Check that there are no visible regressions in RTL and LTR language UIs on Linux & Windows.

Review URL: http://codereview.chromium.org/5154009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67237 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 30 insertions, 68 deletions

diff --git a/chrome/browser/browser_main_win.cc b/chrome/browser/browser_main_win.cc
index de6f2f1..f4f9a59 100644
--- a/chrome/browser/browser_main_win.cc
+++ b/chrome/browser/browser_main_win.cc
@@ -134,7 +134,7 @@ void PrepareRestartOnCrashEnviroment(const CommandLine &parsed_command_line) {
   dlg_strings.push_back('|');
   string16 adjusted_string(
       l10n_util::GetStringUTF16(IDS_CRASH_RECOVERY_CONTENT));
-  base::i18n::AdjustStringForLocaleDirection(adjusted_string, &adjusted_string);
+  base::i18n::AdjustStringForLocaleDirection(&adjusted_string);
   dlg_strings.append(adjusted_string);
   dlg_strings.push_back('|');
   dlg_strings.append(ASCIIToUTF16(
diff --git a/chrome/browser/chromeos/dom_ui/imageburner_ui.cc b/chrome/browser/chromeos/dom_ui/imageburner_ui.cc
index 709a061..b827c93 100644
--- a/chrome/browser/chromeos/dom_ui/imageburner_ui.cc
+++ b/chrome/browser/chromeos/dom_ui/imageburner_ui.cc
@@ -312,20 +312,13 @@ std::wstring ImageBurnHandler::GetBurnProgressText(int64 total_burnt,
   std::wstring burnt_size =
       UTF16ToWideHack(FormatBytes(total_burnt, amount_units, true));
 
-  std::wstring burnt_size_localized;
-  if (base::i18n::AdjustStringForLocaleDirection(burnt_size,
-                                                 &burnt_size_localized)) {
-    burnt_size.assign(burnt_size_localized);
-  }
+  base::i18n::AdjustStringForLocaleDirection(&burnt_size);
 
   if (image_size) {
     amount_units = GetByteDisplayUnits(image_size);
     std::wstring total_text =
         UTF16ToWideHack(FormatBytes(image_size, amount_units, true));
-    std::wstring total_text_localized;
-    if (base::i18n::AdjustStringForLocaleDirection(total_text,
-                                                   &total_text_localized))
-    total_text.assign(total_text_localized);
+    base::i18n::AdjustStringForLocaleDirection(&total_text);
 
     return l10n_util::GetStringF(IDS_IMAGEBURN_BURN_PROGRESS,
                                  burnt_size,
diff --git a/chrome/browser/chromeos/dom_ui/system_settings_provider.cc b/chrome/browser/chromeos/dom_ui/system_settings_provider.cc
index 2c6fe6a..77350f4 100644
--- a/chrome/browser/chromeos/dom_ui/system_settings_provider.cc
+++ b/chrome/browser/chromeos/dom_ui/system_settings_provider.cc
@@ -272,9 +272,8 @@ string16 SystemSettingsProvider::GetTimezoneName(
   string16 result(l10n_util::GetStringFUTF16(
       IDS_OPTIONS_SETTINGS_TIMEZONE_DISPLAY_TEMPLATE, ASCIIToUTF16(offset_str),
       string16(name.getBuffer(), name.length()), GetExemplarCity(timezone)));
-  string16 rtl_safe_result = result;
-  base::i18n::AdjustStringForLocaleDirection(result, &rtl_safe_result);
-  return rtl_safe_result;
+  base::i18n::AdjustStringForLocaleDirection(&result);
+  return result;
 }
 
 string16 SystemSettingsProvider::GetTimezoneID(
diff --git a/chrome/browser/dom_ui/dom_ui.cc b/chrome/browser/dom_ui/dom_ui.cc
index e6481b6..06c7d2d 100644
--- a/chrome/browser/dom_ui/dom_ui.cc
+++ b/chrome/browser/dom_ui/dom_ui.cc
@@ -179,9 +179,7 @@ void DOMMessageHandler::SetURLAndTitle(DictionaryValue* dictionary,
     if (using_url_as_the_title) {
       base::i18n::WrapStringWithLTRFormatting(&title_to_set);
     } else {
-      bool success =
-          base::i18n::AdjustStringForLocaleDirection(title, &title_to_set);
-      DCHECK(success ? (title != title_to_set) : (title == title_to_set));
+      base::i18n::AdjustStringForLocaleDirection(&title_to_set);
     }
   }
   dictionary->SetString("title", title_to_set);
diff --git a/chrome/browser/download/download_util.cc b/chrome/browser/download/download_util.cc
index c7a261f..2c856be 100644
--- a/chrome/browser/download/download_util.cc
+++ b/chrome/browser/download/download_util.cc
@@ -544,21 +544,15 @@ std::wstring GetProgressStatusText(DownloadItem* download) {
 
   // Adjust both strings for the locale direction since we don't yet know which
   // string we'll end up using for constructing the final progress string.
-  std::wstring amount_localized;
-  if (base::i18n::AdjustStringForLocaleDirection(amount, &amount_localized)) {
-    amount.assign(amount_localized);
-    received_size.assign(amount_localized);
-  }
+  base::i18n::AdjustStringForLocaleDirection(&amount);
 
   if (total) {
     amount_units = GetByteDisplayUnits(total);
     std::wstring total_text =
         UTF16ToWideHack(FormatBytes(total, amount_units, true));
-    std::wstring total_text_localized;
-    if (base::i18n::AdjustStringForLocaleDirection(total_text,
-                                                   &total_text_localized))
-      total_text.assign(total_text_localized);
+    base::i18n::AdjustStringForLocaleDirection(&total_text);
 
+    base::i18n::AdjustStringForLocaleDirection(&received_size);
     amount = l10n_util::GetStringF(IDS_DOWNLOAD_TAB_PROGRESS_SIZE,
                                    received_size,
                                    total_text);
@@ -569,10 +563,7 @@ std::wstring GetProgressStatusText(DownloadItem* download) {
   amount_units = GetByteDisplayUnits(current_speed);
   std::wstring speed_text = UTF16ToWideHack(FormatSpeed(current_speed,
                                                         amount_units, true));
-  std::wstring speed_text_localized;
-  if (base::i18n::AdjustStringForLocaleDirection(speed_text,
-                                                 &speed_text_localized))
-    speed_text.assign(speed_text_localized);
+  base::i18n::AdjustStringForLocaleDirection(&speed_text);
 
   base::TimeDelta remaining;
   string16 time_remaining;
@@ -582,6 +573,7 @@ std::wstring GetProgressStatusText(DownloadItem* download) {
     time_remaining = TimeFormat::TimeRemaining(remaining);
 
   if (time_remaining.empty()) {
+    base::i18n::AdjustStringForLocaleDirection(&amount);
     return l10n_util::GetStringF(IDS_DOWNLOAD_TAB_PROGRESS_STATUS_TIME_UNKNOWN,
                                  speed_text, amount);
   }
diff --git a/chrome/browser/encoding_menu_controller.cc b/chrome/browser/encoding_menu_controller.cc
index d53536d..7d3a89e 100644
--- a/chrome/browser/encoding_menu_controller.cc
+++ b/chrome/browser/encoding_menu_controller.cc
@@ -131,10 +131,7 @@ void EncodingMenuController::GetEncodingMenuItems(Profile* profile,
   for (it = encodings->begin(); it != encodings->end(); ++it) {
     if (it->encoding_id) {
       std::wstring encoding = it->encoding_display_name;
-      std::wstring bidi_safe_encoding;
-      if (base::i18n::AdjustStringForLocaleDirection(encoding,
-                                                     &bidi_safe_encoding))
-        encoding.swap(bidi_safe_encoding);
+      base::i18n::AdjustStringForLocaleDirection(&encoding);
       menuItems->push_back(EncodingMenuItem(it->encoding_id,
                                             WideToUTF16(encoding)));
     } else {
diff --git a/chrome/browser/language_combobox_model.cc b/chrome/browser/language_combobox_model.cc
index 55b98d4..c2787b8 100644
--- a/chrome/browser/language_combobox_model.cc
+++ b/chrome/browser/language_combobox_model.cc
@@ -90,21 +90,11 @@ std::wstring LanguageList::GetLanguageNameAt(int index) const {
   // We must add directionality formatting to both the native name and the
   // locale name in order to avoid text rendering problems such as misplaced
   // parentheses or languages appearing in the wrong order.
-  std::wstring locale_name_localized;
-  std::wstring locale_name;
-  if (base::i18n::AdjustStringForLocaleDirection(locale_names_[index],
-                                                 &locale_name_localized))
-    locale_name.assign(locale_name_localized);
-  else
-    locale_name.assign(locale_names_[index]);
+  std::wstring locale_name = locale_names_[index];
+  base::i18n::AdjustStringForLocaleDirection(&locale_name);
 
-  std::wstring native_name_localized;
-  std::wstring native_name;
-  if (base::i18n::AdjustStringForLocaleDirection(it->second.native_name,
-                                                 &native_name_localized))
-    native_name.assign(native_name_localized);
-  else
-    native_name.assign(it->second.native_name);
+  std::wstring native_name = it->second.native_name;
+  base::i18n::AdjustStringForLocaleDirection(&native_name);
 
   // We used to have a localizable template here, but none of translators
   // changed the format. We also want to switch the order of locale_name
diff --git a/chrome/browser/location_bar_util.cc b/chrome/browser/location_bar_util.cc
index 3542070d..a398ada 100644
--- a/chrome/browser/location_bar_util.cc
+++ b/chrome/browser/location_bar_util.cc
@@ -35,7 +35,7 @@ std::wstring CalculateMinString(const std::wstring& description) {
   } else {
     min_string = description.substr(0, chop_index);
   }
-  base::i18n::AdjustStringForLocaleDirection(min_string, &min_string);
+  base::i18n::AdjustStringForLocaleDirection(&min_string);
   return min_string;
 }
 
diff --git a/chrome/browser/possible_url_model.cc b/chrome/browser/possible_url_model.cc
index f419762..d84c54f 100644
--- a/chrome/browser/possible_url_model.cc
+++ b/chrome/browser/possible_url_model.cc
@@ -132,13 +132,11 @@ std::wstring PossibleURLModel::GetText(int row, int col_id) {
   }
 
   if (col_id == IDS_ASI_PAGE_COLUMN) {
-    const std::wstring& title = GetTitle(row);
+    std::wstring title = GetTitle(row);
     // TODO(xji): Consider adding a special case if the title text is a URL,
     // since those should always have LTR directionality. Please refer to
     // http://crbug.com/6726 for more information.
-    std::wstring localized_title;
-    if (base::i18n::AdjustStringForLocaleDirection(title, &localized_title))
-      return localized_title;
+    base::i18n::AdjustStringForLocaleDirection(&title);
     return title;
   }
 
diff --git a/chrome/browser/search_engines/template_url.cc b/chrome/browser/search_engines/template_url.cc
index 48f5d7d..c1ebf517 100644
--- a/chrome/browser/search_engines/template_url.cc
+++ b/chrome/browser/search_engines/template_url.cc
@@ -578,11 +578,9 @@ TemplateURL::~TemplateURL() {
 }
 
 std::wstring TemplateURL::AdjustedShortNameForLocaleDirection() const {
-  std::wstring bidi_safe_short_name;
-  if (base::i18n::AdjustStringForLocaleDirection(short_name_,
-                                                 &bidi_safe_short_name))
-    return bidi_safe_short_name;
-  return short_name_;
+  std::wstring bidi_safe_short_name = short_name_;
+  base::i18n::AdjustStringForLocaleDirection(&bidi_safe_short_name);
+  return bidi_safe_short_name;
 }
 
 void TemplateURL::SetSuggestionsURL(const std::string& suggestions_url,
diff --git a/chrome/browser/search_engines/template_url_table_model.cc b/chrome/browser/search_engines/template_url_table_model.cc
index 4570b25..f335ba3 100644
--- a/chrome/browser/search_engines/template_url_table_model.cc
+++ b/chrome/browser/search_engines/template_url_table_model.cc
@@ -187,8 +187,7 @@ std::wstring TemplateURLTableModel::GetText(int row, int col_id) {
       // TODO(xji): Consider adding a special case if the short name is a URL,
       // since those should always be displayed LTR. Please refer to
       // http://crbug.com/6726 for more information.
-      base::i18n::AdjustStringForLocaleDirection(url_short_name,
-                                                 &url_short_name);
+      base::i18n::AdjustStringForLocaleDirection(&url_short_name);
       return (template_url_model_->GetDefaultSearchProvider() == &url) ?
           l10n_util::GetStringF(IDS_SEARCH_ENGINES_EDITOR_DEFAULT_ENGINE,
                                 url_short_name) : url_short_name;
diff --git a/chrome/browser/task_manager/task_manager.cc b/chrome/browser/task_manager/task_manager.cc
index c6500fa..5673741 100644
--- a/chrome/browser/task_manager/task_manager.cc
+++ b/chrome/browser/task_manager/task_manager.cc
@@ -500,7 +500,7 @@ string16 TaskManagerModel::GetMemCellText(int64 number) const {
   string16 str = base::FormatNumber(number / 1024);
 
   // Adjust number string if necessary.
-  base::i18n::AdjustStringForLocaleDirection(str, &str);
+  base::i18n::AdjustStringForLocaleDirection(&str);
   return l10n_util::GetStringFUTF16(IDS_TASK_MANAGER_MEM_CELL_TEXT, str);
 #else
   // System expectation is to show "100 KB", "200 MB", etc.
diff --git a/chrome/browser/task_manager/task_manager_resource_providers.cc b/chrome/browser/task_manager/task_manager_resource_providers.cc
index 73476c9..2b339fa 100644
--- a/chrome/browser/task_manager/task_manager_resource_providers.cc
+++ b/chrome/browser/task_manager/task_manager_resource_providers.cc
@@ -149,7 +149,7 @@ std::wstring TaskManagerTabContentsResource::GetTitle() const {
     // as LTR format, the concatenated result will be "!Yahoo! Mail: The best
     // web-based Email :BAT", in which the capital letters "BAT" stands for
     // the Hebrew word for "tab".
-    base::i18n::AdjustStringForLocaleDirection(tab_title, &tab_title);
+    base::i18n::AdjustStringForLocaleDirection(&tab_title);
   }
 
   return l10n_util::GetStringF(IDS_TASK_MANAGER_TAB_PREFIX, tab_title);
@@ -357,8 +357,7 @@ TaskManagerBackgroundContentsResource::TaskManagerBackgroundContentsResource(
   }
   // Ensure that the string has the appropriate direction markers (see comment
   // in TaskManagerTabContentsResource::GetTitle()).
-  base::i18n::AdjustStringForLocaleDirection(application_name_,
-                                             &application_name_);
+  base::i18n::AdjustStringForLocaleDirection(&application_name_);
 }
 
 TaskManagerBackgroundContentsResource::~TaskManagerBackgroundContentsResource(
diff --git a/chrome/browser/ui/views/bookmark_bar_view.cc b/chrome/browser/ui/views/bookmark_bar_view.cc
index 439df1d..6226524 100644
--- a/chrome/browser/ui/views/bookmark_bar_view.cc
+++ b/chrome/browser/ui/views/bookmark_bar_view.cc
@@ -143,9 +143,8 @@ static std::wstring CreateToolTipForURLAndTitle(const gfx::Point& screen_loc,
 
   // First the title.
   if (!title.empty()) {
-    std::wstring localized_title;
-    if (!base::i18n::AdjustStringForLocaleDirection(title, &localized_title))
-      localized_title = title;
+    std::wstring localized_title = title;
+    base::i18n::AdjustStringForLocaleDirection(&localized_title);
     result.append(UTF16ToWideHack(gfx::ElideText(WideToUTF16Hack(
         localized_title), tt_font, max_width, false)));
   }
diff --git a/chrome/browser/ui/views/hung_renderer_view.cc b/chrome/browser/ui/views/hung_renderer_view.cc
index 3482380..b50debf 100644
--- a/chrome/browser/ui/views/hung_renderer_view.cc
+++ b/chrome/browser/ui/views/hung_renderer_view.cc
@@ -97,7 +97,7 @@ std::wstring HungPagesTableModel::GetText(int row, int column_id) {
   // TODO(xji): Consider adding a special case if the title text is a URL,
   // since those should always have LTR directionality. Please refer to
   // http://crbug.com/6726 for more information.
-  base::i18n::AdjustStringForLocaleDirection(title, &title);
+  base::i18n::AdjustStringForLocaleDirection(&title);
   return title;
 }
 
diff --git a/chrome/browser/ui/views/options/passwords_page_view.cc b/chrome/browser/ui/views/options/passwords_page_view.cc
index 6d22d5e..e33b0b6 100644
--- a/chrome/browser/ui/views/options/passwords_page_view.cc
+++ b/chrome/browser/ui/views/options/passwords_page_view.cc
@@ -82,7 +82,7 @@ std::wstring PasswordsTableModel::GetText(int row,
     }
     case IDS_PASSWORDS_PAGE_VIEW_USERNAME_COLUMN: {  // Username.
       std::wstring username = GetPasswordFormAt(row)->username_value;
-      base::i18n::AdjustStringForLocaleDirection(username, &username);
+      base::i18n::AdjustStringForLocaleDirection(&username);
       return username;
     }
     default: