Implement RemovableDeviceNotifications for Mac

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 "base/system_monitor/system_monitor.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace chrome {
+
+namespace {
+
+void GetDiskInfoAndUpdateOnFileThead(

[vandebo (ex-Chrome), 2012/09/10 18:45:03]

nit: Thead -> Thread

[sail, 2012/09/10 21:58:43]

Done.

+    const base::WeakPtr<RemovableDeviceNotificationsMac>& notifications,
+    base::mac::ScopedCFTypeRef<CFDictionaryRef> dict,
+    bool should_remove) {
+  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,
+                 should_remove));
+}
+
+void GetDiskInfoAndUpdate(
+    const base::WeakPtr<RemovableDeviceNotificationsMac>& notifications,
+    DADiskRef disk,
+    bool should_remove) {
+  base::mac::ScopedCFTypeRef<CFDictionaryRef> dict(DADiskCopyDescription(disk));
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(GetDiskInfoAndUpdateOnFileThead,
+                 notifications,
+                 dict,
+                 should_remove));
+}
+
+}  // namespace
+
+RemovableDeviceNotificationsMac::RemovableDeviceNotificationsMac() {
+  session_.reset(DASessionCreate(NULL));
+  DASessionScheduleWithRunLoop(

[vandebo (ex-Chrome), 2012/09/10 18:45:03]

I assume this means you have to register and receive these events on the UI
thread?

[sail, 2012/09/10 21:58:43]

I think we can register on any thread that has a CFRunLoop attached.

The FILE thread doesn't have a CFRunLoop attached so I decided to just register
this on the UI thread instead.

+      session_, CFRunLoopGetCurrent(), kCFRunLoopCommonModes);
+
+  DARegisterDiskAppearedCallback(

[vandebo (ex-Chrome), 2012/09/10 18:45:03]

Does this send notifications for already attached devices?

[sail, 2012/09/10 21:58:43]

Yep! Added comment too.

+      session_,
+      kDADiskDescriptionMatchVolumeMountable,
+      DiskAppearedCallback,
+      this);
+  DARegisterDiskDisappearedCallback(
+      session_,
+      kDADiskDescriptionMatchVolumeMountable,
+      DiskDisappearedCallback,
+      this);
+  DARegisterDiskDescriptionChangedCallback(
+      session_,
+      kDADiskDescriptionMatchVolumeMountable,
+      kDADiskDescriptionWatchVolumePath,
+      DiskDescriptionChangedCallback,
+      this);
+}
+
+RemovableDeviceNotificationsMac::~RemovableDeviceNotificationsMac() {
+  DASessionUnscheduleFromRunLoop(
+      session_, CFRunLoopGetCurrent(), kCFRunLoopCommonModes);
+}
+
+void RemovableDeviceNotificationsMac::UpdateDisk(
+    const DiskInfoMac& info,
+    bool should_remove) {
+  std::map<std::string, DiskInfoMac>::iterator it =
+      disk_info_map_.find(info.device_id());
+  if (it != disk_info_map_.end()) {
+    if (ShouldPostNotificationForDisk(it->second)) {

[vandebo (ex-Chrome), 2012/09/10 18:45:03]

DCHECK(should_remove) ?

[sail, 2012/09/10 21:58:43]

I'm trying to handle the update case here.

For example, if the user renames the volume we get a DiskDescriptionChanged
callback.

In that case I post a Detached and Attached notification. The Attached
notification contains the new mount point.

+      base::SystemMonitor::Get()->ProcessRemovableStorageDetached(
+          it->second.device_id());
+    }
+  }
+
+  if (should_remove) {

[vandebo (ex-Chrome), 2012/09/10 18:45:03]

looks like line 80 and 87 could be merged?

[sail, 2012/09/10 21:58:43]

see above.

+    if (it != disk_info_map_.end())
+      disk_info_map_.erase(it);
+  } else {
+    disk_info_map_[info.device_id()] = info;
+    if (ShouldPostNotificationForDisk(info)) {
+      base::SystemMonitor::Get()->ProcessRemovableStorageAttached(
+          info.device_id(), info.display_name(), info.mount_point().value());
+    }
+  }
+}
+
+// static
+void RemovableDeviceNotificationsMac::DiskAppearedCallback(
+    DADiskRef disk,
+    void* context) {
+  RemovableDeviceNotificationsMac* notifications =
+      static_cast<RemovableDeviceNotificationsMac*>(context);
+  GetDiskInfoAndUpdate(notifications->AsWeakPtr(), disk, false);
+}
+
+// static
+void RemovableDeviceNotificationsMac::DiskDisappearedCallback(
+    DADiskRef disk,
+    void* context) {
+  RemovableDeviceNotificationsMac* notifications =
+      static_cast<RemovableDeviceNotificationsMac*>(context);
+  GetDiskInfoAndUpdate(notifications->AsWeakPtr(), disk, true);
+}
+
+// static
+void RemovableDeviceNotificationsMac::DiskDescriptionChangedCallback(
+    DADiskRef disk,
+    CFArrayRef keys,
+    void *context) {
+  RemovableDeviceNotificationsMac* notifications =
+      static_cast<RemovableDeviceNotificationsMac*>(context);
+  GetDiskInfoAndUpdate(notifications->AsWeakPtr(), disk, false);
+}
+
+bool RemovableDeviceNotificationsMac::ShouldPostNotificationForDisk(
+    const DiskInfoMac& info) const {
+  return !info.bsd_name().empty() &&
+         !info.device_id().empty() &&
+         !info.display_name().empty() &&
+         !info.mount_point().empty() &&
+         (info.type() == MediaStorageUtil::REMOVABLE_MASS_STORAGE_WITH_DCIM ||
+          info.type() == MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM);

[Nico, 2012/09/10 07:02:34]

the other file mentioned FIXED_MASS_FOO. that's not needed here?

[vandebo (ex-Chrome), 2012/09/10 18:45:03]

SystemMonitor only notifies about removable storage, so it shouldn't be needed
in this predicate.

[sail, 2012/09/10 21:58:43]

Yea, we only post notifications about removable storage. Added a comment.

[sail, 2012/09/10 21:58:43]

I didn't understand this comment.

Did you mean that I don't need the WITH_DCIM/NO_DCIM check here because
base::SystemMonitor already does this check?

[vandebo (ex-Chrome), 2012/09/10 22:53:05]

No, I was replying to Nico's comment.

+}
+
+}  // namespace chrome