i18n of several browser messages

chrome/browser/ui/views/critical_notification_bubble_view.cc

Index: chrome/browser/ui/views/critical_notification_bubble_view.cc
diff --git a/chrome/browser/ui/views/critical_notification_bubble_view.cc b/chrome/browser/ui/views/critical_notification_bubble_view.cc
index ded52e7bc5d936e5251b4921819a180f8901aa7b..d8d1913f0b5779bcb1e5e914e4e39ea82b329606 100644
--- a/chrome/browser/ui/views/critical_notification_bubble_view.cc
+++ b/chrome/browser/ui/views/critical_notification_bubble_view.cc
@@ -4,7 +4,7 @@
 
 #include "chrome/browser/ui/views/critical_notification_bubble_view.h"
 
-#include "base/strings/string_number_conversions.h"
+#include "base/i18n/message_formatter.h"

[Greg Levin, 2016/05/27 16:58:03]

TODO: Remove in next patch

[Greg Levin, 2016/05/27 19:31:47]

Done.

 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
@@ -83,13 +83,10 @@ void CriticalNotificationBubbleView::OnCountdown() {
 
 base::string16 CriticalNotificationBubbleView::GetWindowTitle() const {
   int seconds = GetRemainingTime();
-  return seconds > 0 ? l10n_util::GetStringFUTF16(
-                           IDS_CRITICAL_NOTIFICATION_HEADLINE,
-                           l10n_util::GetStringUTF16(IDS_PRODUCT_NAME),
-                           base::IntToString16(seconds))
-                     : l10n_util::GetStringFUTF16(
-                           IDS_CRITICAL_NOTIFICATION_HEADLINE_ALTERNATE,
-                           l10n_util::GetStringUTF16(IDS_PRODUCT_NAME));
+  return seconds > 0 ? l10n_util::GetPluralStringFUTF16(
+                           IDS_CRITICAL_NOTIFICATION_HEADLINE, seconds)
+                     : l10n_util::GetStringUTF16(
+                           IDS_CRITICAL_NOTIFICATION_HEADLINE_ALTERNATE);

[jungshik at Google, 2016/05/27 17:53:12]

You can use l10n_util::GetStringUTF16(IDS_PRODUCT_NAME) and there's no need to
keep two parallel strings in two grd files. 

Any reason to avoid using IDS_PRODUCT_NAME? perf? Want to use a shorter 'Chrome'
instead of 'Google Chrome' ?  

If you do that, instead of a convenience wrapper GetPluralStringFUTF16, you have
to use MessageFormatter directrly with two input parameters (one is for product
name and the other is seconds). That's what I suggested in my previous comment
(quoted again below):
Done.

[Peter Kasting, 2016/05/27 18:48:29]

I thought the l10n folks asked us to phase out substituting IDS_PRODUCT_NAME
into strings, and instead split things into separate strings in separate branded
vs. unbranded resource files, for better localizability.

In other words, I think Greg is doing the right thing.

[Greg Levin, 2016/05/27 19:31:47]

Acknowledged.

 }
 
 gfx::ImageSkia CriticalNotificationBubbleView::GetWindowIcon() {
@@ -139,8 +136,7 @@ void CriticalNotificationBubbleView::Init() {
   views::Label* message = new views::Label();
   message->SetMultiLine(true);
   message->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  message->SetText(l10n_util::GetStringFUTF16(IDS_CRITICAL_NOTIFICATION_TEXT,
-      l10n_util::GetStringUTF16(IDS_PRODUCT_NAME)));
+  message->SetText(l10n_util::GetStringUTF16(IDS_CRITICAL_NOTIFICATION_TEXT));

[jungshik at Google, 2016/05/27 17:53:12]

Any reason NOT to use a common message along with
l10n_util::GetStringUTF16(IDS_PRODUCT_NAME) ? 

[Peter Kasting, 2016/05/27 18:48:29]

Same comment.

[Greg Levin, 2016/05/27 19:31:47]

Acknowledged.

   message->SizeToFit(views::Widget::GetLocalizedContentsWidth(
       IDS_CRUCIAL_NOTIFICATION_BUBBLE_WIDTH_CHARS));
   AddChildView(message);