Extends DBusThreadManager to connect ibus-bus.

chrome/browser/chromeos/input_method/ibus_controller_impl.cc

Index: chrome/browser/chromeos/input_method/ibus_controller_impl.cc
diff --git a/chrome/browser/chromeos/input_method/ibus_controller_impl.cc b/chrome/browser/chromeos/input_method/ibus_controller_impl.cc
index 10a354a9e7c5bb725a9d1c61288aaa4b0ce4fbb1..29d4c0bd2589546656d6bc6c2ece20b8ed359bd0 100644
--- a/chrome/browser/chromeos/input_method/ibus_controller_impl.cc
+++ b/chrome/browser/chromeos/input_method/ibus_controller_impl.cc
@@ -12,12 +12,19 @@
 #include <stack>
 #include <utility>
 
+#include "base/bind.h"
+#include "base/environment.h"
+#include "base/files/file_path_watcher.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "base/rand_util.h"
 #include "base/stringprintf.h"
 #include "base/string_split.h"
 #include "chrome/browser/chromeos/input_method/input_method_config.h"
 #include "chrome/browser/chromeos/input_method/input_method_property.h"
 #include "chrome/browser/chromeos/input_method/input_method_util.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "content/public/browser/browser_thread.h"
 
 // TODO(nona): Remove libibus dependency from this file. Then, write unit tests
 // for all functions in this file. crbug.com/26334
@@ -357,13 +364,60 @@ std::string PrintPropList(IBusPropList *prop_list, int tree_level) {
   return stream.str();
 }
 
+// The implementation of FilePathWatcher::Delegate, used in
+// StartAddressFileWatch function.
+class IBusAddressFileWatcherDelegate
+    : public base::files::FilePathWatcher::Delegate {
+ public:
+  IBusAddressFileWatcherDelegate(const std::string& ibus_address,
+                                 base::files::FilePathWatcher* watcher)
+      : ibus_address_(ibus_address),
+        watcher_(watcher) {
+  }
+
+  virtual ~IBusAddressFileWatcherDelegate() {}
+
+  virtual void OnFilePathChanged(const FilePath& file_path) {
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&chromeos::DBusThreadManager::MaybeResetIBusBus,
+                   base::Unretained(chromeos::DBusThreadManager::Get()),
+                   ibus_address_));
+    MessageLoop::current()->DeleteSoon(FROM_HERE, watcher_);
+  }
+
+ private:
+

[Yusuke Sato, 2012/04/26 00:46:28]

nit: remove

[Seigo Nonaka, 2012/04/26 17:40:59]

Done.

+  // The ibus-daemon address.
+  std::string ibus_address_;

[Yusuke Sato, 2012/04/26 00:46:28]

nit: const std::string would be better.

[Seigo Nonaka, 2012/04/26 17:40:59]

Done.

+
+  base::files::FilePathWatcher* watcher_;

[Yusuke Sato, 2012/04/26 00:46:28]

nit: vertical space after this line.

[Seigo Nonaka, 2012/04/26 17:40:59]

Done.

+  DISALLOW_COPY_AND_ASSIGN(IBusAddressFileWatcherDelegate);
+};
+
+void StartAddressFileWatch(const std::string& ibus_address) {
+  scoped_ptr<base::Environment> env(base::Environment::Create());
+  std::string address_file_path;
+  env->GetVar("IBUS_ADDRESS_FILE", &address_file_path);
+
+  // The |watcher| instance will be released by |delegate|.
+  base::files::FilePathWatcher* watcher = new base::files::FilePathWatcher();

[Yusuke Sato, 2012/04/26 00:46:28]

nit: you can omit '()'.

[Seigo Nonaka, 2012/04/26 17:40:59]

Done.

+
+  // The |delegate| is owned by watcher.
+  IBusAddressFileWatcherDelegate* delegate = new IBusAddressFileWatcherDelegate(
+      ibus_address, watcher);
+  bool result = watcher->Watch(FilePath(address_file_path), delegate);
+  DCHECK(result);
+}
 }  // namespace
 
 IBusControllerImpl::IBusControllerImpl()
     : ibus_(NULL),
       ibus_config_(NULL),
       should_launch_daemon_(false),
-      process_handle_(base::kNullProcessHandle) {
+      process_handle_(base::kNullProcessHandle),
+      weak_ptr_factory_(this) {

[Yusuke Sato, 2012/04/26 00:46:28]

ALLOW_THIS_IN_INITIALIZER_LIST?

[Seigo Nonaka, 2012/04/26 17:40:59]

Done.

 }
 
 IBusControllerImpl::~IBusControllerImpl() {
@@ -408,6 +462,7 @@ bool IBusControllerImpl::Start() {
   should_launch_daemon_ = true;
   if (IBusConnectionsAreAlive())
     return true;
+

[Yusuke Sato, 2012/04/26 00:46:28]

nit: remove

[Seigo Nonaka, 2012/04/26 17:40:59]

Done.

   return MaybeLaunchIBusDaemon();
 }
 
@@ -893,8 +948,6 @@ void IBusControllerImpl::UpdateProperty(IBusPanelService* panel,
 }
 
 bool IBusControllerImpl::MaybeLaunchIBusDaemon() {
-  static const char kIBusDaemonPath[] = "/usr/bin/ibus-daemon";
-
   if (process_handle_ != base::kNullProcessHandle) {
     DVLOG(1) << "MaybeLaunchIBusDaemon: ibus-daemon is already running.";
     return false;
@@ -902,17 +955,35 @@ bool IBusControllerImpl::MaybeLaunchIBusDaemon() {
   if (!should_launch_daemon_)
     return false;
 
+  std::string ibus_address = base::StringPrintf(

[Yusuke Sato, 2012/04/26 00:46:28]

looks like this function still has a race condition. what happens if the
function is called twice or more quickly? e.g.

1. login as guest. at this point, xkb:us::eng is the only ime enabled.
2. enable mozc. this function is called.
3. before LaunchIBusDaemon is called, enable mozc-jp. this function is called
again.

[Yusuke Sato, 2012/04/26 00:46:28]

const

[Seigo Nonaka, 2012/04/26 17:40:59]

Done.

[Seigo Nonaka, 2012/04/26 17:40:59]

I introduce is_initializing_ibus_daemon_ to avoid double initialization.
On 2012/04/26 00:46:28, Yusuke Sato wrote:

+      "unix:abstract=/tmp/ibus-%0lX", base::RandUint64());
+
+  // Set up ibus-daemon address file watcher before launching ibus-daemon,
+  // because if watcher start after ibus-daemon, we may miss the ibus connection
+  // initialization.
+  content::BrowserThread::PostTaskAndReply(
+      content::BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&StartAddressFileWatch, ibus_address),
+      base::Bind(&IBusControllerImpl::LaunchIBusDaemon,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 ibus_address));
+  return true;
+}
+
+void IBusControllerImpl::LaunchIBusDaemon(const std::string& ibus_address) {
+  static const char kIBusDaemonPath[] = "/usr/bin/ibus-daemon";
   // TODO(zork): Send output to /var/log/ibus.log
   const std::string ibus_daemon_command_line =
-      base::StringPrintf("%s --panel=disable --cache=none --restart --replace",
-                         kIBusDaemonPath);
+      base::StringPrintf("%s --panel=disable --cache=none --restart --replace"
+                         " --address=%s",
+                         kIBusDaemonPath,
+                         ibus_address.c_str());
   if (!LaunchProcess(ibus_daemon_command_line,
                      &process_handle_,
                      reinterpret_cast<GChildWatchFunc>(OnIBusDaemonExit))) {
     DVLOG(1) << "Failed to launch " << ibus_daemon_command_line;
-    return false;
   }
-  return true;
 }
 
 bool IBusControllerImpl::LaunchProcess(const std::string& command_line,