Eliminate getting disk metadata for floppy drives.

chrome/browser/storage_monitor/volume_mount_watcher_win.cc

 // 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/storage_monitor/volume_mount_watcher_win.h"
 
 #include <windows.h>
 #include <dbt.h>
 #include <fileapi.h>
 
 #include "base/stl_util.h"
 #include "base/string_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/storage_monitor/media_device_notifications_utils.h"
 #include "chrome/browser/storage_monitor/media_storage_util.h"
 #include "content/public/browser/browser_thread.h"
 
 using content::BrowserThread;
 
 namespace {
 
 const DWORD kMaxPathBufLen = MAX_PATH + 1;
 
+string16 QueryDevicePath(const string16& mount_point) {
+  string16 device = mount_point;
+  if (EndsWith(mount_point, L"\\", false))
+    device = mount_point.substr(0, device.length() - 1);
+  string16 device_path;
+  if (!QueryDosDevice(device.c_str(), WriteInto(&device_path, kMaxPathBufLen),
+                      kMaxPathBufLen))
+    return string16();
+  return device_path;
+}
+
 bool IsRemovable(const string16& mount_point) {

[vandebo (ex-Chrome), 2013/03/06 00:38:26]

Instead of splitting this into two functions that basically do the same thing,
how about changing it to DeviceType GetDeviceType(...); enum DeviceType {
Floppy, Removable, Fixed}; ?

Line 94 can then set the removable boolean as well as skip the rest if it's a
floppy and line 158 can test for exactly removable (which was the old definition
of IsRemovable).

[Greg Billock, 2013/03/06 01:40:03]

I like that plan.

   if (GetDriveType(mount_point.c_str()) != DRIVE_REMOVABLE)
     return false;
 
   // We don't consider floppy disks as removable, so check for that.
-  string16 device = mount_point;
-  if (EndsWith(mount_point, L"\\", false))
-    device = mount_point.substr(0, device.length() - 1);
-  string16 device_path;
-  if (!QueryDosDevice(device.c_str(), WriteInto(&device_path, kMaxPathBufLen),
-                      kMaxPathBufLen))
-    return true;
+  string16 device_path = QueryDevicePath(mount_point);
   return device_path.find(L"Floppy") == string16::npos;
 }
 
+bool IsFloppyDrive(const string16& mount_point) {
+  string16 device_path = QueryDevicePath(mount_point);
+  return device_path.find(L"Floppy") != string16::npos;
+}
+
 // Returns 0 if the devicetype is not volume.
 uint32 GetVolumeBitMaskFromBroadcastHeader(LPARAM data) {
   DEV_BROADCAST_VOLUME* dev_broadcast_volume =
       reinterpret_cast<DEV_BROADCAST_VOLUME*>(data);
   if (dev_broadcast_volume->dbcv_devicetype == DBT_DEVTYP_VOLUME)
     return dev_broadcast_volume->dbcv_unitmask;
   return 0;
 }
 
 // Returns true if |data| represents a logical volume structure.
@@ skipping 30 matching lines @@
                          kMaxPathBufLen)) {
     return false;
   }
 
   if (removable)
     *removable = IsRemovable(mount_point);
 
   if (device_location)
     *device_location = mount_point;
 
+  // If we're adding a floppy drive, return without querying any more
+  // drive metadata -- it will cause the floppy drive to seek.
+  if (IsFloppyDrive(device_path))
+    return true;
+
   if (volume_size)
     *volume_size = GetVolumeSize(mount_point);
 
   if (unique_id) {
     string16 guid;
     if (!GetVolumeNameForVolumeMountPoint(mount_point.c_str(),
                                           WriteInto(&guid, kMaxPathBufLen),
                                           kMaxPathBufLen)) {
       return false;
     }
@@ skipping 33 matching lines @@
                                        kMaxPathBufLen);
   if (find_handle == INVALID_HANDLE_VALUE)
     return result;
 
   while (true) {
     string16 volume_path;
     DWORD return_count;
     if (GetVolumePathNamesForVolumeName(volume_name.c_str(),
                                         WriteInto(&volume_path, kMaxPathBufLen),
                                         kMaxPathBufLen, &return_count)) {
       if (IsRemovable(volume_path))

[vandebo (ex-Chrome), 2013/03/06 00:38:26]

This call site still wants the old notion of removable.

[Greg Billock, 2013/03/06 01:40:03]

Yes. I think we can accommodate that.

         result.push_back(base::FilePath(volume_path));
     } else {
       DPLOG(ERROR);
     }
     if (!FindNextVolume(find_handle, WriteInto(&volume_name, kMaxPathBufLen),
                         kMaxPathBufLen)) {
       if (GetLastError() != ERROR_NO_MORE_FILES)
         DPLOG(ERROR);
       break;
     }
@@ skipping 235 matching lines @@
   // If the device isn't type removable (like a CD), it won't be there.
   if (device_info == device_metadata_.end())
     return;
 
   if (notifications_)
     notifications_->ProcessDetach(device_info->second.device_id);
   device_metadata_.erase(device_info);
 }
 
 }  // namespace chrome