i18n of several browser messages

chrome/browser/ui/views/critical_notification_bubble_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 #include "chrome/browser/upgrade_detector.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/locale_settings.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/user_metrics.h"
@@ skipping 58 matching lines @@
 
   // Update the counter. It may seem counter-intuitive to update the message
   // after we attempt restart, but remember that shutdown may be aborted by
   // an onbeforeunload handler, leaving the bubble up when the browser should
   // have restarted (giving the user another chance).
   GetBubbleFrameView()->UpdateWindowTitle();
 }
 
 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() {
   return *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
       IDR_UPDATE_MENU_SEVERITY_HIGH);
 }
 
 bool CriticalNotificationBubbleView::ShouldShowWindowIcon() const {
   return true;
 }
@@ skipping 29 matching lines @@
 }
 
 void CriticalNotificationBubbleView::Init() {
   bubble_created_ = base::TimeTicks::Now();
 
   SetLayoutManager(new views::FillLayout());
 
   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);
 
   refresh_timer_.Start(FROM_HERE,
       base::TimeDelta::FromMilliseconds(kRefreshBubbleEvery),
       this, &CriticalNotificationBubbleView::OnCountdown);
 
   content::RecordAction(UserMetricsAction("CriticalNotificationShown"));
 }
 
 void CriticalNotificationBubbleView::GetAccessibleState(
     ui::AXViewState* state) {
   state->role = ui::AX_ROLE_ALERT;
 }
 
 void CriticalNotificationBubbleView::ViewHierarchyChanged(
     const ViewHierarchyChangedDetails& details) {
   if (details.is_add && details.child == this)
     NotifyAccessibilityEvent(ui::AX_EVENT_ALERT, true);
 }