Makes IBusConfigClient initialize asynchronous.

chromeos/dbus/ibus/ibus_config_client.cc

Index: chromeos/dbus/ibus/ibus_config_client.cc
diff --git a/chromeos/dbus/ibus/ibus_config_client.cc b/chromeos/dbus/ibus/ibus_config_client.cc
index 659cde41bae779420cb748a81ec0c4286f16c0e9..2c67c368440c1e1db5cef39297f4e80cb68eae45 100644
--- a/chromeos/dbus/ibus/ibus_config_client.cc
+++ b/chromeos/dbus/ibus/ibus_config_client.cc
@@ -4,6 +4,8 @@
 
 #include "chromeos/dbus/ibus/ibus_config_client.h"
 
+#include <vector>
+
 #include "base/bind.h"
 #include "base/callback.h"
 #include "chromeos/dbus/ibus/ibus_constants.h"
@@ -17,23 +19,69 @@ namespace chromeos {
 
 namespace {
 
+// Called when the |signal| is connected.
+void OnSignalConnected(const std::string& interface,
+                       const std::string& signal,
+                       bool succeeded) {
+  DLOG_IF(ERROR, !succeeded) << "Connect to " << interface << " "

[satorux1, 2012/11/30 05:52:58]

LOG_IF. no need to make it a DLOG?

[Seigo Nonaka, 2012/11/30 07:36:34]

Done.

+                             << signal << " failed.";
+}
+
+// Called when the GetNameOwner method call is failed.
+void OnGetNameOwnerFail(dbus::ErrorResponse* response) {
+  // Do nothing, because method call sometimes fails due to bootstrap timing of
+  // ibus-memconf.
+}
+
 // The IBusConfigClient implementation.
 class IBusConfigClientImpl : public IBusConfigClient {
  public:
   explicit IBusConfigClientImpl(dbus::Bus* bus)
-      : proxy_(bus->GetObjectProxy(ibus::kServiceName,
-                                   dbus::ObjectPath(
-                                       ibus::config::kServicePath))),
+      : proxy_(NULL),
+        bus_(bus),
         weak_ptr_factory_(this) {
   }
 
   virtual ~IBusConfigClientImpl() {}
 
   // IBusConfigClient override.
+  virtual void InitializeAsync(const OnIBusConfigReady& on_ready) OVERRIDE {
+    // We should check that the ibus-config daemon actually works first, so we
+    // can't initialize synchronously.
+    dbus::ObjectProxy* dbus_proxy = bus_->GetObjectProxy(
+        ibus::kServiceName,
+        dbus::ObjectPath(ibus::kDBusObjectPath));
+
+    // Watch NameOwnerChanged signal which is fired when the ibus-config daemon
+    // request its name ownership.
+    dbus_proxy->ConnectToSignal(
+        ibus::kDBusInterface,
+        ibus::kNameOwnerChangedSignal,
+        base::Bind(&IBusConfigClientImpl::OnNameOwnerChanged,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   on_ready),
+        base::Bind(&OnSignalConnected));
+
+    dbus::MethodCall method_call(ibus::kDBusInterface,
+                                 ibus::kGetNameOwnerMethod);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(ibus::config::kServiceName);
+    dbus_proxy->CallMethodWithErrorCallback(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&IBusConfigClientImpl::OnGetNameOwner,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   on_ready),
+        base::Bind(&OnGetNameOwnerFail));
+  }
+
+  // IBusConfigClient override.
   virtual void SetStringValue(const std::string& section,
                               const std::string& key,
                               const std::string& value,
                               const ErrorCallback& error_callback) OVERRIDE {
+    if (!proxy_)
+      return;
     DCHECK(!error_callback.is_null());
     dbus::MethodCall method_call(ibus::config::kServiceInterface,
                                  ibus::config::kSetValueMethod);
@@ -52,6 +100,8 @@ class IBusConfigClientImpl : public IBusConfigClient {
                            const std::string& key,
                            int value,
                            const ErrorCallback& error_callback) OVERRIDE {
+    if (!proxy_)
+      return;
     DCHECK(!error_callback.is_null());
     dbus::MethodCall method_call(ibus::config::kServiceInterface,
                                  ibus::config::kSetValueMethod);
@@ -70,6 +120,8 @@ class IBusConfigClientImpl : public IBusConfigClient {
                             const std::string& key,
                             bool value,
                             const ErrorCallback& error_callback) OVERRIDE {
+    if (!proxy_)
+      return;
     DCHECK(!error_callback.is_null());
     dbus::MethodCall method_call(ibus::config::kServiceInterface,
                                  ibus::config::kSetValueMethod);
@@ -89,6 +141,8 @@ class IBusConfigClientImpl : public IBusConfigClient {
       const std::string& key,
       const std::vector<std::string>& value,
       const ErrorCallback& error_callback) OVERRIDE {
+    if (!proxy_)
+      return;
     DCHECK(!error_callback.is_null());
     dbus::MethodCall method_call(ibus::config::kServiceInterface,
                                  ibus::config::kSetValueMethod);
@@ -111,6 +165,8 @@ class IBusConfigClientImpl : public IBusConfigClient {
  private:
   void CallWithDefaultCallback(dbus::MethodCall* method_call,
                                const ErrorCallback& error_callback) {
+    if (!proxy_)
+      return;
     proxy_->CallMethodWithErrorCallback(
         method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
@@ -136,7 +192,72 @@ class IBusConfigClientImpl : public IBusConfigClient {
     error_callback.Run();
   }
 
+  void OnNameOwnerChanged(const OnIBusConfigReady& on_ready,
+                          dbus::Signal* signal) {
+    DCHECK(signal);
+    std::string name;
+    std::string old_owner;
+    std::string new_owner;
+
+    dbus::MessageReader reader(signal);
+    if (!reader.PopString(&name) ||
+        !reader.PopString(&old_owner) ||
+        !reader.PopString(&new_owner)) {
+      DLOG(ERROR) << "Invalid response of NameOwnerChanged."
+                  << signal->ToString();
+      return;
+    }
+
+    if (name != ibus::config::kServiceName)
+      return;  // Not a signal for ibus-config.
+
+    if (!old_owner.empty() || new_owner.empty()) {
+      DVLOG(1) << "Unexpected name owner change: name=" << name
+               << ", old_owner=" << old_owner << ", new_owner=" << new_owner;
+      proxy_ = NULL;
+      return;
+    }
+
+    if (proxy_)
+      return;  // Already initialized.
+
+    proxy_ = bus_->GetObjectProxy(ibus::config::kServiceName,
+                                  dbus::ObjectPath(
+                                      ibus::config::kServicePath));
+    if (!on_ready.is_null())
+      on_ready.Run();
+  }
+
+  // Handles response of GetNameOwner.
+  void OnGetNameOwner(const OnIBusConfigReady& on_ready,
+                      dbus::Response* response) {
+    if (!response) {
+      DLOG(ERROR) << "Response is NULL.";

[satorux1, 2012/11/30 05:52:58]

LOG(ERROR) ? For ERRORs, I think we should use LOG

[Seigo Nonaka, 2012/11/30 07:36:34]

Done.

+      return;
+    }
+    std::string owner;
+    dbus::MessageReader reader(response);
+
+    if (!reader.PopString(&owner)) {
+      DLOG(ERROR) << "Invalid response of GetNameOwner."

[satorux1, 2012/11/30 05:52:58]

ditto

[Seigo Nonaka, 2012/11/30 07:36:34]

Done.

+                  << response->ToString();
+      return;
+    }
+
+    // If the owner is empty, ibus-config daemon is not ready. So will
+    // initialize object proxy on NameOwnerChanged signal.
+    if (owner.empty())
+      return;
+
+    proxy_ = bus_->GetObjectProxy(ibus::config::kServiceName,
+                                  dbus::ObjectPath(
+                                      ibus::config::kServicePath));
+    if (!on_ready.is_null())
+      on_ready.Run();
+  }
+
   dbus::ObjectProxy* proxy_;
+  dbus::Bus* bus_;
   base::WeakPtrFactory<IBusConfigClientImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(IBusConfigClientImpl);
@@ -147,6 +268,7 @@ class IBusConfigClientStubImpl : public IBusConfigClient {
  public:
   IBusConfigClientStubImpl() {}
   virtual ~IBusConfigClientStubImpl() {}
+  virtual void InitializeAsync(const OnIBusConfigReady& on_ready) OVERRIDE {}
   virtual void SetStringValue(const std::string& section,
                               const std::string& key,
                               const std::string& value,