gdata cleanup: Make GDataFileSystem::AddUploadedFile() take scoped_ptr<DocumentEntry>.

chrome/browser/chromeos/gdata/gdata_file_system.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/chromeos/gdata/gdata_file_system.h"
 
 #include <set>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 215 matching lines @@
     const std::string& /* md5 */) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (!callback.is_null())
     callback.Run();
 }
 
 // Helper function called upon completion of AddUploadFile invoked by
 // OnTransferCompleted.
 void OnAddUploadFileCompleted(
     const FileOperationCallback& callback,
-    base::PlatformFileError error,
-    scoped_ptr<UploadFileInfo> /* upload_file_info */) {
+    base::PlatformFileError error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (!callback.is_null())
     callback.Run(error);
 }
 
 // The class to wait for the initial load of root feed and runs the callback
 // after the initialization.
 class InitialLoadObserver : public GDataFileSystemInterface::Observer {
  public:
   InitialLoadObserver(GDataFileSystemInterface* file_system,
@@ skipping 910 matching lines @@
 }
 
 void GDataFileSystem::OnTransferCompleted(
     const FileOperationCallback& callback,
     base::PlatformFileError error,
     scoped_ptr<UploadFileInfo> upload_file_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(upload_file_info.get());
 
   if (error == base::PLATFORM_FILE_OK && upload_file_info->entry.get()) {
-    // Save a local copy of the UploadFileInfo pointer. Depending on order of
-    // argument evaluation, base::Passed() may invalidate the scoped pointer
-    // |upload_file_info| before it can be dereferenced to access its members.
-    const UploadFileInfo* upload_file_info_ptr = upload_file_info.get();

[satorux1, 2012/07/11 19:17:16]

glad to see this brittle piece to be gone.

     AddUploadedFile(UPLOAD_NEW_FILE,
-                    upload_file_info_ptr->gdata_path.DirName(),
-                    upload_file_info_ptr->entry.get(),
-                    upload_file_info_ptr->file_path,
+                    upload_file_info->gdata_path.DirName(),
+                    upload_file_info->entry.Pass(),
+                    upload_file_info->file_path,
                     GDataCache::FILE_OPERATION_COPY,
-                    base::Bind(&OnAddUploadFileCompleted,
-                               callback,
-                               error,
-                               base::Passed(&upload_file_info)));
+                    base::Bind(&OnAddUploadFileCompleted, callback, error));
   } else if (!callback.is_null()) {
     callback.Run(error);
   }
 }
 
 void GDataFileSystem::Copy(const FilePath& src_file_path,
                            const FilePath& dest_file_path,
                            const FileOperationCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
          BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ skipping 1189 matching lines @@
     scoped_ptr<UploadFileInfo> upload_file_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(upload_file_info.get());
 
   if (error != base::PLATFORM_FILE_OK) {
     if (!callback.is_null())
       callback.Run(error);
     return;
   }
 
-  // See comments in OnTransferCompleted() for why we copy this pointer.
-  const UploadFileInfo* upload_file_info_ptr = upload_file_info.get();

[satorux1, 2012/07/11 19:17:16]

this one too.

   AddUploadedFile(UPLOAD_EXISTING_FILE,
-                  upload_file_info_ptr->gdata_path.DirName(),
-                  upload_file_info_ptr->entry.get(),
-                  upload_file_info_ptr->file_path,
+                  upload_file_info->gdata_path.DirName(),
+                  upload_file_info->entry.Pass(),
+                  upload_file_info->file_path,
                   GDataCache::FILE_OPERATION_MOVE,
-                  base::Bind(&OnAddUploadFileCompleted,
-                             callback,
-                             error,
-                             base::Passed(&upload_file_info)));
+                  base::Bind(&OnAddUploadFileCompleted, callback, error));
 }
 
 void GDataFileSystem::GetAvailableSpace(
     const GetAvailableSpaceCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
          BrowserThread::CurrentlyOn(BrowserThread::IO));
   RunTaskOnUIThread(base::Bind(&GDataFileSystem::GetAvailableSpaceOnUIThread,
                                ui_weak_ptr_,
                                CreateRelayCallback(callback)));
 }
@@ skipping 1085 matching lines @@
   if (!parent_dir->RemoveEntry(entry))
     return base::PLATFORM_FILE_ERROR_NOT_FOUND;
 
   NotifyDirectoryChanged(parent_dir->GetFilePath());
   return base::PLATFORM_FILE_OK;
 }
 
 void GDataFileSystem::AddUploadedFile(
     UploadMode upload_mode,
     const FilePath& virtual_dir_path,
-    DocumentEntry* entry,
+    scoped_ptr<DocumentEntry> entry,
     const FilePath& file_content_path,
     GDataCache::FileOperationType cache_operation,
     const base::Closure& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Post a task to the same thread, rather than calling it here, as
   // AddUploadedFile() is asynchronous.
   base::MessageLoopProxy::current()->PostTask(
       FROM_HERE,
       base::Bind(&GDataFileSystem::AddUploadedFileOnUIThread,
                  ui_weak_ptr_,
                  upload_mode,
                  virtual_dir_path,
-                 entry,
+                 base::Passed(&entry),
                  file_content_path,
                  cache_operation,
                  callback));
 }
 
 void GDataFileSystem::AddUploadedFileOnUIThread(
     UploadMode upload_mode,
     const FilePath& virtual_dir_path,
-    DocumentEntry* entry,
+    scoped_ptr<DocumentEntry> entry,
     const FilePath& file_content_path,
     GDataCache::FileOperationType cache_operation,
     const base::Closure& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!callback.is_null());
 
-  if (!entry) {
+  if (!entry.get()) {
     NOTREACHED();
     callback.Run();
     return;
   }
 
   GDataEntry* dir_entry = GetGDataEntryByPath(virtual_dir_path);
   if (!dir_entry) {
     callback.Run();
     return;
   }
 
   GDataDirectory* parent_dir  = dir_entry->AsGDataDirectory();
   if (!parent_dir) {
     callback.Run();
     return;
   }
 
   scoped_ptr<GDataEntry> new_entry(
-      GDataEntry::FromDocumentEntry(parent_dir, entry, root_.get()));
+      GDataEntry::FromDocumentEntry(parent_dir, entry.get(), root_.get()));
   if (!new_entry.get()) {
     callback.Run();
     return;
   }
 
   if (upload_mode == UPLOAD_EXISTING_FILE) {
     // Remove an existing entry, which should be present.
     GDataEntry* existing_entry = root_->GetEntryByResourceId(
         new_entry->resource_id());
     if (existing_entry &&
@@ skipping 354 matching lines @@
   // must go through here. Removes the |file_path| from the remembered set so
   // that subsequent operations can open the file again.
   open_files_.erase(file_path);
 
   // Then invokes the user-supplied callback function.
   if (!callback.is_null())
     callback.Run(result);
 }
 
 }  // namespace gdata