Skip to content

Commit

Permalink
dbus: Make ObjectManager get objects after service is ready.
Browse files Browse the repository at this point in the history
Make dbus::ObjectManager automatically fetch managed objects
after the remote service becomes available. Also remove
Bus::GetManagedObjects(), which produces a large amount of
log spam due to some services not yet being ready. (It was
called by chromeos::DBusThreadManager and
bluez::BluezDBusManager at startup.)

BUG=636554

Review-Url: https://codereview.chromium.org/2239123002
Cr-Commit-Position: refs/heads/master@{#417405}
  • Loading branch information
derat authored and Commit bot committed Sep 8, 2016
1 parent 04803b4 commit 2ef7d0e
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 66 deletions.
5 changes: 0 additions & 5 deletions chromeos/dbus/dbus_thread_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,11 +216,6 @@ void DBusThreadManager::InitializeClients() {
GetSystemClockClient()->Init(GetSystemBus());
GetUpdateEngineClient()->Init(GetSystemBus());

// This must be called after the list of clients so they've each had a
// chance to register with their object g_dbus_thread_managers.
if (GetSystemBus())
GetSystemBus()->GetManagedObjects();

client_bundle_->SetupDefaultEnvironment();
}

Expand Down
7 changes: 0 additions & 7 deletions dbus/bus.cc
Original file line number Diff line number Diff line change
Expand Up @@ -400,13 +400,6 @@ void Bus::RemoveObjectManagerInternalHelper(
callback.Run();
}

void Bus::GetManagedObjects() {
for (ObjectManagerTable::iterator iter = object_manager_table_.begin();
iter != object_manager_table_.end(); ++iter) {
iter->second->GetManagedObjects();
}
}

bool Bus::Connect() {
// dbus_bus_get_private() and dbus_bus_get() are blocking calls.
AssertOnDBusThread();
Expand Down
6 changes: 0 additions & 6 deletions dbus/bus.h
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,6 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
const ObjectPath& object_path,
const base::Closure& callback);

// Instructs all registered object managers to retrieve their set of managed
// objects from their respective remote objects. There is no need to call this
// manually, this is called automatically by the D-Bus thread manager once
// implementation classes are registered.
virtual void GetManagedObjects();

// Shuts down the bus and blocks until it's done. More specifically, this
// function does the following:
//
Expand Down
1 change: 0 additions & 1 deletion dbus/mock_object_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class MockObjectManager : public ObjectManager {
MOCK_METHOD1(GetObjectProxy, ObjectProxy*(const ObjectPath&));
MOCK_METHOD2(GetProperties, PropertySet*(const ObjectPath&,
const std::string&));
MOCK_METHOD0(GetManagedObjects, void());

protected:
virtual ~MockObjectManager();
Expand Down
66 changes: 33 additions & 33 deletions dbus/object_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,35 +167,6 @@ void ObjectManager::CleanUp() {
match_rule_.clear();
}

void ObjectManager::InitializeObjects() {
DCHECK(bus_);
DCHECK(object_proxy_);
DCHECK(setup_success_);

// |object_proxy_| is no longer valid if the Bus was shut down before this
// call. Don't initiate any other action from the origin thread.
if (cleanup_called_)
return;

object_proxy_->ConnectToSignal(
kObjectManagerInterface,
kObjectManagerInterfacesAdded,
base::Bind(&ObjectManager::InterfacesAddedReceived,
weak_ptr_factory_.GetWeakPtr()),
base::Bind(&ObjectManager::InterfacesAddedConnected,
weak_ptr_factory_.GetWeakPtr()));

object_proxy_->ConnectToSignal(
kObjectManagerInterface,
kObjectManagerInterfacesRemoved,
base::Bind(&ObjectManager::InterfacesRemovedReceived,
weak_ptr_factory_.GetWeakPtr()),
base::Bind(&ObjectManager::InterfacesRemovedConnected,
weak_ptr_factory_.GetWeakPtr()));

GetManagedObjects();
}

bool ObjectManager::SetupMatchRuleAndFilter() {
DCHECK(bus_);
DCHECK(!setup_success_);
Expand Down Expand Up @@ -235,10 +206,39 @@ bool ObjectManager::SetupMatchRuleAndFilter() {
}

void ObjectManager::OnSetupMatchRuleAndFilterComplete(bool success) {
LOG_IF(WARNING, !success) << service_name_ << " " << object_path_.value()
<< ": Failed to set up match rule.";
if (success)
InitializeObjects();
if (!success) {
LOG(WARNING) << service_name_ << " " << object_path_.value()
<< ": Failed to set up match rule.";
return;
}

DCHECK(bus_);
DCHECK(object_proxy_);
DCHECK(setup_success_);

// |object_proxy_| is no longer valid if the Bus was shut down before this
// call. Don't initiate any other action from the origin thread.
if (cleanup_called_)
return;

object_proxy_->ConnectToSignal(
kObjectManagerInterface,
kObjectManagerInterfacesAdded,
base::Bind(&ObjectManager::InterfacesAddedReceived,
weak_ptr_factory_.GetWeakPtr()),
base::Bind(&ObjectManager::InterfacesAddedConnected,
weak_ptr_factory_.GetWeakPtr()));

object_proxy_->ConnectToSignal(
kObjectManagerInterface,
kObjectManagerInterfacesRemoved,
base::Bind(&ObjectManager::InterfacesRemovedReceived,
weak_ptr_factory_.GetWeakPtr()),
base::Bind(&ObjectManager::InterfacesRemovedConnected,
weak_ptr_factory_.GetWeakPtr()));

if (!service_name_owner_.empty())
GetManagedObjects();
}

// static
Expand Down
16 changes: 7 additions & 9 deletions dbus/object_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@
// object_manager_->UnregisterInterface(kInterface);
// }
//
// The D-Bus thread manager takes care of issuing the necessary call to
// GetManagedObjects() after the implementation classes have been set up.
// This class calls GetManagedObjects() asynchronously after the remote service
// becomes available and additionally refreshes managed objects after the
// service stops or restarts.
//
// The object manager interface class has one abstract method that must be
// implemented by the class to create Properties structures on demand. As well
Expand Down Expand Up @@ -238,17 +239,14 @@ class CHROME_DBUS_EXPORT ObjectManager
private:
friend class base::RefCountedThreadSafe<ObjectManager>;

// Connects the InterfacesAdded and InterfacesRemoved signals and calls
// GetManagedObjects. Called from OnSetupMatchRuleAndFilterComplete.
void InitializeObjects();

// Called from the constructor to add a match rule for PropertiesChanged
// signals on the DBus thread and set up a corresponding filter function.
// signals on the D-Bus thread and set up a corresponding filter function.
bool SetupMatchRuleAndFilter();

// Called on the origin thread once the match rule and filter have been set
// up. |success| is false, if an error occurred during set up; it's true
// otherwise.
// up. Connects the InterfacesAdded and InterfacesRemoved signals and
// refreshes objects if the service is available. |success| is false if an
// error occurred during setup and true otherwise.
void OnSetupMatchRuleAndFilterComplete(bool success);

// Called by dbus:: when a message is received. This is used to filter
Expand Down
5 changes: 0 additions & 5 deletions device/bluetooth/dbus/bluez_dbus_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,6 @@ void BluezDBusManager::InitializeClients() {
client_bundle_->bluetooth_media_client()->Init(GetSystemBus());
client_bundle_->bluetooth_media_transport_client()->Init(GetSystemBus());
client_bundle_->bluetooth_profile_manager_client()->Init(GetSystemBus());

// This must be called after the list of clients so they've each had a
// chance to register with their object g_dbus_thread_managers.
if (GetSystemBus())
GetSystemBus()->GetManagedObjects();
}

// static
Expand Down

0 comments on commit 2ef7d0e

Please sign in to comment.