Redesigned and refactored ScopedMTPDeviceMapEntry, MTPDeviceMapService & MTPDeviceDelegate classes.

webkit/fileapi/media/device_media_file_util.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 "webkit/fileapi/media/device_media_file_util.h"
 
 #include "base/file_util.h"
 #include "base/message_loop_proxy.h"
 #include "webkit/fileapi/file_system_operation_context.h"
 #include "webkit/fileapi/file_system_url.h"
@@ skipping 45 matching lines @@
     bool exclusive,
     bool recursive) {
   return base::PLATFORM_FILE_ERROR_SECURITY;
 }
 
 PlatformFileError DeviceMediaFileUtil::GetFileInfo(
     FileSystemOperationContext* context,
     const FileSystemURL& url,
     PlatformFileInfo* file_info,
     FilePath* platform_path) {
-  DCHECK(context->mtp_device_delegate());
+  if (!context->mtp_device_delegate().get())
+    return base::PLATFORM_FILE_ERROR_NOT_FOUND;

[Lei Zhang, 2012/11/21 01:33:30]

Perhaps PLATFORM_FILE_ERROR_FAILED is more appropriate? Ditto below.

[kmadhusu, 2012/11/21 04:09:53]

kinuko preferred base::PLATFORM_FILE_ERROR_NOT_FOUND in CreateSnapshotFile. So,
I used the same in GetFileInfo().

If the extension requesting the file information has valid file system access
permissions, then the only reason |mtp_device_delgate().get()| can be NULL is
when the attached MTP device is no longer available to complete the given
request. Therefore, base::PLATFORM_FILE_ERROR_NOT_FOUND seems more relevant to
the context rather than a generic base::PLATFORM_FILE_ERROR_FAILED message.

+
   PlatformFileError error =
       context->mtp_device_delegate()->GetFileInfo(url.path(), file_info);
   if (error != base::PLATFORM_FILE_OK)
     return error;
 
   if (file_info->is_directory ||
       context->media_path_filter()->Match(url.path()))
     return base::PLATFORM_FILE_OK;
   return base::PLATFORM_FILE_ERROR_NOT_FOUND;
 }
 
 scoped_ptr<FileSystemFileUtil::AbstractFileEnumerator>
     DeviceMediaFileUtil::CreateFileEnumerator(
         FileSystemOperationContext* context,
         const FileSystemURL& url,
         bool recursive) {
-  DCHECK(context->mtp_device_delegate());
+  if (!context->mtp_device_delegate().get())
+    return make_scoped_ptr(new FileSystemFileUtil::EmptyFileEnumerator())
+        .PassAs<FileSystemFileUtil::AbstractFileEnumerator>();

[kinuko, 2012/11/20 06:25:33]

nit: hmm... if we're just creating a new scoped_ptr (but not passing rvalues)
just using scoped_ptr<foo>(new bar) would look simpler.

scoped_ptr<FileSystemFileUtil::AbstractFileEnumerator>(new ...)

[kmadhusu, 2012/11/21 04:09:53]

Done.

   return make_scoped_ptr(new FilteringFileEnumerator(
       context->mtp_device_delegate()->CreateFileEnumerator(url.path(),
                                                            recursive),
       context->media_path_filter()))
       .PassAs<FileSystemFileUtil::AbstractFileEnumerator>();
 }
 
 PlatformFileError DeviceMediaFileUtil::GetLocalFilePath(
     FileSystemOperationContext* context,
     const FileSystemURL& file_system_url,
@@ skipping 12 matching lines @@
 PlatformFileError DeviceMediaFileUtil::Truncate(
     FileSystemOperationContext* context,
     const FileSystemURL& url,
     int64 length) {
   return base::PLATFORM_FILE_ERROR_FAILED;
 }
 
 bool DeviceMediaFileUtil::IsDirectoryEmpty(
     FileSystemOperationContext* context,
     const FileSystemURL& url) {
-  DCHECK(context->mtp_device_delegate());
+  if (!context->mtp_device_delegate().get())
+    return false;
+
   scoped_ptr<AbstractFileEnumerator> enumerator(
       CreateFileEnumerator(context, url, false));
   FilePath path;
   while (!(path = enumerator->Next()).empty()) {
     if (enumerator->IsDirectory() ||
         context->media_path_filter()->Match(path))
       return false;
   }
   return true;
 }
@@ skipping 27 matching lines @@
 
 base::PlatformFileError DeviceMediaFileUtil::CreateSnapshotFile(
     FileSystemOperationContext* context,
     const FileSystemURL& url,
     base::PlatformFileInfo* file_info,
     FilePath* local_path,
     SnapshotFilePolicy* snapshot_policy) {
   DCHECK(file_info);
   DCHECK(local_path);
   DCHECK(snapshot_policy);
-  DCHECK(context->mtp_device_delegate());
+  if (!context->mtp_device_delegate().get())
+    return base::PLATFORM_FILE_ERROR_NOT_FOUND;
 
   // We return a temporary file as a snapshot.
   *snapshot_policy = FileSystemFileUtil::kSnapshotFileTemporary;
 
   // Create a temp file in "profile_path_/kDeviceMediaFileUtilTempDir".
   FilePath isolated_media_file_system_dir_path =
       profile_path_.Append(kDeviceMediaFileUtilTempDir);
   bool dir_exists = file_util::DirectoryExists(
       isolated_media_file_system_dir_path);
   if (!dir_exists &&
       !file_util::CreateDirectory(isolated_media_file_system_dir_path)) {
     LOG(WARNING) << "Could not create a directory for media snapshot file "
                  << isolated_media_file_system_dir_path.value();
     return base::PLATFORM_FILE_ERROR_FAILED;
   }
 
   bool file_created = file_util::CreateTemporaryFileInDir(
       isolated_media_file_system_dir_path, local_path);
   if (!file_created) {
     LOG(WARNING) << "Could not create a temporary file for media snapshot in "
                  << isolated_media_file_system_dir_path.value();
     return base::PLATFORM_FILE_ERROR_FAILED;
   }
   return context->mtp_device_delegate()->CreateSnapshotFile(
       url.path(), *local_path, file_info);
 }
 
 }  // namespace fileapi