Make ExportedObject and ObjectProxy own Bus as scoped_refptr.

Make ExportedObject and ObjectProxy own Bus as scoped_refptr.

They should own Bus as scoped_refptr, rather than raw pointer.
Otherwise, they may reference |bus_| after Bus is deleted.

I know this is convoluted. I'm planning to minimize use of
scoped_refptr from dbus/* but this change is necessary until then.

TEST=run dbus_unittest under valgrind and confirm no memory leaks
BUG=chromium-os:21379
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=105222

[stevenjb, 2011-10-10 22:09:17]

I may need to go over this in person with you so that I can understand this, but
it seems like:
* dbus::Bus holds a reference to ExportedObject in exported_object_table_
* I don't see anyplace where that reference is removed (except implicitly in
~dbus::Bus())
* ExportedObject now holds a reference to dbus::Bus.
* I don't see anyplace where ExportedObject.bus_ is unreferenced

So, how will dbus::Bus get unreferenced and deleted?

http://codereview.chromium.org/8201023/diff/1/dbus/bus.cc
File dbus/bus.cc (right):

http://codereview.chromium.org/8201023/diff/1/dbus/bus.cc#newcode298
dbus/bus.cc:298: // reference.
nit: s/here than/here rather than/
nit: s/reference/references

[satorux1, 2011-10-11 00:05:49]

On 2011/10/10 22:09:17, Steven Bennetts wrote:
> I may need to go over this in person with you so that I can understand this,
but
> it seems like:
> * dbus::Bus holds a reference to ExportedObject in exported_object_table_
> * I don't see anyplace where that reference is removed (except implicitly in
> ~dbus::Bus())
> * ExportedObject now holds a reference to dbus::Bus.
> * I don't see anyplace where ExportedObject.bus_ is unreferenced
> 
> So, how will dbus::Bus get unreferenced and deleted?

Yes, this is tricky.

object_proxy_table_.clear() and  exported_object_table_.clear() are introduced
in ShutdownAndBlock() in this patch to solve this problem. Let me explain with
an example:

- bus is created (bus's refcount becomes 1)
- exported object is created from the bus (exported object's refcount becomes 1)
(bus's refcount becomes 2, as it's referenced by exported object)
- ShutdownAndBlock() is called (exported object refcount becomes 0) (bus's
refcount becomes 1 as exported object is now gone)
- bus is deleted (bus's refcount becomes 0)

It was tricky so I confirmed that there was no leak with valgrind, and there was
leak reported if I don't add exported_object_table_.clear() in
ShutdownAndBlock().

> 
> http://codereview.chromium.org/8201023/diff/1/dbus/bus.cc
> File dbus/bus.cc (right):
> 
> http://codereview.chromium.org/8201023/diff/1/dbus/bus.cc#newcode298
> dbus/bus.cc:298: // reference.
> nit: s/here than/here rather than/
> nit: s/reference/references

[satorux1, 2011-10-11 17:24:33]

forgot to publish this.

http://codereview.chromium.org/8201023/diff/1/dbus/bus.cc
File dbus/bus.cc (right):

http://codereview.chromium.org/8201023/diff/1/dbus/bus.cc#newcode298
dbus/bus.cc:298: // reference.
On 2011/10/10 22:09:17, Steven Bennetts wrote:
> nit: s/here than/here rather than/
> nit: s/reference/references

Done.

[stevenjb, 2011-10-11 18:35:17]

OK, that makes sense. I think :)
LGTM

[commit-bot: I haz the power, 2011-10-12 21:41:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/satorux@chromium.org/8201023/5001

[commit-bot: I haz the power, 2011-10-13 00:45:30]

Change committed as 105222