gdata: Remove GDataDirectoryService::FindEntryByPathAndRunSync().

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 670 matching lines @@
         entry_proto.Pass(),
         base::Bind(&RunGetEntryInfoWithFilePathCallback,
                    callback, entry->GetFilePath()));
   } else {
     callback.Run(GDATA_FILE_ERROR_NOT_FOUND,
                  FilePath(),
                  scoped_ptr<GDataEntryProto>());
   }
 }
 
-void GDataFileSystem::FindEntryByPathAsyncOnUIThread(
-    const FilePath& search_file_path,
-    const FindEntryCallback& callback) {
+void GDataFileSystem::LoadFeedIfNeeded(const FileOperationCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!callback.is_null());
 
   if (directory_service_->origin() == INITIALIZING) {
     // If root feed is not initialized but the initialization process has
     // already started, add an observer to execute the remaining task after
     // the end of the initialization.
-    AddObserver(new InitialLoadObserver(
-        this,
-        base::Bind(&GDataFileSystem::FindEntryByPathSyncOnUIThread,
-                   ui_weak_ptr_,
-                   search_file_path,
-                   callback)));
+    AddObserver(new InitialLoadObserver(this,
+                                        base::Bind(callback, GDATA_FILE_OK)));
     return;
   } else if (directory_service_->origin() == UNINITIALIZED) {
     // Load root feed from this disk cache. Upon completion, kick off server
     // fetching.
     directory_service_->set_origin(INITIALIZING);
     feed_loader_->LoadFromCache(
         true,  // should_load_from_server
-        // This is the initial load, hence we'll notify when it's done.
-        base::Bind(&GDataFileSystem::FindAndNotifyInitialLoadFinished,
+        base::Bind(&GDataFileSystem::NotifyInitialLoadFinishedAndRun,
                    ui_weak_ptr_,
-                   search_file_path,
                    callback));
     return;
   }
 
   // Post a task to the same thread, rather than calling it here, as

[achuithb, 2012/08/17 19:36:49]

Could you please also add a comment here that the feed has already been loaded,
so we're just calling the callback asynchronously without doing anything.

We could also consider putting this in an else block so the early returns are
not necessary. I think it would make this more readable, but up to you. I try to
avoid early returns, but they are ok for error conditions.

[satorux1, 2012/08/17 20:35:30]

Added a comment.
Let's keep this as is for now.

-  // FindEntryByPath() is asynchronous.
+  // LoadFeedIfNeeded() is asynchronous.
   base::MessageLoopProxy::current()->PostTask(
       FROM_HERE,
-      base::Bind(&GDataFileSystem::FindEntryByPathSyncOnUIThread,
-                 ui_weak_ptr_,
-                 search_file_path,
-                 callback));
-}
-
-void GDataFileSystem::FindEntryByPathSyncOnUIThread(
-    const FilePath& search_file_path,
-    const FindEntryCallback& callback) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(!callback.is_null());
-
-  directory_service_->FindEntryByPathAndRunSync(search_file_path, callback);
+      base::Bind(callback, GDATA_FILE_OK));
 }
 
 void GDataFileSystem::TransferFileFromRemoteToLocal(
     const FilePath& remote_src_file_path,
     const FilePath& local_dest_file_path,
     const FileOperationCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!callback.is_null());
 
   GetFileByPath(remote_src_file_path,
@@ skipping 1142 matching lines @@
       params.get_content_callback);
 }
 
 void GDataFileSystem::GetEntryInfoByPath(const FilePath& file_path,
                                          const GetEntryInfoCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
          BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(!callback.is_null());
 
   RunTaskOnUIThread(
-      base::Bind(&GDataFileSystem::GetEntryInfoByPathAsyncOnUIThread,
+      base::Bind(&GDataFileSystem::GetEntryInfoByPathOnUIThread,
                  ui_weak_ptr_,
                  file_path,
                  CreateRelayCallback(callback)));
 }
 
-void GDataFileSystem::GetEntryInfoByPathAsyncOnUIThread(
+void GDataFileSystem::GetEntryInfoByPathOnUIThread(
     const FilePath& file_path,
     const GetEntryInfoCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!callback.is_null());
 
-  FindEntryByPathAsyncOnUIThread(
-      file_path,
-      base::Bind(&GDataFileSystem::OnGetEntryInfo,
+  LoadFeedIfNeeded(
+      base::Bind(&GDataFileSystem::GetEntryInfoByPathOnUIThreadAfterLoad,
                  ui_weak_ptr_,
+                 file_path,
                  callback));
 }
 
-void GDataFileSystem::OnGetEntryInfo(const GetEntryInfoCallback& callback,
-                                    GDataFileError error,
-                                    GDataEntry* entry) {
+void GDataFileSystem::GetEntryInfoByPathOnUIThreadAfterLoad(
+    const FilePath& file_path,
+    const GetEntryInfoCallback& callback,
+    GDataFileError error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!callback.is_null());
 
   if (error != GDATA_FILE_OK) {
     callback.Run(error, scoped_ptr<GDataEntryProto>());
     return;
   }
-  DCHECK(entry);
 
-  scoped_ptr<GDataEntryProto> entry_proto(new GDataEntryProto);
-  entry->ToProtoFull(entry_proto.get());
+  directory_service_->GetEntryInfoByPath(
+      file_path,
+      base::Bind(&GDataFileSystem::GetEntryInfoByPathOnUIThreadAfterGetEntry,
+                 ui_weak_ptr_,
+                 callback));
+}
+
+void GDataFileSystem::GetEntryInfoByPathOnUIThreadAfterGetEntry(
+    const GetEntryInfoCallback& callback,
+    GDataFileError error,
+    scoped_ptr<GDataEntryProto> entry_proto) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!callback.is_null());
+
+  if (error != GDATA_FILE_OK) {
+    callback.Run(error, scoped_ptr<GDataEntryProto>());
+    return;
+  }
+  DCHECK(entry_proto.get());
 
   CheckLocalModificationAndRun(entry_proto.Pass(), callback);
 }
 
 void GDataFileSystem::ReadDirectoryByPath(
     const FilePath& file_path,
     const ReadDirectoryWithSettingCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
          BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(!callback.is_null());
 
   RunTaskOnUIThread(
-      base::Bind(&GDataFileSystem::ReadDirectoryByPathAsyncOnUIThread,
+      base::Bind(&GDataFileSystem::ReadDirectoryByPathOnUIThread,
                  ui_weak_ptr_,
                  file_path,
                  CreateRelayCallback(callback)));
 }
 
-void GDataFileSystem::ReadDirectoryByPathAsyncOnUIThread(
+void GDataFileSystem::ReadDirectoryByPathOnUIThread(
     const FilePath& file_path,
     const ReadDirectoryWithSettingCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!callback.is_null());
 
-  FindEntryByPathAsyncOnUIThread(
+  LoadFeedIfNeeded(
+      base::Bind(&GDataFileSystem::ReadDirectoryByPathOnUIThreadAfterLoad,
+                 ui_weak_ptr_,
+                 file_path,
+                 callback));
+}
+
+void GDataFileSystem::ReadDirectoryByPathOnUIThreadAfterLoad(
+    const FilePath& file_path,
+    const ReadDirectoryWithSettingCallback& callback,
+    GDataFileError error) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!callback.is_null());
+
+  if (error != GDATA_FILE_OK) {
+    callback.Run(error,
+                 hide_hosted_docs_,
+                 scoped_ptr<GDataEntryProtoVector>());
+    return;
+  }
+
+  directory_service_->ReadDirectoryByPath(
       file_path,
-      base::Bind(&GDataFileSystem::OnReadDirectory,
+      base::Bind(&GDataFileSystem::ReadDirectoryByPathOnUIThreadAfterRead,
                  ui_weak_ptr_,
                  callback));
 }
 
-void GDataFileSystem::OnReadDirectory(
+void GDataFileSystem::ReadDirectoryByPathOnUIThreadAfterRead(
     const ReadDirectoryWithSettingCallback& callback,
     GDataFileError error,
-    GDataEntry* entry) {
+    scoped_ptr<GDataEntryProtoVector> entries) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!callback.is_null());
 
   if (error != GDATA_FILE_OK) {
-    if (!callback.is_null())
-      callback.Run(error,
-                   hide_hosted_docs_,
-                   scoped_ptr<GDataEntryProtoVector>());
+    callback.Run(error,
+                 hide_hosted_docs_,
+                 scoped_ptr<GDataEntryProtoVector>());
     return;
   }
-  DCHECK(entry);
+  DCHECK(entries.get());

[achuithb, 2012/08/17 19:36:49]

Is this DCHECK valid for empty directories?

[satorux1, 2012/08/17 20:35:30]

Yes. added a comment.

[achuithb, 2012/08/17 20:52:05]

nit: s/emptry/empty

 
-  GDataDirectory* directory = entry->AsGDataDirectory();
-  if (!directory) {
-    if (!callback.is_null())
-      callback.Run(GDATA_FILE_ERROR_NOT_FOUND,
-                   hide_hosted_docs_,
-                   scoped_ptr<GDataEntryProtoVector>());
-    return;
-  }
-
-  scoped_ptr<GDataEntryProtoVector> entries(directory->ToProtoVector());
-
-  if (!callback.is_null())
-    callback.Run(GDATA_FILE_OK, hide_hosted_docs_, entries.Pass());
+  callback.Run(GDATA_FILE_OK, hide_hosted_docs_, entries.Pass());
 }
 
 void GDataFileSystem::RequestDirectoryRefresh(const FilePath& file_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
          BrowserThread::CurrentlyOn(BrowserThread::IO));
   RunTaskOnUIThread(
       base::Bind(&GDataFileSystem::RequestDirectoryRefreshOnUIThread,
                  ui_weak_ptr_,
                  file_path));
 }
@@ skipping 832 matching lines @@
 
 void GDataFileSystem::NotifyFileSystemToBeUnmounted() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DVLOG(1) << "File System is to be unmounted";
   // Notify the observers that the file system is being unmounted.
   FOR_EACH_OBSERVER(GDataFileSystemInterface::Observer, observers_,
                     OnFileSystemBeingUnmounted());
 }
 
-void GDataFileSystem::FindAndNotifyInitialLoadFinished(
-    const FilePath& search_file_path,
-    const FindEntryCallback& callback,
+void GDataFileSystem::NotifyInitialLoadFinishedAndRun(
+    const FileOperationCallback& callback,
     GDataFileError error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!callback.is_null());
 
-  // TODO(satorux): Remove this: crbug.com/141196.
-  directory_service_->FindEntryByPathAndRunSync(search_file_path, callback);
-
-  DVLOG(1) << "RunAndNotifyInitialLoadFinished";
-
   // Notify the observers that root directory has been initialized.
   FOR_EACH_OBSERVER(GDataFileSystemInterface::Observer, observers_,
                     OnInitialLoadFinished());
+
+  callback.Run(error);
 }
 
 GDataFileError GDataFileSystem::AddNewDirectory(
     const FilePath& directory_path, base::Value* entry_value) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (!entry_value)
     return GDATA_FILE_ERROR_FAILED;
 
   scoped_ptr<DocumentEntry> doc_entry(DocumentEntry::CreateFrom(*entry_value));
@@ skipping 585 matching lines @@
     return;
   }
 
   PlatformFileInfoProto entry_file_info;
   GDataEntry::ConvertPlatformFileInfoToProto(*file_info, &entry_file_info);
   *entry_proto->mutable_file_info() = entry_file_info;
   callback.Run(GDATA_FILE_OK, entry_proto.Pass());
 }
 
 }  // namespace gdata