gdata: Convert FindEntryByResourceIdSync() to asynchronous.

chrome/browser/chromeos/gdata/gdata_file_system.h

 // 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_GDATA_GDATA_FILE_SYSTEM_H_
 #define CHROME_BROWSER_CHROMEOS_GDATA_GDATA_FILE_SYSTEM_H_
 #pragma once
 
 #include <map>
 #include <string>
 #include <vector>
 
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/platform_file.h"
 #include "base/timer.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/sequenced_worker_pool.h"
-#include "chrome/browser/chromeos/gdata/find_entry_callback.h"
 #include "chrome/browser/chromeos/gdata/gdata_cache.h"
 #include "chrome/browser/chromeos/gdata/gdata_files.h"
 #include "chrome/browser/prefs/pref_change_registrar.h"
 #include "content/public/browser/notification_observer.h"
 
 namespace base {
 class WaitableEvent;
 }
 
 namespace gdata {
@@ skipping 76 matching lines @@
 
 // Used by GDataFileSystem::GetDocumentResourceIdOnBlockingPool to return
 // the resource ID read from a document JSON file on the local file system.
 typedef base::Callback<void(const std::string& resource_id)>
     GetDocumentResourceIdCallback;
 
 // Callback for GetCacheState operation.
 typedef base::Callback<void(base::PlatformFileError error,
                             int cache_state)> GetCacheStateCallback;
 
+// Callback used to find a directory element for file system updates.
+void ReadOnlyFindEntryCallback(GDataEntry** out,

[achuithb, 2012/06/11 21:48:41]

Is this only used in 2 places now? GDataFileSystem::GetGDataEntryByPath and the
unittest? I think you could probably move it out of the header file and have the
unit-test call a method on GDataFileSystem?

[hshi1, 2012/06/11 22:42:21]

Yes this is only called in GDataFileSystem::GetGDataEntryByPath and unittest.

Can you elaborate what do you mean by moving it out of the header file? This is
already put in the gdata_file_system.h, where else shall I put it?

[achuithb, 2012/06/11 22:56:11]

Both the functions in the unit test and GDataFileSystem are very similar (I
think the only thing that's different is the lock assertion). You could have one
common function that's called by the unittest, and either drop the lock
assertion or create a wrapper function for it.

[hshi1, 2012/06/11 23:17:52]

I see, thanks for the explanation! I've removed this from the header file and
moved the implementation to the anonymous namespace.

+                               base::PlatformFileError error,
+                               GDataEntry* entry);
+
 // GData file system abstraction layer.
 // The interface is defined to make GDataFileSystem mockable.
 class GDataFileSystemInterface {
  public:
   virtual ~GDataFileSystemInterface() {}
 
   // Used to notify events on the file system.
   // All events are notified on UI thread.
   class Observer {
    public:
@@ skipping 49 matching lines @@
   };
 
   // Authenticates the user by fetching the auth token as
   // needed. |callback| will be run with the error code and the auth
   // token, on the thread this function is run.
   //
   // Must be called on UI thread.
   virtual void Authenticate(const AuthStatusCallback& callback) = 0;
 
   // Finds file info by using |resource_id|. This call does not initiate
-  // content refreshing and will invoke one of |callback| methods directly as
-  // it executes.
+  // content refreshing.
   //
-  // Can be called from UI/IO thread. |callback| is run on the calling thread
-  // synchronously.
-  virtual void FindEntryByResourceIdSync(const std::string& resource_id,
-                                         const FindEntryCallback& callback) = 0;
+  // Can be called from UI/IO thread. |callback| is run on the calling thread.

[achuithb, 2012/06/11 21:48:41]

This is not true, right? |callback| is run on the UI thread, right? Is the
intention to run this callback on the calling thread?

[hshi1, 2012/06/11 22:42:21]

Actually this IS true and |callback| is run on the calling thread. The
CreateRelayCallback ensures that.

This is the convention for many functions in GDataFileSystem (search for string
"|callback| is run on the calling thread." in gdata_file_system.h)

[achuithb, 2012/06/11 22:56:11]

Ah, ok. Didn't realize that's what CreateRelayCallback did.

+  virtual void FindEntryByResourceId(const std::string& resource_id,
+                                     const FindEntryCallback& callback) = 0;
 
   // Initiates transfer of |remote_src_file_path| to |local_dest_file_path|.
   // |remote_src_file_path| is the virtual source path on the gdata file system.
   // |local_dest_file_path| is the destination path on the local file system.
   //
   // Must be called from *UI* thread. |callback| is run on the calling thread.
   virtual void TransferFileFromRemoteToLocal(
       const FilePath& remote_src_file_path,
       const FilePath& local_dest_file_path,
       const FileOperationCallback& callback) = 0;
@@ skipping 224 matching lines @@
   virtual ~GDataFileSystem();
 
   // GDataFileSystem overrides.
   virtual void Initialize() OVERRIDE;
   virtual void AddObserver(Observer* observer) OVERRIDE;
   virtual void RemoveObserver(Observer* observer) OVERRIDE;
   virtual void StartUpdates() OVERRIDE;
   virtual void StopUpdates() OVERRIDE;
   virtual void CheckForUpdates() OVERRIDE;
   virtual void Authenticate(const AuthStatusCallback& callback) OVERRIDE;
-  virtual void FindEntryByResourceIdSync(
+  virtual void FindEntryByResourceId(
       const std::string& resource_id,
       const FindEntryCallback& callback) OVERRIDE;
   virtual void SearchAsync(const std::string& search_query,
                            const ReadDirectoryCallback& callback) OVERRIDE;
   virtual void TransferFileFromRemoteToLocal(
       const FilePath& remote_src_file_path,
       const FilePath& local_dest_file_path,
       const FileOperationCallback& callback) OVERRIDE;
   virtual void TransferFileFromLocalToRemote(
       const FilePath& local_src_file_path,
@@ skipping 1004 matching lines @@
   void GetFileByResourceIdOnUIThread(
       const std::string& resource_id,
       const GetFileCallback& get_file_callback,
       const GetDownloadDataCallback& get_download_data_callback);
   void GetEntryInfoByPathAsyncOnUIThread(
       const FilePath& file_path,
       const GetEntryInfoCallback& callback);
   void GetFileInfoByPathAsyncOnUIThread(
       const FilePath& file_path,
       const GetFileInfoCallback& callback);
+  void FindEntryByResourceIdOnUIThread(const std::string& resource_id,
+                                       const FindEntryCallback& callback);
   void ReadDirectoryByPathAsyncOnUIThread(
       const FilePath& file_path,
       const ReadDirectoryCallback& callback);
   void RequestDirectoryRefreshOnUIThread(
       const FilePath& file_path);
   void OnRequestDirectoryRefresh(GetDocumentsParams* params,
                                  base::PlatformFileError error);
   void GetCacheStateOnUIThread(const std::string& resource_id,
                                const std::string& md5,
                                const GetCacheStateCallback& callback);
@@ skipping 63 matching lines @@
 };
 
 // Sets the free disk space getter for testing.
 // The existing getter is deleted.
 void SetFreeDiskSpaceGetterForTesting(
     FreeDiskSpaceGetterInterface* getter);
 
 }  // namespace gdata
 
 #endif  // CHROME_BROWSER_CHROMEOS_GDATA_GDATA_FILE_SYSTEM_H_