contacts: Add ContactManagerInterface::GetWeakPtr().

contacts: Add ContactManagerInterface::GetWeakPtr().

The ContactManager singleton is destroyed in
PostMainMessageLoopRun(), which appears to run before the
(not-yet-checked-in) ContactProvider class is destroyed in
response to the user profile being destroyed.  This change
lets ContactProvider hold a weak pointer to ContactManager
so it can avoid accessing it during shutdown.

I'm also renaming a couple of references to "partial
updates" to "incremental updates" for consistency.

BUG=128805, 141877
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=154327

[satorux1, 2012-08-30 16:43:28]

sorry for the belated response. I have a question:

http://codereview.chromium.org/10896031/diff/2001/chrome/browser/chromeos/con...
File chrome/browser/chromeos/contacts/contact_manager.h (right):

http://codereview.chromium.org/10896031/diff/2001/chrome/browser/chromeos/con...
chrome/browser/chromeos/contacts/contact_manager.h:39: virtual
base::WeakPtr<ContactManagerInterface> GetWeakPtr() = 0;
This looks worrisome. I'm not sure if WeakPtr works nicely with derived classes.
There was a  thread about this topic:

[chromium-dev] Weak ptrs for derived types?

which I didn't follow. :~(

Could you educate me if use of WeakPtr in this way is acceptable or not.

[Daniel Erat, 2012-08-30 16:56:16]

http://codereview.chromium.org/10896031/diff/2001/chrome/browser/chromeos/con...
File chrome/browser/chromeos/contacts/contact_manager.h (right):

http://codereview.chromium.org/10896031/diff/2001/chrome/browser/chromeos/con...
chrome/browser/chromeos/contacts/contact_manager.h:39: virtual
base::WeakPtr<ContactManagerInterface> GetWeakPtr() = 0;
On 2012/08/30 16:43:29, satorux1 wrote:
> This looks worrisome. I'm not sure if WeakPtr works nicely with derived
classes.
> There was a  thread about this topic:
> 
> [chromium-dev] Weak ptrs for derived types?
> 
> which I didn't follow. :~(
> 
> Could you educate me if use of WeakPtr in this way is acceptable or not.

Thanks for pointing out that thread!

The original question there appeared to be about declaring a
WeakPtrFactory<Base> in Base (i.e. ContactManagerInterface) but then using it
from within Derived (i.e. ContactManager, ContactManagerStub) to vend
WeakPtr<Derived> objects.  There's some discussion about whether WeakPtr<Base>
should be downcastable to WeakPtr<Derived>.

I think that my usage is different: I'm declaring an abstract method in Base
(ContactManagerInterface) that returns WeakPtr<Base>, but Derived
(ContactManager, ContactManagerStub) creates its own WeakPtrFactory<Base>, and
there's never a need to downcast WeakPtr<Base> to WeakPtr<Derived>.

[satorux1, 2012-08-30 17:19:26]

LGTM with nits

http://codereview.chromium.org/10896031/diff/2001/chrome/browser/chromeos/con...
File chrome/browser/chromeos/contacts/contact_manager.cc (right):

http://codereview.chromium.org/10896031/diff/2001/chrome/browser/chromeos/con...
chrome/browser/chromeos/contacts/contact_manager.cc:41: weak_ptr_factory_(this)
{
ALLOW_THIS_IN_INITIALIZER_LIST ?

http://codereview.chromium.org/10896031/diff/2001/chrome/browser/chromeos/con...
File chrome/browser/chromeos/contacts/contact_manager_stub.cc (right):

http://codereview.chromium.org/10896031/diff/2001/chrome/browser/chromeos/con...
chrome/browser/chromeos/contacts/contact_manager_stub.cc:21:
weak_ptr_factory_(this) {
ALLOW_THIS_IN_INITIALIZER_LIST ?

[Daniel Erat, 2012-08-30 20:59:45]

http://codereview.chromium.org/10896031/diff/2001/chrome/browser/chromeos/con...
File chrome/browser/chromeos/contacts/contact_manager.cc (right):

http://codereview.chromium.org/10896031/diff/2001/chrome/browser/chromeos/con...
chrome/browser/chromeos/contacts/contact_manager.cc:41: weak_ptr_factory_(this)
{
On 2012/08/30 17:19:26, satorux1 wrote:
> ALLOW_THIS_IN_INITIALIZER_LIST ?

Done.

http://codereview.chromium.org/10896031/diff/2001/chrome/browser/chromeos/con...
File chrome/browser/chromeos/contacts/contact_manager_stub.cc (right):

http://codereview.chromium.org/10896031/diff/2001/chrome/browser/chromeos/con...
chrome/browser/chromeos/contacts/contact_manager_stub.cc:21:
weak_ptr_factory_(this) {
On 2012/08/30 17:19:26, satorux1 wrote:
> ALLOW_THIS_IN_INITIALIZER_LIST ?

Done.

[commit-bot: I haz the power, 2012-08-30 21:53:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/10896031/14001

[commit-bot: I haz the power, 2012-08-31 00:28:48]

Change committed as 154327