Linux/CrOS: Listen for mtpd service owner change events and communicate with the new service owner.

The mtpd dbus service may not start right away. Any attempts to use it may be racy due to the lack of a service owner. Listening for service owner changes fixes this race.

BUG=241302
TEST=Manual, see bug for repro case.

Review URL: https://chromiumcodereview.appspot.com/15741025

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

4 files changed, 395 insertions, 22 deletions

diff --git a/dbus/bus.cc b/dbus/bus.cc
index 1ddeeb8..c830f06 100644
--- a/dbus/bus.cc
+++ b/dbus/bus.cc
@@ -9,10 +9,12 @@
 #include "base/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/stl_util.h"
+#include "base/stringprintf.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time.h"
 #include "dbus/exported_object.h"
+#include "dbus/message.h"
 #include "dbus/object_manager.h"
 #include "dbus/object_path.h"
 #include "dbus/object_proxy.h"
@@ -27,6 +29,15 @@ const char kDisconnectedMatchRule[] =
     "type='signal', path='/org/freedesktop/DBus/Local',"
     "interface='org.freedesktop.DBus.Local', member='Disconnected'";
 
+// The NameOwnerChanged member in org.freedesktop.DBus
+const char kNameOwnerChangedSignal[] = "NameOwnerChanged";
+
+// The match rule used to filter for changes to a given service name owner.
+const char kServiceNameOwnerChangeMatchRule[] =
+    "type='signal',interface='org.freedesktop.DBus',"
+    "member='NameOwnerChanged',path='/org/freedesktop/DBus',"
+    "sender='org.freedesktop.DBus',arg0='%s'";
+
 // The class is used for watching the file descriptor used for D-Bus
 // communication.
 class Watch : public base::MessagePumpLibevent::Watcher {
@@ -908,6 +919,112 @@ void Bus::GetServiceOwnerInternal(const std::string& service_name,
   PostTaskToOriginThread(FROM_HERE, base::Bind(callback, service_owner));
 }
 
+void Bus::ListenForServiceOwnerChange(
+    const std::string& service_name,
+    const GetServiceOwnerCallback& callback) {
+  AssertOnOriginThread();
+  DCHECK(!service_name.empty());
+  DCHECK(!callback.is_null());
+
+  PostTaskToDBusThread(FROM_HERE,
+                       base::Bind(&Bus::ListenForServiceOwnerChangeInternal,
+                                  this, service_name, callback));
+}
+
+void Bus::ListenForServiceOwnerChangeInternal(
+    const std::string& service_name,
+    const GetServiceOwnerCallback& callback) {
+  AssertOnDBusThread();
+  DCHECK(!service_name.empty());
+  DCHECK(!callback.is_null());
+
+  if (!Connect() || !SetUpAsyncOperations())
+    return;
+
+  if (service_owner_changed_listener_map_.empty()) {
+    bool filter_added =
+        AddFilterFunction(Bus::OnServiceOwnerChangedFilter, this);
+    DCHECK(filter_added);
+  }
+
+  ServiceOwnerChangedListenerMap::iterator it =
+      service_owner_changed_listener_map_.find(service_name);
+  if (it == service_owner_changed_listener_map_.end()) {
+    // Add a match rule for the new service name.
+    const std::string name_owner_changed_match_rule =
+        base::StringPrintf(kServiceNameOwnerChangeMatchRule,
+                           service_name.c_str());
+    ScopedDBusError error;
+    AddMatch(name_owner_changed_match_rule, error.get());
+    if (error.is_set()) {
+      LOG(ERROR) << "Failed to add match rule for " << service_name
+                 << ". Got " << error.name() << ": " << error.message();
+      return;
+    }
+
+    service_owner_changed_listener_map_[service_name].push_back(callback);
+    return;
+  }
+
+  // Check if the callback has already been added.
+  std::vector<GetServiceOwnerCallback>& callbacks = it->second;
+  for (size_t i = 0; i < callbacks.size(); ++i) {
+    if (callbacks[i].Equals(callback))
+      return;
+  }
+  callbacks.push_back(callback);
+}
+
+void Bus::UnlistenForServiceOwnerChange(
+    const std::string& service_name,
+    const GetServiceOwnerCallback& callback) {
+  AssertOnOriginThread();
+  DCHECK(!service_name.empty());
+  DCHECK(!callback.is_null());
+
+  PostTaskToDBusThread(FROM_HERE,
+                       base::Bind(&Bus::UnlistenForServiceOwnerChangeInternal,
+                                  this, service_name, callback));
+}
+
+void Bus::UnlistenForServiceOwnerChangeInternal(
+    const std::string& service_name,
+    const GetServiceOwnerCallback& callback) {
+  AssertOnDBusThread();
+  DCHECK(!service_name.empty());
+  DCHECK(!callback.is_null());
+
+  ServiceOwnerChangedListenerMap::iterator it =
+      service_owner_changed_listener_map_.find(service_name);
+  if (it == service_owner_changed_listener_map_.end())
+    return;
+
+  std::vector<GetServiceOwnerCallback>& callbacks = it->second;
+  for (size_t i = 0; i < callbacks.size(); ++i) {
+    if (callbacks[i].Equals(callback)) {
+      callbacks.erase(callbacks.begin() + i);
+      break;  // There can be only one.
+    }
+  }
+  if (!callbacks.empty())
+    return;
+
+  // Last callback for |service_name| has been removed, remove match rule.
+  const std::string name_owner_changed_match_rule =
+      base::StringPrintf(kServiceNameOwnerChangeMatchRule,
+                         service_name.c_str());
+  ScopedDBusError error;
+  RemoveMatch(name_owner_changed_match_rule, error.get());
+  // And remove |service_owner_changed_listener_map_| entry.
+  service_owner_changed_listener_map_.erase(it);
+
+  if (service_owner_changed_listener_map_.empty()) {
+    bool filter_removed =
+        RemoveFilterFunction(Bus::OnServiceOwnerChangedFilter, this);
+    DCHECK(filter_removed);
+  }
+}
+
 dbus_bool_t Bus::OnAddWatch(DBusWatch* raw_watch) {
   AssertOnDBusThread();
 
@@ -1000,36 +1117,81 @@ void Bus::OnConnectionDisconnected(DBusConnection* connection) {
   ShutdownAndBlock();
 }
 
+void Bus::OnServiceOwnerChanged(DBusMessage* message) {
+  DCHECK(message);
+  AssertOnDBusThread();
+
+  // |message| will be unrefed on exit of the function. Increment the
+  // reference so we can use it in Signal::FromRawMessage() below.
+  dbus_message_ref(message);
+  scoped_ptr<Signal> signal(Signal::FromRawMessage(message));
+
+  // Confirm the validity of the NameOwnerChanged signal.
+  if (signal->GetMember() != kNameOwnerChangedSignal ||
+      signal->GetInterface() != DBUS_INTERFACE_DBUS ||
+      signal->GetSender() != DBUS_SERVICE_DBUS) {
+    return;
+  }
+
+  MessageReader reader(signal.get());
+  std::string service_name;
+  std::string old_owner;
+  std::string new_owner;
+  if (!reader.PopString(&service_name) ||
+      !reader.PopString(&old_owner) ||
+      !reader.PopString(&new_owner)) {
+    return;
+  }
+
+  ServiceOwnerChangedListenerMap::const_iterator it =
+      service_owner_changed_listener_map_.find(service_name);
+  if (it == service_owner_changed_listener_map_.end())
+    return;
+
+  const std::vector<GetServiceOwnerCallback>& callbacks = it->second;
+  for (size_t i = 0; i < callbacks.size(); ++i) {
+    PostTaskToOriginThread(FROM_HERE,
+                           base::Bind(callbacks[i], new_owner));
+  }
+}
+
+// static
 dbus_bool_t Bus::OnAddWatchThunk(DBusWatch* raw_watch, void* data) {
   Bus* self = static_cast<Bus*>(data);
   return self->OnAddWatch(raw_watch);
 }
 
+// static
 void Bus::OnRemoveWatchThunk(DBusWatch* raw_watch, void* data) {
   Bus* self = static_cast<Bus*>(data);
   self->OnRemoveWatch(raw_watch);
 }
 
+// static
 void Bus::OnToggleWatchThunk(DBusWatch* raw_watch, void* data) {
   Bus* self = static_cast<Bus*>(data);
   self->OnToggleWatch(raw_watch);
 }
 
+// static
 dbus_bool_t Bus::OnAddTimeoutThunk(DBusTimeout* raw_timeout, void* data) {
   Bus* self = static_cast<Bus*>(data);
   return self->OnAddTimeout(raw_timeout);
 }
 
+// static
 void Bus::OnRemoveTimeoutThunk(DBusTimeout* raw_timeout, void* data) {
   Bus* self = static_cast<Bus*>(data);
   self->OnRemoveTimeout(raw_timeout);
 }
 
+// static
 void Bus::OnToggleTimeoutThunk(DBusTimeout* raw_timeout, void* data) {
   Bus* self = static_cast<Bus*>(data);
   self->OnToggleTimeout(raw_timeout);
 }
 
+// static
 void Bus::OnDispatchStatusChangedThunk(DBusConnection* connection,
                                        DBusDispatchStatus status,
                                        void* data) {
@@ -1037,6 +1199,7 @@ void Bus::OnDispatchStatusChangedThunk(DBusConnection* connection,
   self->OnDispatchStatusChanged(connection, status);
 }
 
+// static
 DBusHandlerResult Bus::OnConnectionDisconnectedFilter(
     DBusConnection* connection,
     DBusMessage* message,
@@ -1045,11 +1208,26 @@ DBusHandlerResult Bus::OnConnectionDisconnectedFilter(
                              DBUS_INTERFACE_LOCAL,
                              kDisconnectedSignal)) {
     Bus* self = static_cast<Bus*>(data);
-    self->AssertOnDBusThread();
     self->OnConnectionDisconnected(connection);
     return DBUS_HANDLER_RESULT_HANDLED;
   }
   return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
+// static
+DBusHandlerResult Bus::OnServiceOwnerChangedFilter(
+    DBusConnection* connection,
+    DBusMessage* message,
+    void* data) {
+  if (dbus_message_is_signal(message,
+                             DBUS_INTERFACE_DBUS,
+                             kNameOwnerChangedSignal)) {
+    Bus* self = static_cast<Bus*>(data);
+    self->OnServiceOwnerChanged(message);
+  }
+  // Always return unhandled to let others, e.g. ObjectProxies, handle the same
+  // signal.
+  return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+}
+
 }  // namespace dbus
diff --git a/dbus/bus.h b/dbus/bus.h
index 03e2aa5..220ed78 100644
--- a/dbus/bus.h
+++ b/dbus/bus.h
@@ -11,6 +11,7 @@
 #include <set>
 #include <string>
 #include <utility>
+#include <vector>
 
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
@@ -22,7 +23,6 @@
 namespace base {
 class SequencedTaskRunner;
 class SingleThreadTaskRunner;
-class Thread;
 }
 
 namespace tracked_objects {
@@ -563,6 +563,28 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
   virtual void GetServiceOwner(const std::string& service_name,
                                const GetServiceOwnerCallback& callback);
 
+  // Whenever the owner for |service_name| changes, run |callback| with the
+  // name of the new owner. If the owner goes away, then |callback| receives
+  // an empty string.
+  //
+  // Any unique (service_name, callback) can be used. Duplicate are ignored.
+  // |service_name| must not be empty and |callback| must not be null.
+  //
+  // Must be called in the origin thread.
+  virtual void ListenForServiceOwnerChange(
+      const std::string& service_name,
+      const GetServiceOwnerCallback& callback);
+
+  // Stop listening for |service_name| owner changes for |callback|.
+  // Any unique (service_name, callback) can be used. Non-registered callbacks
+  // for a given service name are ignored.
+  // |service_name| must not be empty and |callback| must not be null.
+  //
+  // Must be called in the origin thread.
+  virtual void UnlistenForServiceOwnerChange(
+      const std::string& service_name,
+      const GetServiceOwnerCallback& callback);
+
   // Returns true if the bus is connected to D-Bus.
   bool is_connected() { return connection_ != NULL; }
 
@@ -592,6 +614,16 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
   void GetServiceOwnerInternal(const std::string& service_name,
                                const GetServiceOwnerCallback& callback);
 
+  // Helper function used for ListenForServiceOwnerChange().
+  void ListenForServiceOwnerChangeInternal(
+      const std::string& service_name,
+      const GetServiceOwnerCallback& callback);
+
+  // Helper function used for UnListenForServiceOwnerChange().
+  void UnlistenForServiceOwnerChangeInternal(
+      const std::string& service_name,
+      const GetServiceOwnerCallback& callback);
+
   // Processes the all incoming data to the connection, if any.
   //
   // BLOCKING CALL.
@@ -625,6 +657,9 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
   // Called when the connection is diconnected.
   void OnConnectionDisconnected(DBusConnection* connection);
 
+  // Called when a service owner change occurs.
+  void OnServiceOwnerChanged(DBusMessage* message);
+
   // Callback helper functions. Redirects to the corresponding member function.
   static dbus_bool_t OnAddWatchThunk(DBusWatch* raw_watch, void* data);
   static void OnRemoveWatchThunk(DBusWatch* raw_watch, void* data);
@@ -636,12 +671,18 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
                                            DBusDispatchStatus status,
                                            void* data);
 
-  // Calls OnConnectionDisconnected if the Diconnected signal is received.
+  // Calls OnConnectionDisconnected if the Disconnected signal is received.
   static DBusHandlerResult OnConnectionDisconnectedFilter(
       DBusConnection* connection,
       DBusMessage* message,
       void* user_data);
 
+  // Calls OnServiceOwnerChanged for a NameOwnerChanged signal.
+  static DBusHandlerResult OnServiceOwnerChangedFilter(
+      DBusConnection* connection,
+      DBusMessage* message,
+      void* user_data);
+
   const BusType bus_type_;
   const ConnectionType connection_type_;
   scoped_refptr<base::SequencedTaskRunner> dbus_task_runner_;
@@ -684,6 +725,16 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
                    scoped_refptr<dbus::ObjectManager> > ObjectManagerTable;
   ObjectManagerTable object_manager_table_;
 
+  // A map of NameOwnerChanged signals to listen for and the callbacks to run
+  // on the origin thread when the owner changes.
+  // Only accessed on the DBus thread.
+  // Key: Service name
+  // Value: Vector of callbacks. Unique and expected to be small. Not using
+  //        std::set here because base::Callbacks don't have a '<' operator.
+  typedef std::map<std::string, std::vector<GetServiceOwnerCallback> >
+      ServiceOwnerChangedListenerMap;
+  ServiceOwnerChangedListenerMap service_owner_changed_listener_map_;
+
   bool async_operations_set_up_;
   bool shutdown_completed_;
 
diff --git a/dbus/bus_unittest.cc b/dbus/bus_unittest.cc
index 82949d3..3c453f7 100644
--- a/dbus/bus_unittest.cc
+++ b/dbus/bus_unittest.cc
@@ -7,11 +7,13 @@
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "base/memory/ref_counted.h"
+#include "base/run_loop.h"
 #include "base/threading/thread.h"
 #include "dbus/exported_object.h"
 #include "dbus/object_path.h"
 #include "dbus/object_proxy.h"
 #include "dbus/scoped_dbus_error.h"
+#include "dbus/test_service.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -24,6 +26,49 @@ DBusHandlerResult DummyHandler(DBusConnection* connection,
   return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
+// Test helper for BusTest.ListenForServiceOwnerChange that wraps a
+// base::RunLoop. At Run() time, the caller pass in the expected number of
+// quit calls, and at QuitIfConditionIsSatisified() time, only quit the RunLoop
+// if the expected number of quit calls have been reached.
+class RunLoopWithExpectedCount {
+ public:
+  RunLoopWithExpectedCount() : expected_quit_calls_(0), actual_quit_calls_(0) {}
+  ~RunLoopWithExpectedCount() {}
+
+  void Run(int expected_quit_calls) {
+    DCHECK_EQ(0, expected_quit_calls_);
+    DCHECK_EQ(0, actual_quit_calls_);
+    expected_quit_calls_ = expected_quit_calls;
+    run_loop_.reset(new base::RunLoop());
+    run_loop_->Run();
+  }
+
+  void QuitIfConditionIsSatisified() {
+    if (++actual_quit_calls_ != expected_quit_calls_)
+      return;
+    run_loop_->Quit();
+    expected_quit_calls_ = 0;
+    actual_quit_calls_ = 0;
+  }
+
+ private:
+  scoped_ptr<base::RunLoop> run_loop_;
+  int expected_quit_calls_;
+  int actual_quit_calls_;
+
+  DISALLOW_COPY_AND_ASSIGN(RunLoopWithExpectedCount);
+};
+
+// Test helper for BusTest.ListenForServiceOwnerChange.
+void OnServiceOwnerChanged(RunLoopWithExpectedCount* run_loop_state,
+                           std::string* service_owner,
+                           int* num_of_owner_changes,
+                           const std::string& new_service_owner) {
+  *service_owner = new_service_owner;
+  ++(*num_of_owner_changes);
+  run_loop_state->QuitIfConditionIsSatisified();
+}
+
 }  // namespace
 
 TEST(BusTest, GetObjectProxy) {
@@ -296,3 +341,80 @@ TEST(BusTest, DoubleAddAndRemoveMatch) {
 
   bus->ShutdownAndBlock();
 }
+
+TEST(BusTest, ListenForServiceOwnerChange) {
+  // Setup the current thread's MessageLoop. Must be of TYPE_IO for the
+  // listeners to work.
+  base::MessageLoop message_loop(base::MessageLoop::TYPE_IO);
+  RunLoopWithExpectedCount run_loop_state;
+
+  // Create the bus.
+  dbus::Bus::Options bus_options;
+  scoped_refptr<dbus::Bus> bus = new dbus::Bus(bus_options);
+
+  // Add a listener.
+  std::string service_owner1;
+  int num_of_owner_changes1 = 0;
+  dbus::Bus::GetServiceOwnerCallback callback1 =
+      base::Bind(&OnServiceOwnerChanged,
+                 &run_loop_state,
+                 &service_owner1,
+                 &num_of_owner_changes1);
+  bus->ListenForServiceOwnerChange("org.chromium.TestService", callback1);
+  // This should be a no-op.
+  bus->ListenForServiceOwnerChange("org.chromium.TestService", callback1);
+  base::RunLoop().RunUntilIdle();
+
+  // Nothing has happened yet. Check initial state.
+  EXPECT_TRUE(service_owner1.empty());
+  EXPECT_EQ(0, num_of_owner_changes1);
+
+  // Make an ownership change.
+  ASSERT_TRUE(bus->RequestOwnershipAndBlock("org.chromium.TestService"));
+  run_loop_state.Run(1);
+
+  {
+    // Get the current service owner and check to make sure the listener got
+    // the right value.
+    std::string current_service_owner =
+        bus->GetServiceOwnerAndBlock("org.chromium.TestService",
+                                     dbus::Bus::REPORT_ERRORS);
+    ASSERT_FALSE(current_service_owner.empty());
+
+    // Make sure the listener heard about the new owner.
+    EXPECT_EQ(current_service_owner, service_owner1);
+
+    // Test the second ListenForServiceOwnerChange() above is indeed a no-op.
+    EXPECT_EQ(1, num_of_owner_changes1);
+  }
+
+  // Add a second listener.
+  std::string service_owner2;
+  int num_of_owner_changes2 = 0;
+  dbus::Bus::GetServiceOwnerCallback callback2 =
+      base::Bind(&OnServiceOwnerChanged,
+                 &run_loop_state,
+                 &service_owner2,
+                 &num_of_owner_changes2);
+  bus->ListenForServiceOwnerChange("org.chromium.TestService", callback2);
+  base::RunLoop().RunUntilIdle();
+
+  // Release the ownership and make sure the service owner listeners fire with
+  // the right values and the right number of times.
+  ASSERT_TRUE(bus->ReleaseOwnership("org.chromium.TestService"));
+  run_loop_state.Run(2);
+
+  EXPECT_TRUE(service_owner1.empty());
+  EXPECT_TRUE(service_owner2.empty());
+  EXPECT_EQ(2, num_of_owner_changes1);
+  EXPECT_EQ(1, num_of_owner_changes2);
+
+  // Unlisten so shutdown can proceed correctly.
+  bus->UnlistenForServiceOwnerChange("org.chromium.TestService", callback1);
+  bus->UnlistenForServiceOwnerChange("org.chromium.TestService", callback2);
+  base::RunLoop().RunUntilIdle();
+
+  // Shut down synchronously.
+  bus->ShutdownAndBlock();
+  EXPECT_TRUE(bus->shutdown_completed());
+}
diff --git a/device/media_transfer_protocol/media_transfer_protocol_manager.cc b/device/media_transfer_protocol/media_transfer_protocol_manager.cc
index 869e924..9bcf5be 100644
--- a/device/media_transfer_protocol/media_transfer_protocol_manager.cc
+++ b/device/media_transfer_protocol/media_transfer_protocol_manager.cc
@@ -50,15 +50,29 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
     session_bus_ = new dbus::Bus(options);
 #endif
 
-    dbus::Bus::GetServiceOwnerCallback reply_task =
+    // Listen for future mtpd service owner changes, in case it is not
+    // available right now. There is no guarantee on Linux or ChromeOS that
+    // mtpd is running already.
+    mtpd_owner_changed_callback_ =
         base::Bind(&MediaTransferProtocolManagerImpl::FinishSetupOnOriginThread,
                    weak_ptr_factory_.GetWeakPtr());
-    GetBus()->GetServiceOwner(mtpd::kMtpdServiceName, reply_task);
+    GetBus()->ListenForServiceOwnerChange(mtpd::kMtpdServiceName,
+                                          mtpd_owner_changed_callback_);
+    GetBus()->GetServiceOwner(mtpd::kMtpdServiceName,
+                              mtpd_owner_changed_callback_);
   }
 
   virtual ~MediaTransferProtocolManagerImpl() {
     DCHECK(g_media_transfer_protocol_manager);
     g_media_transfer_protocol_manager = NULL;
+    GetBus()->UnlistenForServiceOwnerChange(mtpd::kMtpdServiceName,
+                                            mtpd_owner_changed_callback_);
+
+#if !defined(OS_CHROMEOS)
+      session_bus_->PostTaskToDBusThread(
+          FROM_HERE, base::Bind(&dbus::Bus::ShutdownAndBlock, session_bus_));
+#endif
+
     VLOG(1) << "MediaTransferProtocolManager Shutdown completed";
   }
 
@@ -97,7 +111,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
                            const std::string& mode,
                            const OpenStorageCallback& callback) OVERRIDE {
     DCHECK(thread_checker_.CalledOnValidThread());
-    if (!ContainsKey(storage_info_map_, storage_name)) {
+    if (!ContainsKey(storage_info_map_, storage_name) || !mtp_client_) {
       callback.Run(std::string(), true);
       return;
     }
@@ -115,7 +129,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
   virtual void CloseStorage(const std::string& storage_handle,
                             const CloseStorageCallback& callback) OVERRIDE {
     DCHECK(thread_checker_.CalledOnValidThread());
-    if (!ContainsKey(handles_, storage_handle)) {
+    if (!ContainsKey(handles_, storage_handle) || !mtp_client_) {
       callback.Run(true);
       return;
     }
@@ -134,7 +148,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
       const std::string& path,
       const ReadDirectoryCallback& callback) OVERRIDE {
     DCHECK(thread_checker_.CalledOnValidThread());
-    if (!ContainsKey(handles_, storage_handle)) {
+    if (!ContainsKey(handles_, storage_handle) || !mtp_client_) {
       callback.Run(std::vector<MtpFileEntry>(), true);
       return;
     }
@@ -154,7 +168,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
       uint32 file_id,
       const ReadDirectoryCallback& callback) OVERRIDE {
     DCHECK(thread_checker_.CalledOnValidThread());
-    if (!ContainsKey(handles_, storage_handle)) {
+    if (!ContainsKey(handles_, storage_handle) || !mtp_client_) {
       callback.Run(std::vector<MtpFileEntry>(), true);
       return;
     }
@@ -175,7 +189,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
                                    uint32 count,
                                    const ReadFileCallback& callback) OVERRIDE {
     DCHECK(thread_checker_.CalledOnValidThread());
-    if (!ContainsKey(handles_, storage_handle)) {
+    if (!ContainsKey(handles_, storage_handle) || !mtp_client_) {
       callback.Run(std::string(), true);
       return;
     }
@@ -195,7 +209,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
                                  uint32 count,
                                  const ReadFileCallback& callback) OVERRIDE {
     DCHECK(thread_checker_.CalledOnValidThread());
-    if (!ContainsKey(handles_, storage_handle)) {
+    if (!ContainsKey(handles_, storage_handle) || !mtp_client_) {
       callback.Run(std::string(), true);
       return;
     }
@@ -212,7 +226,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
                                  const std::string& path,
                                  const GetFileInfoCallback& callback) OVERRIDE {
     DCHECK(thread_checker_.CalledOnValidThread());
-    if (!ContainsKey(handles_, storage_handle)) {
+    if (!ContainsKey(handles_, storage_handle) || !mtp_client_) {
       callback.Run(MtpFileEntry(), true);
       return;
     }
@@ -230,7 +244,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
                                uint32 file_id,
                                const GetFileInfoCallback& callback) OVERRIDE {
     DCHECK(thread_checker_.CalledOnValidThread());
-    if (!ContainsKey(handles_, storage_handle)) {
+    if (!ContainsKey(handles_, storage_handle) || !mtp_client_) {
       callback.Run(MtpFileEntry(), true);
       return;
     }
@@ -259,6 +273,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
 
   void OnStorageChanged(bool is_attach, const std::string& storage_name) {
     DCHECK(thread_checker_.CalledOnValidThread());
+    DCHECK(mtp_client_);
     if (is_attach) {
       mtp_client_->GetStorageInfo(
           storage_name,
@@ -283,6 +298,7 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
 
   void OnEnumerateStorages(const std::vector<std::string>& storage_names) {
     DCHECK(thread_checker_.CalledOnValidThread());
+    DCHECK(mtp_client_);
     for (size_t i = 0; i < storage_names.size(); ++i) {
       mtp_client_->GetStorageInfo(
           storage_names[i],
@@ -396,20 +412,22 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
   }
 
   // Callback to finish initialization after figuring out if the mtp service
-  // has an owner.
-  // |service_owner| contains the name of the current owner, if any.
-  void FinishSetupOnOriginThread(const std::string& service_owner) {
+  // has an owner, or if the service owner has changed.
+  // |mtpd_service_owner| contains the name of the current owner, if any.
+  void FinishSetupOnOriginThread(const std::string& mtpd_service_owner) {
     DCHECK(thread_checker_.CalledOnValidThread());
 
-    if (service_owner.empty()) {
-#if !defined(OS_CHROMEOS)
-      // |session_bus_| will not get used. Manually shut it down.
-      session_bus_->PostTaskToDBusThread(
-          FROM_HERE, base::Bind(&dbus::Bus::ShutdownAndBlock, session_bus_));
-#endif
+    if (mtpd_service_owner == current_mtpd_owner_)
+      return;
+
+    if (mtpd_service_owner.empty()) {
+      current_mtpd_owner_.clear();
+      mtp_client_.reset();
       return;
     }
 
+    current_mtpd_owner_ = mtpd_service_owner;
+
     mtp_client_.reset(
         MediaTransferProtocolDaemonClient::Create(GetBus(),
                                                   false /* not stub */));
@@ -445,6 +463,10 @@ class MediaTransferProtocolManagerImpl : public MediaTransferProtocolManager {
   // Set of open storage handles.
   std::set<std::string> handles_;
 
+  dbus::Bus::GetServiceOwnerCallback mtpd_owner_changed_callback_;
+
+  std::string current_mtpd_owner_;
+
   // Queued callbacks.
   OpenStorageCallbackQueue open_storage_callbacks_;
   CloseStorageCallbackQueue close_storage_callbacks_;