Extends DBusThreadManager to connect ibus-bus.

chrome/browser/chromeos/input_method/ibus_controller_impl.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/chromeos/input_method/ibus_controller_impl.h"
 
 #include <algorithm>  // for std::reverse.
 #include <cstdio>
 #include <cstring>  // for std::strcmp.
 #include <set>
 #include <sstream>
 #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
 #if defined(HAVE_IBUS)
 #include <ibus.h>
 #endif
 
 namespace {
 
 // Finds a property which has |new_prop.key| from |prop_list|, and replaces the
@@ skipping 319 matching lines @@
   std::stringstream stream;
   for (int i = 0;; ++i) {
     IBusProperty* prop = ibus_prop_list_get(prop_list, i);
     if (!prop)
       break;
     stream << PrintProp(prop, tree_level);
   }
   return stream.str();
 }
 
+class IBusAddressWatcher {
+ public:
+  class IBusAddressFileWatcherDelegate
+      : public base::files::FilePathWatcher::Delegate {
+   public:
+    IBusAddressFileWatcherDelegate(
+        const std::string& ibus_address,
+        IBusControllerImpl* controller,
+        IBusAddressWatcher* watcher)
+        : ibus_address_(ibus_address),
+          controller_(controller),
+          watcher_(watcher) {
+      DCHECK(watcher);
+      DCHECK(!ibus_address.empty());
+    }
+
+    virtual ~IBusAddressFileWatcherDelegate() {}
+
+    virtual void OnFilePathChanged(const FilePath& file_path) {
+      if (!watcher_->IsWatching())
+        return;
+      bool success = content::BrowserThread::PostTask(
+          content::BrowserThread::UI,
+          FROM_HERE,
+          base::Bind(
+              &IBusControllerImpl::IBusDaemonInitializationDone,
+              controller_,
+              ibus_address_));
+      DCHECK(success);
+      watcher_->StopSoon();
+    }
+
+   private:
+    // The ibus-daemon address.
+    const std::string ibus_address_;
+    IBusControllerImpl* controller_;
+    IBusAddressWatcher* watcher_;
+
+    DISALLOW_COPY_AND_ASSIGN(IBusAddressFileWatcherDelegate);
+  };
+
+  static void Start(const std::string& ibus_address,
+                    IBusControllerImpl* controller) {
+    IBusAddressWatcher* instance = IBusAddressWatcher::Get();
+    scoped_ptr<base::Environment> env(base::Environment::Create());
+    std::string address_file_path;
+    // TODO(nona): Moves reading environment variables to UI thread.
+    env->GetVar("IBUS_ADDRESS_FILE", &address_file_path);
+    DCHECK(!address_file_path.empty());
+
+    if (instance->IsWatching()) {

[Yusuke Sato, 2012/05/02 06:17:06]

remove { }

[Seigo Nonaka, 2012/05/02 18:09:13]

Done.

+      instance->StopNow();
+    }
+    instance->watcher_ = new base::files::FilePathWatcher;
+
+    // The |delegate| is owned by watcher.
+    IBusAddressFileWatcherDelegate* delegate =
+        new IBusAddressFileWatcherDelegate(ibus_address, controller, instance);
+    bool result = instance->watcher_->Watch(FilePath(address_file_path),
+                                            delegate);
+    DCHECK(result);
+  }
+
+  void StopNow() {
+    if (!watcher_)

[Yusuke Sato, 2012/05/02 06:17:06]

nit: remove 431-432. deleting NULL is safe.

[Seigo Nonaka, 2012/05/02 18:09:13]

Done.

+      return;
+    delete watcher_;
+    watcher_ = NULL;
+  }
+
+  void StopSoon() {
+    if (!watcher_)
+      return;
+    MessageLoop::current()->DeleteSoon(FROM_HERE, watcher_);
+    watcher_ = NULL;
+  }
+
+  bool IsWatching() {

[Yusuke Sato, 2012/05/02 06:17:06]

nit: const

[Seigo Nonaka, 2012/05/02 18:09:13]

Done.

+    return watcher_ != NULL;
+  }
+
+ private:
+  static IBusAddressWatcher* Get() {
+    static IBusAddressWatcher* instance = new IBusAddressWatcher;

[Yusuke Sato, 2012/05/02 06:17:06]

add a thread ID check like
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));

[Seigo Nonaka, 2012/05/02 18:09:13]

Done.

+    return instance;
+  }
+
+  // Singleton
+  IBusAddressWatcher()
+      :  watcher_(NULL) {}
+  base::files::FilePathWatcher* watcher_;
+  DISALLOW_COPY_AND_ASSIGN(IBusAddressWatcher);

[Yusuke Sato, 2012/05/02 06:17:06]

nit: space between 457 & 458

[Seigo Nonaka, 2012/05/02 18:09:13]

Done.

+};
+
 }  // namespace
 
 IBusControllerImpl::IBusControllerImpl()
     : ibus_(NULL),
       ibus_config_(NULL),
-      should_launch_daemon_(false),
-      process_handle_(base::kNullProcessHandle) {
+      process_handle_(base::kNullProcessHandle),
+      ibus_daemon_status_(IBUS_DAEMON_STOP),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
 }
 
 IBusControllerImpl::~IBusControllerImpl() {
   // Disconnect signals so the handler functions will not be called with
   // |this| which is already freed.
   if (ibus_) {
     g_signal_handlers_disconnect_by_func(
         ibus_,
         reinterpret_cast<gpointer>(G_CALLBACK(BusConnectedThunk)),
         this);
@@ skipping 21 matching lines @@
       g_signal_handlers_disconnect_by_func(
           ibus_panel_service,
           reinterpret_cast<gpointer>(G_CALLBACK(UpdatePropertyThunk)),
           this);
     }
   }
 }
 
 bool IBusControllerImpl::Start() {
   MaybeInitializeIBusBus();
-  should_launch_daemon_ = true;
   if (IBusConnectionsAreAlive())
     return true;
-  return MaybeLaunchIBusDaemon();
+  if (ibus_daemon_status_ == IBUS_DAEMON_STOP ||
+      ibus_daemon_status_ == IBUS_DAEMON_SHUTTING_DOWN) {
+    return StartIBusDaemon();
+  }
+  return true;
 }
 
 void IBusControllerImpl::Reset() {
   if (!IBusConnectionsAreAlive())
     return;
   IBusInputContext* context =
       GetInputContext(current_input_context_path_, ibus_);
   if (!context)
     return;
   ibus_input_context_reset(context);
 }
 
 bool IBusControllerImpl::Stop() {
+  if (ibus_daemon_status_ == IBUS_DAEMON_SHUTTING_DOWN ||
+      ibus_daemon_status_ == IBUS_DAEMON_STOP) {
+    // The daemon hasn't been started yet or now shutting down.
+    return true;
+  }
+
+  ibus_daemon_status_ = IBUS_DAEMON_SHUTTING_DOWN;
+  // TODO(nona): Shutdown ibus-bus connection.
   if (IBusConnectionsAreAlive()) {
     // Ask IBus to exit *asynchronously*.
     ibus_bus_exit_async(ibus_,
                         FALSE  /* do not restart */,
                         -1  /* timeout */,
                         NULL  /* cancellable */,
                         NULL  /* callback */,
                         NULL  /* user_data */);
     if (ibus_config_) {
       // Release |ibus_config_| unconditionally to make sure next
       // IBusConnectionsAreAlive() call will return false.
       g_object_unref(ibus_config_);
       ibus_config_ = NULL;
     }
-  } else if (process_handle_ != base::kNullProcessHandle) {
+  } else {
     base::KillProcess(process_handle_, -1, false /* wait */);
     DVLOG(1) << "Killing ibus-daemon. PID="
              << base::GetProcId(process_handle_);
-  } else {
-    // The daemon hasn't been started yet.
   }
-
   process_handle_ = base::kNullProcessHandle;
-  should_launch_daemon_ = false;
   return true;
 }
 
 bool IBusControllerImpl::ChangeInputMethod(const std::string& id) {
-  DCHECK(should_launch_daemon_);
+  // TODO(nona): use ibus_daemon_state_ after implement crosbug/26334

[Yusuke Sato, 2012/05/02 06:17:06]

??

looks like you're already checking |ibus_daemon_state_| in line 563-566.

[Seigo Nonaka, 2012/05/02 18:09:13]

Oops this comment is obsolete.
Removed.

On 2012/05/02 06:17:06, Yusuke Sato wrote:

+  if (ibus_daemon_status_ == IBUS_DAEMON_SHUTTING_DOWN ||
+      ibus_daemon_status_ == IBUS_DAEMON_STOP) {
+    return false;
+  }
 
   // Sanity checks.
   DCHECK(!InputMethodUtil::IsKeyboardLayout(id));
   if (!whitelist_.InputMethodIdIsWhitelisted(id) &&
       !InputMethodUtil::IsExtensionInputMethod(id))
     return false;
 
   // Clear input method properties unconditionally if |id| is not equal to
   // |current_input_method_id_|.
   //
@@ skipping 101 matching lines @@
   IBusInputContext* context =
       GetInputContext(current_input_context_path_, ibus_);
   if (!context)
     return;
   ibus_input_context_cancel_hand_writing(context, n_strokes);
   g_object_unref(context);
 }
 #endif
 
 bool IBusControllerImpl::IBusConnectionsAreAlive() {
-  return (process_handle_ != base::kNullProcessHandle) &&
+  return (ibus_daemon_status_ == IBUS_DAEMON_RUNNING) &&
       ibus_ && ibus_bus_is_connected(ibus_) && ibus_config_;
 }
 
 void IBusControllerImpl::MaybeRestoreConnections() {
   if (IBusConnectionsAreAlive())
     return;
   MaybeRestoreIBusConfig();
   if (IBusConnectionsAreAlive()) {
     DVLOG(1) << "ibus-daemon and ibus-memconf processes are ready.";
     ConnectPanelServiceSignals();
@@ skipping 299 matching lines @@
 
   // Notify the change.
   if (!prop_list.empty()) {
     for (size_t i = 0; i < prop_list.size(); ++i) {
       FindAndUpdateProperty(prop_list[i], &current_property_list_);
     }
     FOR_EACH_OBSERVER(Observer, observers_, PropertyChanged());
   }
 }
 
-bool IBusControllerImpl::MaybeLaunchIBusDaemon() {
-  static const char kIBusDaemonPath[] = "/usr/bin/ibus-daemon";
-
-  if (process_handle_ != base::kNullProcessHandle) {
-    DVLOG(1) << "MaybeLaunchIBusDaemon: ibus-daemon is already running.";
+bool IBusControllerImpl::StartIBusDaemon() {
+  if (ibus_daemon_status_ == IBUS_DAEMON_INITIALIZING ||
+      ibus_daemon_status_ == IBUS_DAEMON_RUNNING) {
     return false;

[Yusuke Sato, 2012/05/02 06:17:06]

keep the DVLOG(1) for now.

[Seigo Nonaka, 2012/05/02 18:09:13]

Done.

   }
-  if (!should_launch_daemon_)
-    return false;
 
+  ibus_daemon_status_ = IBUS_DAEMON_INITIALIZING;
+  const std::string ibus_address = base::StringPrintf(
+      "unix:abstract=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.
+  bool success = content::BrowserThread::PostTaskAndReply(
+      content::BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&IBusAddressWatcher::Start,
+                 ibus_address,
+                 base::Unretained(this)),
+      base::Bind(&IBusControllerImpl::LaunchIBusDaemon,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 ibus_address));
+  DCHECK(success);
+  return true;
+}
+
+void IBusControllerImpl::LaunchIBusDaemon(const std::string& ibus_address) {

[Yusuke Sato, 2012/05/02 06:17:06]

update ibus_daemon_address_ in this function.

[Seigo Nonaka, 2012/05/02 18:09:13]

Done.

+  DCHECK_EQ(base::kNullProcessHandle, process_handle_);
+  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,
                                        base::ProcessHandle* process_handle,
                                        GChildWatchFunc watch_func) {
   std::vector<std::string> argv;
   base::ProcessHandle handle = base::kNullProcessHandle;
 
   base::SplitString(command_line, ' ', &argv);
 
   if (!base::LaunchProcess(argv, base::LaunchOptions(), &handle)) {
     DVLOG(1) << "Could not launch: " << command_line;
     return false;
   }
 
   // g_child_watch_add is necessary to prevent the process from becoming a
   // zombie.
   // TODO(yusukes): port g_child_watch_add to base/process_utils_posix.cc.
   const base::ProcessId pid = base::GetProcId(handle);
   g_child_watch_add(pid, watch_func, this);
 
   *process_handle = handle;
   DVLOG(1) << command_line << "is started. PID=" << pid;
   return true;
 }
 
 // static
+void IBusControllerImpl::IBusDaemonInitializationDone(
+    IBusControllerImpl* controller,
+    const std::string& ibus_address) {
+  if (controller->ibus_daemon_address_ != ibus_address)
+    return;
+
+  if (controller->ibus_daemon_status_ != IBUS_DAEMON_INITIALIZING) {
+    // Corresponding to killing ibus-daemon under initializing, so do nothing.
+    return;
+  }
+  chromeos::DBusThreadManager::Get()->MaybeResetIBusBus(ibus_address);
+  controller->ibus_daemon_status_ = IBUS_DAEMON_RUNNING;
+}
+
+// static
 void IBusControllerImpl::SetInputMethodConfigCallback(GObject* source_object,
                                                       GAsyncResult* res,
                                                       gpointer user_data) {
   IBusConfig* config = IBUS_CONFIG(user_data);
   g_return_if_fail(config);
 
   GError* error = NULL;
   const gboolean result =
       ibus_config_set_value_async_finish(config, res, &error);
 
   if (!result) {
     std::string message = "(unknown error)";
     if (error && error->message) {
       message = error->message;
     }
     DVLOG(1) << "ibus_config_set_value_async failed: " << message;
   }
 
   if (error)
     g_error_free(error);
   g_object_unref(config);
 }
 
 // static
 void IBusControllerImpl::OnIBusDaemonExit(GPid pid,
                                           gint status,
                                           IBusControllerImpl* controller) {
-  if (controller->process_handle_ != base::kNullProcessHandle &&
-      base::GetProcId(controller->process_handle_) == pid) {
-    controller->process_handle_ = base::kNullProcessHandle;
+  const IBusDaemonStatus on_exit_state = controller->ibus_daemon_status_;

[Yusuke Sato, 2012/05/02 06:17:06]

nit: move this down to 1135.

[Seigo Nonaka, 2012/05/02 18:09:13]

Done.

+
+  if (controller->process_handle_ != base::kNullProcessHandle) {
+    if (base::GetProcId(controller->process_handle_) == pid) {
+      // Corresponding to shutting down ibus-daemon by exit message or crashing
+      // ibus-daemon. So reset handler and try re-launch if necessary.
+      controller->process_handle_ = base::kNullProcessHandle;
+    } else {
+      // This condition is corresponding as follows.
+      // 1. Called Stop (process_handle_ become null)
+      // 2. Called LaunchProcess (process_handle_ become new instance

[Yusuke Sato, 2012/05/02 06:17:06]

)

[Seigo Nonaka, 2012/05/02 18:09:13]

Done.

+      // 3. Callbacked OnIBusDaemonExit for old instance and reach here.
+      // In this case, we should not reset process_handle_ as null, and do not
+      // re-launch ibus-daemon.
+      // TODO(nona): Shutdown ibus-bus connection.
+      return;
+    }
   }
-  // Restart the daemon if needed.
-  controller->MaybeLaunchIBusDaemon();
+
+  controller->ibus_daemon_status_ = IBUS_DAEMON_STOP;
+  if (on_exit_state == IBUS_DAEMON_SHUTTING_DOWN) {
+    // Normal exitting, so do nothing.
+    return;
+  }
+
+  LOG(ERROR) << "The ibus-daemon is crashed. Re-launching...";

[Yusuke Sato, 2012/05/02 06:17:06]

s/is//

[Seigo Nonaka, 2012/05/02 18:09:13]

Done.

+  controller->StartIBusDaemon();
 }
 #endif  // defined(HAVE_IBUS)
 
 // static
 bool IBusControllerImpl::FindAndUpdatePropertyForTesting(
     const chromeos::input_method::InputMethodProperty& new_prop,
     chromeos::input_method::InputMethodPropertyList* prop_list) {
   return FindAndUpdateProperty(new_prop, prop_list);
 }
 
 }  // namespace input_method
 }  // namespace chromeos