Make ChromotingHost::ui_strings() immutable. Use string16 for localized strings.

There were two issues:
 - ui_strings() were mutable which creates possibility for race
   condition. Made it immutable, and added SetUiString() which can be
   called only before session is started.
 - strings were loaded after host is started which sometimes leads to
   non-localized string being shown. Particularly I often saw empty
   text on Disconnect button.

BUG=None
TEST=Disconnect button text is always localized.

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

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

9 files changed, 79 insertions, 69 deletions

diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc
index 98a2a6c..dd8299d 100644
--- a/remoting/host/chromoting_host.cc
+++ b/remoting/host/chromoting_host.cc
@@ -361,6 +361,13 @@ void ChromotingHost::PauseSession(bool pause) {
   desktop_environment_->OnPause(!pause);
 }
 
+void ChromotingHost::SetUiStrings(const UiStrings& ui_strings) {
+  DCHECK_EQ(context_->main_message_loop(), MessageLoop::current());
+  DCHECK_EQ(state_, kInitial);
+
+  ui_strings_ = ui_strings;
+}
+
 void ChromotingHost::OnClientDisconnected(ConnectionToClient* connection) {
   DCHECK_EQ(context_->main_message_loop(), MessageLoop::current());
 
diff --git a/remoting/host/chromoting_host.h b/remoting/host/chromoting_host.h
index 0b8f296..fc35798 100644
--- a/remoting/host/chromoting_host.h
+++ b/remoting/host/chromoting_host.h
@@ -145,7 +145,10 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
   // is ignored.
   void PauseSession(bool pause);
 
-  UiStrings* ui_strings() { return &ui_strings_; }
+  const UiStrings& ui_strings() { return ui_strings_; }
+
+  // Set localized strings. Must be called before host is started.
+  void SetUiStrings(const UiStrings& ui_strings);
 
  private:
   friend class base::RefCountedThreadSafe<ChromotingHost>;
diff --git a/remoting/host/continue_window_linux.cc b/remoting/host/continue_window_linux.cc
index 3933f21..431144d 100644
--- a/remoting/host/continue_window_linux.cc
+++ b/remoting/host/continue_window_linux.cc
@@ -8,6 +8,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/utf_string_conversions.h"
 #include "remoting/host/chromoting_host.h"
 #include "remoting/host/ui_strings.h"
 #include "ui/base/gtk/gtk_signal.h"
@@ -25,7 +26,7 @@ class ContinueWindowLinux : public remoting::ContinueWindow {
  private:
   CHROMEGTK_CALLBACK_1(ContinueWindowLinux, void, OnResponse, int);
 
-  void CreateWindow(UiStrings* ui_strings);
+  void CreateWindow(const UiStrings& ui_strings);
 
   ChromotingHost* host_;
   GtkWidget* continue_window_;
@@ -41,15 +42,17 @@ ContinueWindowLinux::ContinueWindowLinux()
 ContinueWindowLinux::~ContinueWindowLinux() {
 }
 
-void ContinueWindowLinux::CreateWindow(UiStrings* ui_strings) {
+void ContinueWindowLinux::CreateWindow(const UiStrings& ui_strings) {
   if (continue_window_) return;
 
   continue_window_ = gtk_dialog_new_with_buttons(
-      ui_strings->product_name.c_str(),
+      UTF16ToUTF8(ui_strings.product_name).c_str(),
       NULL,
       static_cast<GtkDialogFlags>(GTK_DIALOG_MODAL | GTK_DIALOG_NO_SEPARATOR),
-      ui_strings->stop_sharing_button_text.c_str(), GTK_RESPONSE_CANCEL,
-      ui_strings->continue_button_text.c_str(), GTK_RESPONSE_OK,
+      UTF16ToUTF8(ui_strings.stop_sharing_button_text).c_str(),
+      GTK_RESPONSE_CANCEL,
+      UTF16ToUTF8(ui_strings.continue_button_text).c_str(),
+      GTK_RESPONSE_OK,
       NULL);
 
   gtk_dialog_set_default_response(GTK_DIALOG(continue_window_),
@@ -66,7 +69,8 @@ void ContinueWindowLinux::CreateWindow(UiStrings* ui_strings) {
   GtkWidget* content_area =
       gtk_dialog_get_content_area(GTK_DIALOG(continue_window_));
 
-  GtkWidget* text_label = gtk_label_new(ui_strings->continue_prompt.c_str());
+  GtkWidget* text_label =
+      gtk_label_new(UTF16ToUTF8(ui_strings.continue_prompt).c_str());
   gtk_label_set_line_wrap(GTK_LABEL(text_label), TRUE);
   // TODO(lambroslambrou): Fix magic numbers, as in disconnect_window_linux.cc.
   gtk_misc_set_padding(GTK_MISC(text_label), 12, 12);
diff --git a/remoting/host/disconnect_window.h b/remoting/host/disconnect_window.h
index 2694512..6ceb671 100644
--- a/remoting/host/disconnect_window.h
+++ b/remoting/host/disconnect_window.h
@@ -24,9 +24,6 @@ class DisconnectWindow {
   static const char kDisconnectKeysWin[];
 
   // Show the disconnect window allowing the user to shut down |host|.
-  // TODO(jamiewalch): Once all platforms are using localized strings,
-  // remove |username| from this signature (it will be substituted as
-  // appropriate at the localization stage).
   virtual void Show(ChromotingHost* host, const std::string& username) = 0;
 
   // Hide the disconnect window.
diff --git a/remoting/host/disconnect_window_linux.cc b/remoting/host/disconnect_window_linux.cc
index ad2707d..b2dd6f5 100644
--- a/remoting/host/disconnect_window_linux.cc
+++ b/remoting/host/disconnect_window_linux.cc
@@ -8,6 +8,8 @@
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/string_util.h"
+#include "base/utf_string_conversions.h"
 #include "remoting/host/chromoting_host.h"
 #include "remoting/host/ui_strings.h"
 #include "ui/base/gtk/gtk_signal.h"
@@ -36,7 +38,7 @@ class DisconnectWindowLinux : public DisconnectWindow {
  private:
   CHROMEGTK_CALLBACK_1(DisconnectWindowLinux, void, OnResponse, int);
 
-  void CreateWindow(UiStrings* ui_strings);
+  void CreateWindow(const UiStrings& ui_strings);
 
   ChromotingHost* host_;
   GtkWidget* disconnect_window_;
@@ -53,14 +55,15 @@ DisconnectWindowLinux::DisconnectWindowLinux()
 DisconnectWindowLinux::~DisconnectWindowLinux() {
 }
 
-void DisconnectWindowLinux::CreateWindow(UiStrings* ui_strings) {
+void DisconnectWindowLinux::CreateWindow(const UiStrings& ui_strings) {
   if (disconnect_window_) return;
 
   disconnect_window_ = gtk_dialog_new_with_buttons(
-      ui_strings->product_name.c_str(),
+      UTF16ToUTF8(ui_strings.product_name).c_str(),
       NULL,
       GTK_DIALOG_NO_SEPARATOR,
-      ui_strings->disconnect_button_text_plus_shortcut.c_str(), GTK_RESPONSE_OK,
+      UTF16ToUTF8(ui_strings.disconnect_button_text_plus_shortcut).c_str(),
+      GTK_RESPONSE_OK,
       NULL);
 
   GtkWindow* window = GTK_WINDOW(disconnect_window_);
@@ -94,11 +97,13 @@ void DisconnectWindowLinux::CreateWindow(UiStrings* ui_strings) {
 }
 
 void DisconnectWindowLinux::Show(ChromotingHost* host,
-                                 const std::string& /* unused */) {
+                                 const std::string& username) {
   host_ = host;
   CreateWindow(host->ui_strings());
-  gtk_label_set_text(GTK_LABEL(message_),
-                     host->ui_strings()->disconnect_message.c_str());
+
+  string16 text = ReplaceStringPlaceholders(
+      host->ui_strings().disconnect_message, UTF8ToUTF16(username), NULL);
+  gtk_label_set_text(GTK_LABEL(message_), UTF16ToUTF8(text).c_str());
   gtk_window_present(GTK_WINDOW(disconnect_window_));
 }
 
diff --git a/remoting/host/plugin/host_script_object.cc b/remoting/host/plugin/host_script_object.cc
index 0fcb082..3553e2e 100644
--- a/remoting/host/plugin/host_script_object.cc
+++ b/remoting/host/plugin/host_script_object.cc
@@ -7,7 +7,9 @@
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "base/message_loop_proxy.h"
+#include "base/sys_string_conversions.h"
 #include "base/threading/platform_thread.h"
+#include "base/utf_string_conversions.h"
 #include "remoting/base/auth_token_util.h"
 #include "remoting/base/util.h"
 #include "remoting/host/chromoting_host.h"
@@ -350,7 +352,6 @@ void HostNPScriptObject::OnClientAuthenticated(
   if (pos != std::string::npos)
     client_username_.replace(pos, std::string::npos, "");
   LOG(INFO) << "Client " << client_username_ << " connected.";
-  LocalizeStrings();
   OnStateChanged(kConnected);
 }
 
@@ -482,6 +483,8 @@ void HostNPScriptObject::FinishConnect(
   host_->AddStatusObserver(register_request_.get());
   host_->set_it2me(true);
 
+  LocalizeStrings();
+
   // Start the Host.
   host_->Start();
 
@@ -642,15 +645,13 @@ void HostNPScriptObject::SetException(const std::string& exception_string) {
 }
 
 void HostNPScriptObject::LocalizeStrings() {
-  UiStrings* ui_strings = host_->ui_strings();
-  std::string direction;
-  LocalizeString("@@bidi_dir", NULL, &direction);
-  ui_strings->direction =
-      direction == "rtl" ? remoting::UiStrings::RTL
-                         : remoting::UiStrings::LTR;
-  LocalizeString("PRODUCT_NAME", NULL, &ui_strings->product_name);
-  LocalizeString("DISCONNECT_BUTTON", NULL,
-                 &ui_strings->disconnect_button_text);
+  UiStrings ui_strings;
+  string16 direction;
+  LocalizeString("@@bidi_dir", &direction);
+  ui_strings.direction = UTF16ToUTF8(direction) == "rtl" ?
+      remoting::UiStrings::RTL : remoting::UiStrings::LTR;
+  LocalizeString("PRODUCT_NAME", &ui_strings.product_name);
+  LocalizeString("DISCONNECT_BUTTON", &ui_strings.disconnect_button_text);
   LocalizeString(
 #if defined(OS_WIN)
       "DISCONNECT_BUTTON_PLUS_SHORTCUT_WINDOWS",
@@ -659,28 +660,22 @@ void HostNPScriptObject::LocalizeStrings() {
 #else
       "DISCONNECT_BUTTON_PLUS_SHORTCUT_LINUX",
 #endif
-      NULL, &ui_strings->disconnect_button_text_plus_shortcut);
-  LocalizeString("CONTINUE_PROMPT", NULL, &ui_strings->continue_prompt);
-  LocalizeString("CONTINUE_BUTTON", NULL, &ui_strings->continue_button_text);
-  LocalizeString("STOP_SHARING_BUTTON", NULL,
-                 &ui_strings->stop_sharing_button_text);
-  LocalizeString("MESSAGE_SHARED", client_username_.c_str(),
-                 &ui_strings->disconnect_message);
-}
-
-bool HostNPScriptObject::LocalizeString(const char* tag,
-                                        const char* substitution,
-                                        std::string* result) {
+      &ui_strings.disconnect_button_text_plus_shortcut);
+  LocalizeString("CONTINUE_PROMPT", &ui_strings.continue_prompt);
+  LocalizeString("CONTINUE_BUTTON", &ui_strings.continue_button_text);
+  LocalizeString("STOP_SHARING_BUTTON",
+                 &ui_strings.stop_sharing_button_text);
+  LocalizeString("MESSAGE_SHARED", &ui_strings.disconnect_message);
+
+  host_->SetUiStrings(ui_strings);
+}
+
+bool HostNPScriptObject::LocalizeString(const char* tag, string16* result) {
   NPVariant args[2];
   STRINGZ_TO_NPVARIANT(tag, args[0]);
-  int arg_count = 1;
-  if (substitution) {
-    STRINGZ_TO_NPVARIANT(substitution, args[1]);
-    ++arg_count;
-  }
   NPVariant np_result;
   bool is_good = g_npnetscape_funcs->invokeDefault(
-      plugin_, localize_func_.get(), &args[0], arg_count, &np_result);
+      plugin_, localize_func_.get(), &args[0], 1, &np_result);
   if (!is_good) {
     LOG(ERROR) << "Localization failed for " << tag;
     return false;
@@ -691,7 +686,7 @@ bool HostNPScriptObject::LocalizeString(const char* tag,
     LOG(ERROR) << "Missing translation for " << tag;
     return false;
   }
-  *result = translation;
+  *result = UTF8ToUTF16(translation);
   return true;
 }
 
diff --git a/remoting/host/plugin/host_script_object.h b/remoting/host/plugin/host_script_object.h
index 04a35b4..9172ada9 100644
--- a/remoting/host/plugin/host_script_object.h
+++ b/remoting/host/plugin/host_script_object.h
@@ -13,6 +13,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/synchronization/cancellation_flag.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/string16.h"
 #include "base/threading/platform_thread.h"
 #include "base/time.h"
 #include "remoting/base/plugin_message_loop_proxy.h"
@@ -135,8 +136,7 @@ class HostNPScriptObject : public HostStatusObserver {
   // a string->string mapping with one optional substitution parameter. Stores
   // the translation in |result| and returns true on success, or leaves it
   // unchanged and returns false on failure.
-  bool LocalizeString(const char* tag, const char* substitution,
-                      std::string* result);
+  bool LocalizeString(const char* tag, string16* result);
 
   // Helper function for executing InvokeDefault on an NPObject, and ignoring
   // the return value.
diff --git a/remoting/host/ui_strings.cc b/remoting/host/ui_strings.cc
index 760643f..99c59b3 100644
--- a/remoting/host/ui_strings.cc
+++ b/remoting/host/ui_strings.cc
@@ -4,24 +4,26 @@
 
 #include "remoting/host/ui_strings.h"
 
+#include "base/utf_string_conversions.h"
+
 namespace remoting {
 
 UiStrings::UiStrings() :
     direction(LTR),
-    product_name("Chromoting"),
-    // The default string doesn't include the user name, so this will be
-    // missing for remoting_simple_host
-    disconnect_message("Your desktop is currently being shared."),
-    disconnect_button_text("Disconnect"),
+    product_name(ASCIIToUTF16("Chromoting")),
+    disconnect_message(
+        ASCIIToUTF16("Your desktop is currently being shared with $1.")),
+    disconnect_button_text(ASCIIToUTF16("Disconnect")),
     // This is the wrong shortcut on Mac OS X, but remoting_simple_host
     // doesn't have a bundle (and hence no dialog) on that platform and
     // the web-app will provide the correct localization for this string.
-    disconnect_button_shortcut("Ctrl+Alt+Esc"),
-    continue_prompt(
+    disconnect_button_text_plus_shortcut(
+        ASCIIToUTF16("Disconnect (Ctrl+Alt+Esc)")),
+    continue_prompt(ASCIIToUTF16(
         "You are currently sharing this machine with another user. "
-        "Please confirm that you want to continue sharing."),
-    continue_button_text("Continue"),
-    stop_sharing_button_text("Stop Sharing") {
+        "Please confirm that you want to continue sharing.")),
+    continue_button_text(ASCIIToUTF16("Continue")),
+    stop_sharing_button_text(ASCIIToUTF16("Stop Sharing")) {
 }
 
 UiStrings::~UiStrings() { }
diff --git a/remoting/host/ui_strings.h b/remoting/host/ui_strings.h
index c06140f..bdd0c4a 100644
--- a/remoting/host/ui_strings.h
+++ b/remoting/host/ui_strings.h
@@ -5,7 +5,7 @@
 #ifndef REMOTING_HOST_UI_STRINGS_H_
 #define REMOTING_HOST_UI_STRINGS_H_
 
-#include <string>
+#include "base/string16.h"
 
 // This struct contains localized strings to be displayed in host dialogs.
 // For the web-app, these are loaded from the appropriate messages.json
@@ -27,28 +27,25 @@ struct UiStrings {
   Direction direction;
 
   // The product name (Chromoting or Chrome Remote Desktop).
-  std::string product_name;
+  string16 product_name;
 
   // The message in the disconnect dialog.
-  std::string disconnect_message;
+  string16 disconnect_message;
 
   // The label on the disconnect dialog button, without the keyboard shortcut.
-  std::string disconnect_button_text;
+  string16 disconnect_button_text;
 
   // The label on the disconnect dialog button, with the keyboard shortcut.
-  std::string disconnect_button_text_plus_shortcut;
-
-  // The keyboard shortcut for disconnecting clients.
-  std::string disconnect_button_shortcut;
+  string16 disconnect_button_text_plus_shortcut;
 
   // The confirmation prompt displayed by the continue window.
-  std::string continue_prompt;
+  string16 continue_prompt;
 
   // The label on the 'Continue' button of the continue window.
-  std::string continue_button_text;
+  string16 continue_button_text;
 
   // The label on the 'Stop Sharing' button of the continue window.
-  std::string stop_sharing_button_text;
+  string16 stop_sharing_button_text;
 };
 
 }