Remove use of weakptr for RemovableDeviceNotificationsMac

chrome/browser/system_monitor/removable_device_notifications_mac.mm

 // Copyright (c) 2012 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 "chrome/browser/system_monitor/removable_device_notifications_mac.h"
 
 #include "content/public/browser/browser_thread.h"
 
 namespace chrome {
 
 namespace {
 
 static RemovableDeviceNotificationsMac*
     g_removable_device_notifications_mac = NULL;
 
 void GetDiskInfoAndUpdateOnFileThread(
-    const base::WeakPtr<RemovableDeviceNotificationsMac>& notifications,
+    const scoped_refptr<RemovableDeviceNotificationsMac>& notifications,
     base::mac::ScopedCFTypeRef<CFDictionaryRef> dict,
     RemovableDeviceNotificationsMac::UpdateType update_type) {
   DiskInfoMac info = DiskInfoMac::BuildDiskInfoOnFileThread(dict);
   if (info.device_id().empty())
     return;
 
   content::BrowserThread::PostTask(
       content::BrowserThread::UI,
       FROM_HERE,
       base::Bind(&RemovableDeviceNotificationsMac::UpdateDisk,
                  notifications,
                  info,
                  update_type));
 }
 
 void GetDiskInfoAndUpdate(
-    const base::WeakPtr<RemovableDeviceNotificationsMac>& notifications,
+    const scoped_refptr<RemovableDeviceNotificationsMac>& notifications,
     DADiskRef disk,
     RemovableDeviceNotificationsMac::UpdateType update_type) {
   base::mac::ScopedCFTypeRef<CFDictionaryRef> dict(DADiskCopyDescription(disk));
   content::BrowserThread::PostTask(
       content::BrowserThread::FILE,
       FROM_HERE,
       base::Bind(GetDiskInfoAndUpdateOnFileThread,
                  notifications,
                  dict,
                  update_type));
@@ skipping 95 matching lines @@
 
   return false;
 }
 
 // static
 void RemovableDeviceNotificationsMac::DiskAppearedCallback(
     DADiskRef disk,
     void* context) {
   RemovableDeviceNotificationsMac* notifications =
       static_cast<RemovableDeviceNotificationsMac*>(context);
-  GetDiskInfoAndUpdate(notifications->AsWeakPtr(),
+  GetDiskInfoAndUpdate(notifications,

[vandebo (ex-Chrome), 2012/09/18 22:41:56]

Does this work? raw pointer passed to function that cast const ref to refptr? 
Maybe the above functions should just take the raw pointer?

[sail, 2012/09/18 22:52:49]

Yep! When the raw pointer is passed to GetDiskInfoAndUpdate() it creates a new
scoped_refptr<> object. This object will ensure that the
RemovableDeviceNotificationsMac instance will stay alive while the task is
pending.
I think using refcounted object is better than raw pointers since it guarantees
that the instance will be alive when the task complets.

We could use the global object and not pass a pointer at all. I think this is
not as great because then that force this object to be a singleton.

[vandebo (ex-Chrome), 2012/09/18 23:03:47]

What I meant is that maybe GetDiskInfoAndUpdate could take a raw pointer.  Bind
will take a reference when it stores the value.  callback.h isn't very clear on
this point, but the way I understand things, if the type of one of the arguments
is refcount'able, then it takes a reference when it stores it for currying. I
don't think it has to be a scoped_refptr for that to work. By creating a
scoped_refptr in the argument list you're doing an extra ref unref that isn't
strictly needed.  But I could be wrong about this.
Agreed.

[sail, 2012/09/18 23:28:51]

Ahh, I see what you mean.
I tried simply removing scoped_refptr<> from GetDiskInfoAndUpdate() but it
didn't work. It seems like base::bind() isn't smart enough to automatically do
this.

                        disk,
                        UPDATE_DEVICE_ADDED);
 }
 
 // static
 void RemovableDeviceNotificationsMac::DiskDisappearedCallback(
     DADiskRef disk,
     void* context) {
   RemovableDeviceNotificationsMac* notifications =
       static_cast<RemovableDeviceNotificationsMac*>(context);
-  GetDiskInfoAndUpdate(notifications->AsWeakPtr(),
+  GetDiskInfoAndUpdate(notifications,
                        disk,
                        UPDATE_DEVICE_REMOVED);
 }
 
 // static
 void RemovableDeviceNotificationsMac::DiskDescriptionChangedCallback(
     DADiskRef disk,
     CFArrayRef keys,
     void *context) {
   RemovableDeviceNotificationsMac* notifications =
       static_cast<RemovableDeviceNotificationsMac*>(context);
-  GetDiskInfoAndUpdate(notifications->AsWeakPtr(),
+  GetDiskInfoAndUpdate(notifications,
                        disk,
                        UPDATE_DEVICE_CHANGED);
 }
 
 bool RemovableDeviceNotificationsMac::ShouldPostNotificationForDisk(
     const DiskInfoMac& info) const {
   // Only post notifications about disks that have no empty fields and
   // are removable.
   return !info.bsd_name().empty() &&
          !info.device_id().empty() &&
@@ skipping 10 matching lines @@
       it = disk_info_map_.begin(); it != disk_info_map_.end(); ++it) {
     if (it->second.mount_point() == mount_point) {
       *info = it->second;
       return true;
     }
   }
   return false;
 }
 
 }  // namespace chrome