Reuse existing object proxies and exported objects, if these exist.
The Bus object shouldn't return new objects if the bus object already owns
the requested object proxies, or the exported objects.
BUG=90036
TEST=dbus_unittests
Review URL: http://codereview.chromium.org/7702001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@97898 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 2ef498fa1e483669eb2d938aa07e450f5f868708
diff --git a/dbus/bus.cc b/dbus/bus.cc
index c0bae42..a259bc0 100644
--- a/dbus/bus.cc
+++ b/dbus/bus.cc
@@ -212,22 +212,36 @@
const std::string& object_path) {
AssertOnOriginThread();
+ // Check if we already have the requested object proxy.
+ const std::string key = service_name + object_path;
+ ObjectProxyTable::iterator iter = object_proxy_table_.find(key);
+ if (iter != object_proxy_table_.end()) {
+ return iter->second;
+ }
+
scoped_refptr<ObjectProxy> object_proxy =
new ObjectProxy(this, service_name, object_path);
- object_proxies_.push_back(object_proxy);
+ object_proxy_table_[key] = object_proxy;
- return object_proxy;
+ return object_proxy.get();
}
ExportedObject* Bus::GetExportedObject(const std::string& service_name,
const std::string& object_path) {
AssertOnOriginThread();
+ // Check if we already have the requested exported object.
+ const std::string key = service_name + object_path;
+ ExportedObjectTable::iterator iter = exported_object_table_.find(key);
+ if (iter != exported_object_table_.end()) {
+ return iter->second;
+ }
+
scoped_refptr<ExportedObject> exported_object =
new ExportedObject(this, service_name, object_path);
- exported_objects_.push_back(exported_object);
+ exported_object_table_[key] = exported_object;
- return exported_object;
+ return exported_object.get();
}
bool Bus::Connect() {
@@ -260,8 +274,9 @@
AssertOnDBusThread();
// Unregister the exported objects.
- for (size_t i = 0; i < exported_objects_.size(); ++i) {
- exported_objects_[i]->Unregister();
+ for (ExportedObjectTable::iterator iter = exported_object_table_.begin();
+ iter != exported_object_table_.end(); ++iter) {
+ iter->second->Unregister();
}
// Release all service names.
@@ -278,8 +293,9 @@
}
// Detach from the remote objects.
- for (size_t i = 0; i < object_proxies_.size(); ++i) {
- object_proxies_[i]->Detach();
+ for (ObjectProxyTable::iterator iter = object_proxy_table_.begin();
+ iter != object_proxy_table_.end(); ++iter) {
+ iter->second->Detach();
}
// Private connection should be closed.
diff --git a/dbus/bus.h b/dbus/bus.h
index e4b615e..e632e9a 100644
--- a/dbus/bus.h
+++ b/dbus/bus.h
@@ -7,6 +7,7 @@
#define DBUS_BUS_H_
#pragma once
+#include <map>
#include <set>
#include <string>
#include <dbus/dbus.h>
@@ -55,8 +56,8 @@
// Note that it's hard to tell if a libdbus function is actually blocking
// or not (ex. dbus_bus_request_name() internally calls
// dbus_connection_send_with_reply_and_block(), which is a blocking
-// call). To err on the side, we consider all libdbus functions that deal
-// with the connection to dbus-damoen to be blocking.
+// call). To err on the safe side, we consider all libdbus functions that
+// deal with the connection to dbus-damoen to be blocking.
//
// EXAMPLE USAGE:
//
@@ -164,8 +165,15 @@
explicit Bus(const Options& options);
// Gets the object proxy for the given service name and the object path.
- // The caller must not delete the returned object. The bus will own the
- // object. Never returns NULL.
+ // The caller must not delete the returned object.
+ //
+ // Returns an existing object proxy if the bus object already owns the
+ // object proxy for the given service name and the object path.
+ // Never returns NULL.
+ //
+ // The bus will own all object proxies created by the bus, to ensure
+ // that the object proxies are detached from remote objects at the
+ // shutdown time of the bus.
//
// The object proxy is used to call methods of remote objects, and
// receive signals from them.
@@ -178,8 +186,15 @@
const std::string& object_path);
// Gets the exported object for the given service name and the object
- // path. The caller must not delete the returned object. The bus will
- // own the object. Never returns NULL.
+ // path. The caller must not delete the returned object.
+ //
+ // Returns an existing exported object if the bus object already owns
+ // the exported object for the given service name and the object path.
+ // Never returns NULL.
+ //
+ // The bus will own all exported objects created by the bus, to ensure
+ // that the exported objects are unregistered at the shutdown time of
+ // the bus.
//
// The exported object is used to export methods of local objects, and
// send signal from them.
@@ -416,8 +431,19 @@
std::set<std::string> registered_object_paths_;
std::set<DBusHandleMessageFunction> filter_functions_added_;
- std::vector<scoped_refptr<dbus::ObjectProxy> > object_proxies_;
- std::vector<scoped_refptr<dbus::ExportedObject> > exported_objects_;
+ // ObjectProxyTable is used to hold the object proxies created by the
+ // bus object. Key is a concatenated string of service name + object path,
+ // like "org.chromium.TestService/org/chromium/TestObject".
+ typedef std::map<std::string,
+ scoped_refptr<dbus::ObjectProxy> > ObjectProxyTable;
+ ObjectProxyTable object_proxy_table_;
+
+ // ExportedObjectTable is used to hold the exported objects created by
+ // the bus object. Key is a concatenated string of service name +
+ // object path, like "org.chromium.TestService/org/chromium/TestObject".
+ typedef std::map<std::string,
+ scoped_refptr<dbus::ExportedObject> > ExportedObjectTable;
+ ExportedObjectTable exported_object_table_;
bool async_operations_are_set_up_;
diff --git a/dbus/bus_unittest.cc b/dbus/bus_unittest.cc
new file mode 100644
index 0000000..5cd198d
--- /dev/null
+++ b/dbus/bus_unittest.cc
@@ -0,0 +1,59 @@
+// 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 "dbus/bus.h"
+
+#include "base/memory/ref_counted.h"
+#include "dbus/exported_object.h"
+#include "dbus/object_proxy.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(BusTest, GetObjectProxy) {
+ dbus::Bus::Options options;
+ scoped_refptr<dbus::Bus> bus = new dbus::Bus(options);
+
+ dbus::ObjectProxy* object_proxy1 =
+ bus->GetObjectProxy("org.chromium.TestService",
+ "/org/chromium/TestObject");
+ ASSERT_TRUE(object_proxy1);
+
+ // This should return the same object.
+ dbus::ObjectProxy* object_proxy2 =
+ bus->GetObjectProxy("org.chromium.TestService",
+ "/org/chromium/TestObject");
+ ASSERT_TRUE(object_proxy2);
+ EXPECT_EQ(object_proxy1, object_proxy2);
+
+ // This should not.
+ dbus::ObjectProxy* object_proxy3 =
+ bus->GetObjectProxy("org.chromium.TestService",
+ "/org/chromium/DifferentTestObject");
+ ASSERT_TRUE(object_proxy3);
+ EXPECT_NE(object_proxy1, object_proxy3);
+}
+
+TEST(BusTest, GetExportedObject) {
+ dbus::Bus::Options options;
+ scoped_refptr<dbus::Bus> bus = new dbus::Bus(options);
+
+ dbus::ExportedObject* object_proxy1 =
+ bus->GetExportedObject("org.chromium.TestService",
+ "/org/chromium/TestObject");
+ ASSERT_TRUE(object_proxy1);
+
+ // This should return the same object.
+ dbus::ExportedObject* object_proxy2 =
+ bus->GetExportedObject("org.chromium.TestService",
+ "/org/chromium/TestObject");
+ ASSERT_TRUE(object_proxy2);
+ EXPECT_EQ(object_proxy1, object_proxy2);
+
+ // This should not.
+ dbus::ExportedObject* object_proxy3 =
+ bus->GetExportedObject("org.chromium.TestService",
+ "/org/chromium/DifferentTestObject");
+ ASSERT_TRUE(object_proxy3);
+ EXPECT_NE(object_proxy1, object_proxy3);
+}
diff --git a/dbus/dbus.gyp b/dbus/dbus.gyp
index a310931..143f672 100644
--- a/dbus/dbus.gyp
+++ b/dbus/dbus.gyp
@@ -37,6 +37,7 @@
],
'sources': [
'../base/test/run_all_unittests.cc',
+ 'bus_unittest.cc',
'message_unittest.cc',
'end_to_end_async_unittest.cc',
'end_to_end_sync_unittest.cc',