Linux: use our spiffy new DBus client library for KWallet, instead of dbus-glib.

chrome/browser/password_manager/native_backend_kwallet_x.cc

 // Copyright (c) 2011 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/password_manager/native_backend_kwallet_x.h"
 
-#include <sstream>
+#include <vector>
 
 #include "base/logging.h"
 #include "base/pickle.h"
 #include "base/stl_util.h"
 #include "base/stringprintf.h"
+#include "base/synchronization/waitable_event.h"
 #include "content/browser/browser_thread.h"
+#include "dbus/bus.h"
+#include "dbus/message.h"
+#include "dbus/object_proxy.h"
 #include "grit/chromium_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 
-using std::string;
-using std::vector;
 using webkit_glue::PasswordForm;
 
 // We could localize this string, but then changing your locale would cause
 // you to lose access to all your stored passwords. Maybe best not to do that.
 const char NativeBackendKWallet::kKWalletFolder[] = "Chrome Form Data";
 
 const char NativeBackendKWallet::kKWalletServiceName[] = "org.kde.kwalletd";
 const char NativeBackendKWallet::kKWalletPath[] = "/modules/kwalletd";
 const char NativeBackendKWallet::kKWalletInterface[] = "org.kde.KWallet";
 const char NativeBackendKWallet::kKLauncherServiceName[] = "org.kde.klauncher";
 const char NativeBackendKWallet::kKLauncherPath[] = "/KLauncher";
 const char NativeBackendKWallet::kKLauncherInterface[] = "org.kde.KLauncher";
 
 NativeBackendKWallet::NativeBackendKWallet(LocalProfileId id,
                                            PrefService* prefs)
     : profile_id_(id), prefs_(prefs),
-      error_(NULL), connection_(NULL), proxy_(NULL),
       app_name_(l10n_util::GetStringUTF8(IDS_PRODUCT_NAME)) {
   if (PasswordStoreX::PasswordsUseLocalProfileId(prefs)) {
     folder_name_ = GetProfileSpecificFolderName();
     // We already did the migration previously. Don't try again.
     migrate_tried_ = true;
   } else {
     folder_name_ = kKWalletFolder;
     migrate_tried_ = false;
   }
 }
 
 NativeBackendKWallet::~NativeBackendKWallet() {
-  if (proxy_)
-    g_object_unref(proxy_);
+  if (session_bus_)
+    session_bus_->ShutdownAndBlock();

[satorux1, 2011/09/06 20:42:40]

This would crash with DCHECK() if D-Bus thread is set.
ShutdownOnDBusThreadAndBlock() should be used instead if D-Bus thread is in use.

[Mike Mammarella, 2011/09/07 01:01:27]

Ah, one of the perils of code you have never actually run, I suppose. :)
I'm having some problem loading the preferences in my builds and so I can't
actually test this change yet. I'll figure out what's going on and test it
though.

 }
 
 bool NativeBackendKWallet::Init() {
   // Get a connection to the session bus.
-  connection_ = dbus_g_bus_get(DBUS_BUS_SESSION, &error_);
-  if (CheckError())
-    return false;
+  dbus::Bus::Options options;
+  options.bus_type = dbus::Bus::SESSION;
+  options.connection_type = dbus::Bus::PRIVATE;
+  options.dbus_thread_message_loop_proxy =
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB);

[satorux1, 2011/09/06 20:42:40]

Why do we use the DB thread? Could you add a comment?

Looking at the code, all D-Bus calls are issued by CallMethodAndBlock()
synchronously. Then, you don't need the D-Bus thread. The D-Bus thread is only
used for asynchronous operations.

[Mike Mammarella, 2011/09/07 01:01:27]

Hmm. Now that I think about it more, I think I can actually just do everything
on the DB thread, since I've already moved most of the init work to the DB
thread anyway.

The constructor and Init() will be called on the UI thread, but everything else
(possibly excluding the destructor, hmm) will be called on the DB thread. This
is an external constraint for this code.

+  return InitWithBus(new dbus::Bus(options));
+}
 
+// NativeBackendKWallet isn't reference counted, but the one place we post a
+// message to it (in InitWithBus below) waits for the task to run. So we can
+// disable needing reference counting safely here.
+template<>
+struct RunnableMethodTraits<NativeBackendKWallet> {
+  void RetainCallee(NativeBackendKWallet*) {}
+  void ReleaseCallee(NativeBackendKWallet*) {}
+};
+
+bool NativeBackendKWallet::InitWithBus(dbus::Bus* bus) {
+  DCHECK(!session_bus_);
+  session_bus_ = bus;
+  kwallet_proxy_ = bus->GetObjectProxy(kKWalletServiceName, kKWalletPath);
+
+  // We must synchronously do a few DBus calls to figure out if initialization
+  // succeeds, but later, we'll want to do most work on the DB thread. So we
+  // have to do the initialization on the DB thread here too, and wait for it.
+  bool success = false;
+  base::WaitableEvent event(false, false);
+  BrowserThread::PostTask(BrowserThread::DB, FROM_HERE,
+                          NewRunnableMethod(
+                              this, &NativeBackendKWallet::InitOnDBThread,
+                              &event, &success));
+  event.Wait();
+  return success;
+}
+
+void NativeBackendKWallet::InitOnDBThread(base::WaitableEvent* event,
+                                          bool* success) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
   if (!InitWallet()) {
     // kwalletd may not be running. Try to start it and try again.
     if (!StartKWalletd() || !InitWallet())
-      return false;
+      return;  // *success starts out false
+  }
+  *success = true;
+}
+
+bool NativeBackendKWallet::StartKWalletd() {
+  // Sadly kwalletd doesn't use DBus activation, so we have to make a call to
+  // klauncher to start it.
+  dbus::ObjectProxy* klauncher =
+      session_bus_->GetObjectProxy(kKLauncherServiceName, kKLauncherPath);
+
+  dbus::MethodCall method_call(kKLauncherInterface,
+                               "start_service_by_desktop_name");
+  dbus::MessageWriter builder(&method_call);
+  dbus::MessageWriter empty(&method_call);
+  builder.AppendString("kwalletd");  // serviceName
+  builder.OpenArray("s", &empty);    // urls
+  builder.CloseContainer(&empty);
+  builder.OpenArray("s", &empty);    // envs
+  builder.CloseContainer(&empty);
+  builder.AppendString("");          // startup_id
+  builder.AppendBool(false);         // blind
+  scoped_ptr<dbus::Response> response(
+      klauncher->CallMethodAndBlock(
+          &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+  if (!response.get()) {
+    LOG(ERROR) << "Error contacting klauncher to start kwalletd";
+    return false;
+  }
+  dbus::MessageReader reader(response.get());
+  int32_t ret = -1;
+  std::string dbusName;

[satorux1, 2011/09/06 20:42:40]

dbus_name

[Mike Mammarella, 2011/09/07 01:01:27]

Done. Huh. I wonder where this came from. I'm pretty sure I wouldn't have
written it like that unless I copied it from somewhere, but I can't find where.

+  std::string error;
+  int32_t pid = -1;
+  if (!reader.PopInt32(&ret) || !reader.PopString(&dbusName) ||
+      !reader.PopString(&error) || !reader.PopInt32(&pid)) {
+    LOG(ERROR) << "Error reading response from klauncher to start kwalletd";

[satorux1, 2011/09/06 20:42:40]

You could add << response->ToString(). It'll make debugging easier.

[Mike Mammarella, 2011/09/07 01:01:27]

Done.

+    return false;
+  }
+  if (!error.empty()) {
+    LOG(ERROR) << "Error launching kwalletd: " << error;
+    return false;
+  }
+  if (ret) {
+    LOG(ERROR) << "Error launching kwalletd: " << ret;
+    return false;
   }

[satorux1, 2011/09/06 20:42:40]

Maybe you could merge the two conditionals, by using ToString():

if (!error.empty() || ret) {
  LOG(ERROR) <<  "Error launching kwalletd: " << response->ToString();

[Mike Mammarella, 2011/09/07 01:01:27]

Done.

 
   return true;
 }
 
-bool NativeBackendKWallet::StartKWalletd() {
-  // Sadly kwalletd doesn't use DBUS activation, so we have to make a call to
-  // klauncher to start it.
-  DBusGProxy* klauncher_proxy =
-      dbus_g_proxy_new_for_name(connection_, kKLauncherServiceName,
-                                kKLauncherPath, kKLauncherInterface);
-
-  char* empty_string_list = NULL;
-  int ret = 1;
-  char* error = NULL;
-  dbus_g_proxy_call(klauncher_proxy, "start_service_by_desktop_name", &error_,
-                    G_TYPE_STRING,  "kwalletd",          // serviceName
-                    G_TYPE_STRV,    &empty_string_list,  // urls
-                    G_TYPE_STRV,    &empty_string_list,  // envs
-                    G_TYPE_STRING,  "",                  // startup_id
-                    G_TYPE_BOOLEAN, (gboolean) false,    // blind
-                    G_TYPE_INVALID,
-                    G_TYPE_INT,     &ret,                // result
-                    G_TYPE_STRING,  NULL,                // dubsName
-                    G_TYPE_STRING,  &error,              // error
-                    G_TYPE_INT,     NULL,                // pid
-                    G_TYPE_INVALID);
-
-  if (error && *error) {
-    LOG(ERROR) << "Error launching kwalletd: " << error;
-    ret = 1;  // Make sure we return false after freeing.
+bool NativeBackendKWallet::InitWallet() {
+  {
+    // Check that KWallet is enabled.
+    dbus::MethodCall method_call(kKWalletInterface, "isEnabled");
+    scoped_ptr<dbus::Response> response(
+        kwallet_proxy_->CallMethodAndBlock(
+            &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+    if (!response.get()) {
+      LOG(ERROR) << "Error contacting kwalletd (isEnabled)";
+      return false;
+    }
+    dbus::MessageReader reader(response.get());
+    bool enabled = false;
+    if (!reader.PopBool(&enabled)) {
+      LOG(ERROR) << "Error reading response from kwalletd (isEnabled)";

[satorux1, 2011/09/06 20:42:40]

Likewise, I'd recommend including response->ToString(). I found it useful for
debugging. Please consider adding it in other places where it makes sense.

[Mike Mammarella, 2011/09/07 01:01:27]

Done.

+      return false;
+    }
+    // Not enabled? Don't use KWallet. But also don't warn here.
+    if (!enabled)
+      return false;
   }
 
-  g_free(error);
-  g_object_unref(klauncher_proxy);
-
-  if (CheckError() || ret != 0)
-    return false;
-  return true;
-}
-
-bool NativeBackendKWallet::InitWallet() {
-  // Make a proxy to KWallet.
-  proxy_ = dbus_g_proxy_new_for_name(connection_, kKWalletServiceName,
-                                     kKWalletPath, kKWalletInterface);
-
-  // Check KWallet is enabled.
-  gboolean is_enabled = false;
-  dbus_g_proxy_call(proxy_, "isEnabled", &error_,
-                    G_TYPE_INVALID,
-                    G_TYPE_BOOLEAN, &is_enabled,
-                    G_TYPE_INVALID);
-  if (CheckError() || !is_enabled)
-    return false;
-
-  // Get the wallet name.
-  char* wallet_name = NULL;
-  dbus_g_proxy_call(proxy_, "networkWallet", &error_,
-                    G_TYPE_INVALID,
-                    G_TYPE_STRING, &wallet_name,
-                    G_TYPE_INVALID);
-  if (CheckError() || !wallet_name)
-    return false;
-
-  wallet_name_.assign(wallet_name);
-  g_free(wallet_name);
+  {
+    // Get the wallet name.
+    dbus::MethodCall method_call(kKWalletInterface, "networkWallet");
+    scoped_ptr<dbus::Response> response(
+        kwallet_proxy_->CallMethodAndBlock(
+            &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+    if (!response.get()) {
+      LOG(ERROR) << "Error contacting kwalletd (networkWallet)";
+      return false;
+    }
+    dbus::MessageReader reader(response.get());
+    if (!reader.PopString(&wallet_name_)) {
+      LOG(ERROR) << "Error reading response from kwalletd (networkWallet)";
+      return false;
+    }
+  }
 
   return true;
 }
 
 bool NativeBackendKWallet::AddLogin(const PasswordForm& form) {
   int wallet_handle = WalletHandle();
   if (wallet_handle == kInvalidKWalletHandle)
     return false;
 
   PasswordFormList forms;
@@ skipping 63 matching lines @@
   return ok;
 }
 
 bool NativeBackendKWallet::RemoveLoginsCreatedBetween(
     const base::Time& delete_begin,
     const base::Time& delete_end) {
   int wallet_handle = WalletHandle();
   if (wallet_handle == kInvalidKWalletHandle)
     return false;
 
   // We could probably also use readEntryList here.

[satorux1, 2011/09/06 20:42:40]

What's readEntryList? Is this comment still valid?

[Mike Mammarella, 2011/09/07 01:01:27]

It's another possible method call in the KWallet interface, instead of using
entryList and then readEntry on each. But the documentation is pretty sparse on
how to use these in the first place so I'm probably never going to actually make
that change.

-  char** realm_list = NULL;
-  dbus_g_proxy_call(proxy_, "entryList", &error_,
-                    G_TYPE_INT,     wallet_handle,             // handle
-                    G_TYPE_STRING,  folder_name_.c_str(),      // folder
-                    G_TYPE_STRING,  app_name_.c_str(),         // appid
-                    G_TYPE_INVALID,
-                    G_TYPE_STRV,    &realm_list,
-                    G_TYPE_INVALID);
-  if (CheckError())
-    return false;
+  std::vector<std::string> realm_list;
+  {
+    dbus::MethodCall method_call(kKWalletInterface, "entryList");
+    dbus::MessageWriter builder(&method_call);
+    builder.AppendInt32(wallet_handle);  // handle
+    builder.AppendString(folder_name_);  // folder
+    builder.AppendString(app_name_);     // appid
+    scoped_ptr<dbus::Response> response(
+        kwallet_proxy_->CallMethodAndBlock(
+            &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+    if (!response.get()) {
+      LOG(ERROR) << "Error contacting kwalletd (entryList)";
+      return false;
+    }
+    dbus::MessageReader reader(response.get());
+    dbus::MessageReader array(response.get());
+    if (!reader.PopArray(&array)) {
+      LOG(ERROR) << "Error reading response from kwalletd (entryList)";
+      return false;
+    }
+    while (array.HasMoreData()) {
+      std::string realm;
+      if (!array.PopString(&realm)) {
+        LOG(ERROR) << "Error reading response from kwalletd (entryList)";
+        return false;
+      }
+      realm_list.push_back(realm);
+    }
+  }
 
   bool ok = true;
-  for (char** realm = realm_list; *realm; ++realm) {
-    GArray* byte_array = NULL;
-    dbus_g_proxy_call(proxy_, "readEntry", &error_,
-                      G_TYPE_INT,     wallet_handle,           // handle
-                      G_TYPE_STRING,  folder_name_.c_str(),    // folder
-                      G_TYPE_STRING,  *realm,                  // key
-                      G_TYPE_STRING,  app_name_.c_str(),       // appid
-                      G_TYPE_INVALID,
-                      DBUS_TYPE_G_UCHAR_ARRAY, &byte_array,
-                      G_TYPE_INVALID);
-
-    if (CheckError() || !byte_array ||
-        !CheckSerializedValue(byte_array, *realm)) {
+  for (size_t i = 0; i < realm_list.size(); ++i) {
+    const std::string& signon_realm = realm_list[i];
+    dbus::MethodCall method_call(kKWalletInterface, "readEntry");
+    dbus::MessageWriter builder(&method_call);
+    builder.AppendInt32(wallet_handle);  // handle
+    builder.AppendString(folder_name_);  // folder
+    builder.AppendString(signon_realm);  // key
+    builder.AppendString(app_name_);     // appid
+    scoped_ptr<dbus::Response> response(
+        kwallet_proxy_->CallMethodAndBlock(
+            &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+    if (!response.get()) {
+      LOG(ERROR) << "Error contacting kwalletd (readEntry)";
       continue;
     }
+    dbus::MessageReader reader(response.get());
+    uint8_t* bytes = NULL;
+    size_t length = 0;
+    if (!reader.PopArrayOfBytes(&bytes, &length)) {
+      LOG(ERROR) << "Error reading response from kwalletd (readEntry)";
+      continue;
+    }
+    if (!bytes || !CheckSerializedValue(bytes, length, signon_realm))
+      continue;
 
-    string signon_realm(*realm);
-    Pickle pickle(byte_array->data, byte_array->len);
+    // Can't we all just agree on whether bytes are signed or not? Please?
+    Pickle pickle(reinterpret_cast<const char*>(bytes), length);
     PasswordFormList all_forms;
     DeserializeValue(signon_realm, pickle, &all_forms);
-    g_array_free(byte_array, true);
 
     PasswordFormList kept_forms;
     kept_forms.reserve(all_forms.size());
     for (size_t i = 0; i < all_forms.size(); ++i) {
       if (delete_begin <= all_forms[i]->date_created &&
           (delete_end.is_null() || all_forms[i]->date_created < delete_end)) {
         delete all_forms[i];
       } else {
         kept_forms.push_back(all_forms[i]);
       }
     }
 
     if (!SetLoginsList(kept_forms, signon_realm, wallet_handle))
       ok = false;
     STLDeleteElements(&kept_forms);
   }
-  g_strfreev(realm_list);
   return ok;
 }
 
 bool NativeBackendKWallet::GetLogins(const PasswordForm& form,
                                      PasswordFormList* forms) {
   int wallet_handle = WalletHandle();
   if (wallet_handle == kInvalidKWalletHandle)
     return false;
   return GetLoginsList(forms, form.signon_realm, wallet_handle);
 }
@@ skipping 15 matching lines @@
 }
 
 bool NativeBackendKWallet::GetBlacklistLogins(PasswordFormList* forms) {
   int wallet_handle = WalletHandle();
   if (wallet_handle == kInvalidKWalletHandle)
     return false;
   return GetLoginsList(forms, false, wallet_handle);
 }
 
 bool NativeBackendKWallet::GetLoginsList(PasswordFormList* forms,
-                                         const string& signon_realm,
+                                         const std::string& signon_realm,
                                          int wallet_handle) {
   // Is there an entry in the wallet?
-  gboolean has_entry = false;
-  dbus_g_proxy_call(proxy_, "hasEntry", &error_,
-                    G_TYPE_INT,     wallet_handle,         // handle
-                    G_TYPE_STRING,  folder_name_.c_str(),  // folder
-                    G_TYPE_STRING,  signon_realm.c_str(),  // key
-                    G_TYPE_STRING,  app_name_.c_str(),     // appid
-                    G_TYPE_INVALID,
-                    G_TYPE_BOOLEAN, &has_entry,
-                    G_TYPE_INVALID);
-
-  if (CheckError())
-    return false;
-  if (!has_entry) {
-    // This is not an error. There just isn't a matching entry.
-    return true;
+  {
+    dbus::MethodCall method_call(kKWalletInterface, "hasEntry");
+    dbus::MessageWriter builder(&method_call);
+    builder.AppendInt32(wallet_handle);  // handle
+    builder.AppendString(folder_name_);  // folder
+    builder.AppendString(signon_realm);  // key
+    builder.AppendString(app_name_);     // appid
+    scoped_ptr<dbus::Response> response(
+        kwallet_proxy_->CallMethodAndBlock(
+            &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+    if (!response.get()) {
+      LOG(ERROR) << "Error contacting kwalletd (hasEntry)";
+      return false;
+    }
+    dbus::MessageReader reader(response.get());
+    bool has_entry = false;
+    if (!reader.PopBool(&has_entry)) {
+      LOG(ERROR) << "Error reading response from kwalletd (hasEntry)";
+      return false;
+    }
+    if (!has_entry) {
+      // This is not an error. There just isn't a matching entry.
+      return true;
+    }
   }
 
-  GArray* byte_array = NULL;
-  dbus_g_proxy_call(proxy_, "readEntry", &error_,
-                    G_TYPE_INT,     wallet_handle,         // handle
-                    G_TYPE_STRING,  folder_name_.c_str(),  // folder
-                    G_TYPE_STRING,  signon_realm.c_str(),  // key
-                    G_TYPE_STRING,  app_name_.c_str(),     // appid
-                    G_TYPE_INVALID,
-                    DBUS_TYPE_G_UCHAR_ARRAY, &byte_array,
-                    G_TYPE_INVALID);
+  {
+    dbus::MethodCall method_call(kKWalletInterface, "readEntry");
+    dbus::MessageWriter builder(&method_call);
+    builder.AppendInt32(wallet_handle);  // handle
+    builder.AppendString(folder_name_);  // folder
+    builder.AppendString(signon_realm);  // key
+    builder.AppendString(app_name_);     // appid
+    scoped_ptr<dbus::Response> response(
+        kwallet_proxy_->CallMethodAndBlock(
+            &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+    if (!response.get()) {
+      LOG(ERROR) << "Error contacting kwalletd (readEntry)";
+      return false;
+    }
+    dbus::MessageReader reader(response.get());
+    uint8_t* bytes = NULL;
+    size_t length = 0;
+    if (!reader.PopArrayOfBytes(&bytes, &length)) {
+      LOG(ERROR) << "Error reading response from kwalletd (readEntry)";
+      return false;
+    }
+    if (!bytes)
+      return false;
+    if (!CheckSerializedValue(bytes, length, signon_realm)) {
+      // This is weird, but we choose not to call it an error. There is an
+      // invalid entry somehow, but by just ignoring it, we make it easier to
+      // repair without having to delete it using kwalletmanager (that is, by
+      // just saving a new password within this realm to overwrite it).
+      return true;
+    }
 
-  if (CheckError() || !byte_array)
-    return false;
-  if (!CheckSerializedValue(byte_array, signon_realm.c_str())) {
-    // This is weird, but we choose not to call it an error. There's an invalid
-    // entry somehow, but by pretending it just doesn't exist, we make it easier
-    // to repair without having to delete it using kwalletmanager (that is, by
-    // just saving a new password within this realm to overwrite it).
-    g_array_free(byte_array, true);
-    return true;
+    // Can't we all just agree on whether bytes are signed or not? Please?
+    Pickle pickle(reinterpret_cast<const char*>(bytes), length);
+    PasswordFormList all_forms;
+    DeserializeValue(signon_realm, pickle, forms);
   }
 
-  Pickle pickle(byte_array->data, byte_array->len);
-  DeserializeValue(signon_realm, pickle, forms);
-  g_array_free(byte_array, true);
-
   return true;
 }
 
 bool NativeBackendKWallet::GetLoginsList(PasswordFormList* forms,
                                          bool autofillable,
                                          int wallet_handle) {
   PasswordFormList all_forms;
   if (!GetAllLogins(&all_forms, wallet_handle))
     return false;
 
@@ skipping 27 matching lines @@
       delete all_forms[i];
     }
   }
 
   return true;
 }
 
 bool NativeBackendKWallet::GetAllLogins(PasswordFormList* forms,
                                         int wallet_handle) {
   // We could probably also use readEntryList here.
-  char** realm_list = NULL;
-  dbus_g_proxy_call(proxy_, "entryList", &error_,
-                    G_TYPE_INT,     wallet_handle,             // handle
-                    G_TYPE_STRING,  folder_name_.c_str(),      // folder
-                    G_TYPE_STRING,  app_name_.c_str(),         // appid
-                    G_TYPE_INVALID,
-                    G_TYPE_STRV,    &realm_list,
-                    G_TYPE_INVALID);
-  if (CheckError())
-    return false;
+  std::vector<std::string> realm_list;
+  {
+    dbus::MethodCall method_call(kKWalletInterface, "entryList");
+    dbus::MessageWriter builder(&method_call);
+    builder.AppendInt32(wallet_handle);  // handle
+    builder.AppendString(folder_name_);  // folder
+    builder.AppendString(app_name_);     // appid
+    scoped_ptr<dbus::Response> response(
+        kwallet_proxy_->CallMethodAndBlock(
+            &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+    if (!response.get()) {
+      LOG(ERROR) << "Error contacting kwalletd (entryList)";
+      return false;
+    }
+    dbus::MessageReader reader(response.get());
+    dbus::MessageReader array(response.get());
+    if (!reader.PopArray(&array)) {
+      LOG(ERROR) << "Error reading response from kwalletd (entryList)";
+      return false;
+    }
+    while (array.HasMoreData()) {
+      std::string realm;
+      if (!array.PopString(&realm)) {
+        LOG(ERROR) << "Error reading response from kwalletd (entryList)";
+        return false;
+      }
+      realm_list.push_back(realm);
+    }
+  }
 
-  for (char** realm = realm_list; *realm; ++realm) {
-    GArray* byte_array = NULL;
-    dbus_g_proxy_call(proxy_, "readEntry", &error_,
-                      G_TYPE_INT,     wallet_handle,           // handle
-                      G_TYPE_STRING,  folder_name_.c_str(),    // folder
-                      G_TYPE_STRING,  *realm,                  // key
-                      G_TYPE_STRING,  app_name_.c_str(),       // appid
-                      G_TYPE_INVALID,
-                      DBUS_TYPE_G_UCHAR_ARRAY, &byte_array,
-                      G_TYPE_INVALID);
-
-    if (CheckError() || !byte_array ||
-        !CheckSerializedValue(byte_array, *realm)) {
+  for (size_t i = 0; i < realm_list.size(); ++i) {
+    const std::string& signon_realm = realm_list[i];
+    dbus::MethodCall method_call(kKWalletInterface, "readEntry");
+    dbus::MessageWriter builder(&method_call);
+    builder.AppendInt32(wallet_handle);  // handle
+    builder.AppendString(folder_name_);  // folder
+    builder.AppendString(signon_realm);  // key
+    builder.AppendString(app_name_);     // appid
+    scoped_ptr<dbus::Response> response(
+        kwallet_proxy_->CallMethodAndBlock(
+            &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+    if (!response.get()) {
+      LOG(ERROR) << "Error contacting kwalletd (readEntry)";
       continue;
     }
+    dbus::MessageReader reader(response.get());
+    uint8_t* bytes = NULL;
+    size_t length = 0;
+    if (!reader.PopArrayOfBytes(&bytes, &length)) {
+      LOG(ERROR) << "Error reading response from kwalletd (readEntry)";
+      continue;
+    }
+    if (!bytes || !CheckSerializedValue(bytes, length, signon_realm))
+      continue;
 
-    Pickle pickle(byte_array->data, byte_array->len);
-    DeserializeValue(*realm, pickle, forms);
-    g_array_free(byte_array, true);
+    // Can't we all just agree on whether bytes are signed or not? Please?
+    Pickle pickle(reinterpret_cast<const char*>(bytes), length);
+    PasswordFormList all_forms;
+    DeserializeValue(signon_realm, pickle, forms);
   }
-  g_strfreev(realm_list);
   return true;
 }
 
 bool NativeBackendKWallet::SetLoginsList(const PasswordFormList& forms,
-                                         const string& signon_realm,
+                                         const std::string& signon_realm,
                                          int wallet_handle) {
   if (forms.empty()) {
     // No items left? Remove the entry from the wallet.
+    dbus::MethodCall method_call(kKWalletInterface, "removeEntry");
+    dbus::MessageWriter builder(&method_call);
+    builder.AppendInt32(wallet_handle);  // handle
+    builder.AppendString(folder_name_);  // folder
+    builder.AppendString(signon_realm);  // key
+    builder.AppendString(app_name_);     // appid
+    scoped_ptr<dbus::Response> response(
+        kwallet_proxy_->CallMethodAndBlock(
+            &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+    if (!response.get()) {
+      LOG(ERROR) << "Error contacting kwalletd (removeEntry)";
+      return kInvalidKWalletHandle;
+    }
+    dbus::MessageReader reader(response.get());
     int ret = 0;
-    dbus_g_proxy_call(proxy_, "removeEntry", &error_,
-                      G_TYPE_INT,     wallet_handle,         // handle
-                      G_TYPE_STRING,  folder_name_.c_str(),  // folder
-                      G_TYPE_STRING,  signon_realm.c_str(),  // key
-                      G_TYPE_STRING,  app_name_.c_str(),     // appid
-                      G_TYPE_INVALID,
-                      G_TYPE_INT,     &ret,
-                      G_TYPE_INVALID);
-    if (CheckError())
+    if (!reader.PopInt32(&ret)) {
+      LOG(ERROR) << "Error reading response from kwalletd (removeEntry)";
       return false;
+    }
     if (ret != 0)
       LOG(ERROR) << "Bad return code " << ret << " from KWallet removeEntry";
     return ret == 0;
   }
 
   Pickle value;
   SerializeValue(forms, &value);
 
-  // Convert the pickled bytes to a GByteArray.
-  GArray* byte_array = g_array_sized_new(false, false, sizeof(char),
-                                         value.size());
-  g_array_append_vals(byte_array, value.data(), value.size());
-
-  // Make the call.
+  dbus::MethodCall method_call(kKWalletInterface, "writeEntry");
+  dbus::MessageWriter builder(&method_call);
+  builder.AppendInt32(wallet_handle);  // handle
+  builder.AppendString(folder_name_);  // folder
+  builder.AppendString(signon_realm);  // key
+  builder.AppendArrayOfBytes(static_cast<const uint8_t*>(value.data()),
+                             value.size());  // value
+  builder.AppendString(app_name_);     // appid
+  scoped_ptr<dbus::Response> response(
+      kwallet_proxy_->CallMethodAndBlock(
+          &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+  if (!response.get()) {
+    LOG(ERROR) << "Error contacting kwalletd (writeEntry)";
+    return kInvalidKWalletHandle;
+  }
+  dbus::MessageReader reader(response.get());
   int ret = 0;
-  dbus_g_proxy_call(proxy_, "writeEntry", &error_,
-                    G_TYPE_INT,           wallet_handle,         // handle
-                    G_TYPE_STRING,        folder_name_.c_str(),  // folder
-                    G_TYPE_STRING,        signon_realm.c_str(),  // key
-                    DBUS_TYPE_G_UCHAR_ARRAY, byte_array,         // value
-                    G_TYPE_STRING,        app_name_.c_str(),     // appid
-                    G_TYPE_INVALID,
-                    G_TYPE_INT,           &ret,
-                    G_TYPE_INVALID);
-  g_array_free(byte_array, true);
-
-  if (CheckError())
+  if (!reader.PopInt32(&ret)) {
+    LOG(ERROR) << "Error reading response from kwalletd (writeEntry)";
     return false;
+  }
   if (ret != 0)
     LOG(ERROR) << "Bad return code " << ret << " from KWallet writeEntry";
   return ret == 0;
 }
 
 bool NativeBackendKWallet::CompareForms(const PasswordForm& a,
                                         const PasswordForm& b,
                                         bool update_check) {
   // An update check doesn't care about the submit element.
   if (!update_check && a.submit_element != b.submit_element)
@@ skipping 20 matching lines @@
     pickle->WriteString16(form->password_element);
     pickle->WriteString16(form->password_value);
     pickle->WriteString16(form->submit_element);
     pickle->WriteBool(form->ssl_valid);
     pickle->WriteBool(form->preferred);
     pickle->WriteBool(form->blacklisted_by_user);
     pickle->WriteInt64(form->date_created.ToTimeT());
   }
 }
 
-bool NativeBackendKWallet::CheckSerializedValue(const GArray* byte_array,
-                                                const char* realm) {
+bool NativeBackendKWallet::CheckSerializedValue(const uint8_t* byte_array,
+                                                size_t length,
+                                                const std::string& realm) {
   const Pickle::Header* header =
-      reinterpret_cast<const Pickle::Header*>(byte_array->data);
-  if (byte_array->len < sizeof(*header) ||
-      header->payload_size > byte_array->len - sizeof(*header)) {
+      reinterpret_cast<const Pickle::Header*>(byte_array);
+  if (length < sizeof(*header) ||
+      header->payload_size > length - sizeof(*header)) {
     LOG(WARNING) << "Invalid KWallet entry detected (realm: " << realm << ")";
     return false;
   }
   return true;
 }
 
-void NativeBackendKWallet::DeserializeValue(const string& signon_realm,
+void NativeBackendKWallet::DeserializeValue(const std::string& signon_realm,
                                             const Pickle& pickle,
                                             PasswordFormList* forms) {
   void* iter = NULL;
 
   int version = -1;
   if (!pickle.ReadInt(&iter, &version) || version != kPickleVersion) {
     // This is the only version so far, so anything else is an error.
     LOG(ERROR) << "Failed to deserialize KWallet entry "
                << "(realm: " << signon_realm << ")";
     return;
@@ skipping 41 matching lines @@
       break;
     }
     form->scheme = static_cast<PasswordForm::Scheme>(scheme);
     form->date_created = base::Time::FromTimeT(date_created);
     forms->push_back(form.release());
   }
 }
 
 bool NativeBackendKWallet::ReadGURL(const Pickle& pickle, void** iter,
                                     GURL* url) {
-  string url_string;
+  std::string url_string;
   if (!pickle.ReadString(iter, &url_string)) {
     LOG(ERROR) << "Failed to deserialize URL";
     *url = GURL();
     return false;
   }
   *url = GURL(url_string);
   return true;
 }
 
-bool NativeBackendKWallet::CheckError() {
-  if (error_) {
-    LOG(ERROR) << "Failed to complete KWallet call: " << error_->message;
-    g_error_free(error_);
-    error_ = NULL;
-    return true;
-  }
-  return false;
-}
-
 int NativeBackendKWallet::WalletHandle() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+
   // Open the wallet.
   // TODO(mdm): Are we leaking these handles? Find out.
-  int handle = kInvalidKWalletHandle;
-  dbus_g_proxy_call(proxy_, "open", &error_,
-                    G_TYPE_STRING, wallet_name_.c_str(),    // wallet
-                    G_TYPE_INT64,  0LL,                     // wid
-                    G_TYPE_STRING, app_name_.c_str(),       // appid
-                    G_TYPE_INVALID,
-                    G_TYPE_INT,    &handle,
-                    G_TYPE_INVALID);
-  if (CheckError() || handle == kInvalidKWalletHandle)
-    return kInvalidKWalletHandle;
+  int32_t handle = kInvalidKWalletHandle;
+  {
+    dbus::MethodCall method_call(kKWalletInterface, "open");
+    dbus::MessageWriter builder(&method_call);
+    builder.AppendString(wallet_name_);  // wallet
+    builder.AppendInt64(0);              // wid
+    builder.AppendString(app_name_);     // appid
+    scoped_ptr<dbus::Response> response(
+        kwallet_proxy_->CallMethodAndBlock(
+            &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+    if (!response.get()) {
+      LOG(ERROR) << "Error contacting kwalletd (open)";
+      return kInvalidKWalletHandle;
+    }
+    dbus::MessageReader reader(response.get());
+    if (!reader.PopInt32(&handle)) {
+      LOG(ERROR) << "Error reading response from kwalletd (open)";
+      return kInvalidKWalletHandle;
+    }
+    if (handle == kInvalidKWalletHandle) {
+      LOG(ERROR) << "Error obtaining KWallet handle";
+      return kInvalidKWalletHandle;
+    }
+  }
 
   // Check if our folder exists.
-  gboolean has_folder = false;
-  dbus_g_proxy_call(proxy_, "hasFolder", &error_,
-                    G_TYPE_INT,    handle,                  // handle
-                    G_TYPE_STRING, folder_name_.c_str(),    // folder
-                    G_TYPE_STRING, app_name_.c_str(),       // appid
-                    G_TYPE_INVALID,
-                    G_TYPE_BOOLEAN, &has_folder,
-                    G_TYPE_INVALID);
-  if (CheckError())
-    return kInvalidKWalletHandle;
+  bool has_folder = false;
+  {
+    dbus::MethodCall method_call(kKWalletInterface, "hasFolder");
+    dbus::MessageWriter builder(&method_call);
+    builder.AppendInt32(handle);         // handle
+    builder.AppendString(folder_name_);  // folder
+    builder.AppendString(app_name_);     // appid
+    scoped_ptr<dbus::Response> response(
+        kwallet_proxy_->CallMethodAndBlock(
+            &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+    if (!response.get()) {
+      LOG(ERROR) << "Error contacting kwalletd (hasFolder)";
+      return kInvalidKWalletHandle;
+    }
+    dbus::MessageReader reader(response.get());
+    if (!reader.PopBool(&has_folder)) {
+      LOG(ERROR) << "Error reading response from kwalletd (hasFolder)";
+      return kInvalidKWalletHandle;
+    }
+  }
 
   // Create it if it didn't.
   if (!has_folder) {
-    gboolean success = false;
-    dbus_g_proxy_call(proxy_, "createFolder", &error_,
-                      G_TYPE_INT,    handle,                // handle
-                      G_TYPE_STRING, folder_name_.c_str(),  // folder
-                      G_TYPE_STRING, app_name_.c_str(),     // appid
-                      G_TYPE_INVALID,
-                      G_TYPE_BOOLEAN, &success,
-                      G_TYPE_INVALID);
-    if (CheckError() || !success)
+    dbus::MethodCall method_call(kKWalletInterface, "createFolder");
+    dbus::MessageWriter builder(&method_call);
+    builder.AppendInt32(handle);         // handle
+    builder.AppendString(folder_name_);  // folder
+    builder.AppendString(app_name_);     // appid
+    scoped_ptr<dbus::Response> response(
+        kwallet_proxy_->CallMethodAndBlock(
+            &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+    if (!response.get()) {
+      LOG(ERROR) << "Error contacting kwalletd (createFolder)";
       return kInvalidKWalletHandle;
+    }
+    dbus::MessageReader reader(response.get());
+    bool success = false;
+    if (!reader.PopBool(&success)) {
+      LOG(ERROR) << "Error reading response from kwalletd (createFolder)";
+      return kInvalidKWalletHandle;
+    }
+    if (!success) {
+      LOG(ERROR) << "Error creating KWallet folder";
+      return kInvalidKWalletHandle;
+    }
   }
 
   // Successful initialization. Try migration if necessary.
   if (!migrate_tried_)
     MigrateToProfileSpecificLogins();
 
   return handle;
 }
 
 std::string NativeBackendKWallet::GetProfileSpecificFolderName() const {
@@ skipping 42 matching lines @@
     // Each other profile must be able to migrate the shared data as well,
     // so we must leave it alone. After a few releases, we'll add code to
     // delete them, and eventually remove this migration code.
     // TODO(mdm): follow through with the plan above.
     PasswordStoreX::SetPasswordsUseLocalProfileId(prefs_);
   } else {
     // We failed to migrate for some reason. Use the old folder name.
     folder_name_ = kKWalletFolder;
   }
 }