Media Galleries: Fix PTP Devices on Windows

chrome/browser/media_galleries/win/mtp_device_operations_util.cc

 // Copyright (c) 2013 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/media_galleries/win/mtp_device_operations_util.h"
 
 #include <portabledevice.h>
 
 #include <algorithm>
 
@@ skipping 113 matching lines @@
                          base::Time* last_modified_time) {
   DCHECK(properties_values);
   DCHECK(last_modified_time);
   base::win::ScopedPropVariant last_modified_date;
   HRESULT hr = properties_values->GetValue(WPD_OBJECT_DATE_MODIFIED,
                                            last_modified_date.Receive());
   if (FAILED(hr))
     return false;
 
   bool last_modified_time_set = (last_modified_date.get().vt == VT_DATE);
+
+  // Some PTP devices don't provide an mtime. Try using the ctime instead.

[Lei Zhang, 2014/06/18 00:54:26]

s/an mtime/a mtime/

[tommycli, 2014/06/18 16:28:50]

I think "an" for vowel sounds?

+  if (!last_modified_time_set) {
+    last_modified_date.Reset();
+    HRESULT hr = properties_values->GetValue(WPD_OBJECT_DATE_CREATED,
+                                             last_modified_date.Receive());
+    if (FAILED(hr))
+      return false;
+
+    last_modified_time_set = (last_modified_date.get().vt == VT_DATE);
+  }
+
   if (last_modified_time_set) {
     SYSTEMTIME system_time;
     FILETIME file_time;
     if (VariantTimeToSystemTime(last_modified_date.get().date, &system_time) &&
         SystemTimeToFileTime(&system_time, &file_time)) {
       *last_modified_time = base::Time::FromFileTime(file_time);
     } else {
       last_modified_time_set = false;
     }
   }
@@ skipping 10 matching lines @@
   HRESULT hr = properties_values->GetUnsignedLargeIntegerValue(WPD_OBJECT_SIZE,
                                                                &actual_size);
   bool success = SUCCEEDED(hr) && (actual_size <= kint64max);
   if (success)
     *size = static_cast<int64>(actual_size);
   return success;
 }
 
 // Gets the details of the object specified by the |object_id| given the media
 // transfer protocol |device|. On success, returns true and fills in |name|,
-// |is_directory|, |size| and |last_modified_time|.
+// |is_directory|, |size|. |last_modified_time| will be filled in if possible,
+// but failure to get it doesn't prevent success.
 bool GetObjectDetails(IPortableDevice* device,
                       const base::string16 object_id,
                       base::string16* name,
                       bool* is_directory,
                       int64* size,
                       base::Time* last_modified_time) {
   base::ThreadRestrictions::AssertIOAllowed();
   DCHECK(device);
   DCHECK(!object_id.empty());
   DCHECK(name);
@@ skipping 15 matching lines @@
                                          NULL,
                                          CLSCTX_INPROC_SERVER);
   if (FAILED(hr))
     return false;
 
   if (FAILED(properties_to_read->Add(WPD_OBJECT_CONTENT_TYPE)) ||
       FAILED(properties_to_read->Add(WPD_OBJECT_FORMAT)) ||
       FAILED(properties_to_read->Add(WPD_OBJECT_ORIGINAL_FILE_NAME)) ||
       FAILED(properties_to_read->Add(WPD_OBJECT_NAME)) ||
       FAILED(properties_to_read->Add(WPD_OBJECT_DATE_MODIFIED)) ||
+      FAILED(properties_to_read->Add(WPD_OBJECT_DATE_CREATED)) ||
       FAILED(properties_to_read->Add(WPD_OBJECT_SIZE)))
     return false;
 
   base::win::ScopedComPtr<IPortableDeviceValues> properties_values;
   hr = properties->GetValues(object_id.c_str(),
                              properties_to_read.get(),
                              properties_values.Receive());
   if (FAILED(hr))
     return false;
 
   *is_directory = IsDirectory(properties_values.get());
   *name = GetObjectName(properties_values.get());
   if (name->empty())
     return false;
 
   if (*is_directory) {
     // Directory entry does not have size and last modified date property key
     // values.
     *size = 0;
     *last_modified_time = base::Time();
     return true;
   }
-  return (GetObjectSize(properties_values.get(), size) &&
-          GetLastModifiedTime(properties_values.get(), last_modified_time));
+
+  // Try to get the last modified time, but don't fail if we can't.
+  GetLastModifiedTime(properties_values.get(), last_modified_time);

[Lei Zhang, 2014/06/18 00:54:26]

This is the only caller. Just change the return type to void if you are not
going to check it?

[tommycli, 2014/06/18 16:28:50]

Done. I also changed the method's flow a bit since we don't need to return
anything.

+
+  return GetObjectSize(properties_values.get(), size);
 }
 
 // Creates an MTP device object entry for the given |device| and |object_id|.
 // On success, returns true and fills in |entry|.
 bool GetMTPDeviceObjectEntry(IPortableDevice* device,
                              const base::string16& object_id,
                              MTPDeviceObjectEntry* entry) {
   base::ThreadRestrictions::AssertIOAllowed();
   DCHECK(device);
   DCHECK(!object_id.empty());
@@ skipping 165 matching lines @@
       object_entries.empty())
     return base::string16();
   // TODO(thestig): This DCHECK can fail. Multiple MTP objects can have
   // the same name. Handle the situation gracefully. Refer to crbug.com/169930
   // for more details.
   DCHECK_EQ(1U, object_entries.size());
   return object_entries[0].object_id;
 }
 
 }  // namespace media_transfer_protocol