gdata: Convert FindEntryByResourceIdSync() to asynchronous.

chrome/browser/chromeos/gdata/gdata_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 "chrome/browser/chromeos/gdata/gdata_util.h"
 
 #include <string>
 #include <vector>
 #include <utility>
 
@@ skipping 128 matching lines @@
      kReadOnlyFilePermissions));
   cache_paths->push_back(std::make_pair(
       file_system->GetCacheFilePath(resource_id, file_md5,
           GDataCache::CACHE_TYPE_TMP,
           GDataCache::CACHED_FILE_FROM_SERVER),
       kReadOnlyFilePermissions));
 
   callback.Run();
 }
 
+// Invoked upon completion of FindEntryByResourceId initiated by
+// ModifyGDataFileResourceUrl.
+void OnFindEntryByResourceId(Profile* profile,

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

Could we move this next to OpenEditURLUIThread? This name is also not very
satisfactory since it doesn't tell us anything about what's happening in this
function (but I can't really think of anything better).

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

Sounds reasonable, done.

On 2012/06/11 21:48:41, achuith.bhandarkar wrote:

+                                  const std::string& resource_id,

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

indentation is off

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

Done.

+                                  base::PlatformFileError error,
+                                  GDataEntry* entry) {
+  if (error != base::PLATFORM_FILE_OK || !entry || !entry->AsGDataFile())

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

Could you please add a DCHECK for UI thread here?

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

Again, this is run on the CALLER thread, not on UI thread.

On 2012/06/11 21:48:41, achuith.bhandarkar wrote:

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

Which I believe is the UI thread in this case, right? You can't call
OpenEditURLUIThread in this function otherwise.

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

Yes you're right. Done.

+    return;
+
+  std::string file_name = entry->AsGDataFile()->file_name();

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

could we make this const?

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

Done.

+  GURL edit_url = GetFileResourceUrl(resource_id, file_name);

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

could we make this const too? You may need to change the signature of
OpenEditURLUIThread

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

If I declare the object as "const GURL edit_url" here, then the correct
signature for OpenEditURLUIThread should be "void OpenEditURLUIThread(Profile*
profile, GURL const* edit_url)" -- note the 2nd param type is NOT "const GURL*".
Is this what you want? 

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

That's strange. It should be const GURL* since edit_url is const, not the
pointer. But if that's what the compiler wants, sure.

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

Sorry I was mistaken. It should be "const GURL*". Changed as you suggested,
thanks!

+  OpenEditURLUIThread(profile, &edit_url);
+  DVLOG(1) << "OnFindEntryByResourceId " << edit_url;
+}
+
 }  // namespace
 
 const FilePath& GetGDataMountPointPath() {
   CR_DEFINE_STATIC_LOCAL(FilePath, gdata_mount_path,
       (FilePath::FromUTF8Unsafe(kGDataMountPointPath)));
   return gdata_mount_path;
 }
 
 const std::string& GetGDataMountPointPathAsString() {
   CR_DEFINE_STATIC_LOCAL(std::string, gdata_mount_path_string,
@@ skipping 12 matching lines @@
   std::string url(base::StringPrintf(
       "%s:%s",
       chrome::kDriveScheme,
       net::EscapePath(resource_id).c_str()));
   return GURL(url);
 }
 
 void ModifyGDataFileResourceUrl(Profile* profile,
                                 const FilePath& gdata_cache_path,
                                 GURL* url) {
   GDataFileSystem* file_system = GetGDataFileSystem(profile);

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

Could you please add a DCHECK for UI thread here?

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

Done.

   if (!file_system)
     return;
 
   // Handle hosted documents. The edit url is in the temporary file, so we
   // read it on a blocking thread.
   if (file_system->GetCacheDirectoryPath(
           GDataCache::CACHE_TYPE_TMP_DOCUMENTS).IsParent(
       gdata_cache_path)) {
     GURL* edit_url = new GURL();
     content::BrowserThread::GetBlockingPool()->PostTaskAndReply(FROM_HERE,
         base::Bind(&GetHostedDocumentURLBlockingThread,
                    gdata_cache_path, edit_url),
         base::Bind(&OpenEditURLUIThread, profile, base::Owned(edit_url)));
     *url = GURL();
     return;

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

instead of this return, how about using an else instead?

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

Done.

   }
 
   // Handle all other gdata files.
   if (file_system->GetCacheDirectoryPath(
           GDataCache::CACHE_TYPE_TMP).IsParent(gdata_cache_path)) {
     const std::string resource_id =
         gdata_cache_path.BaseName().RemoveExtension().AsUTF8Unsafe();
-    GDataEntry* entry = NULL;
-    file_system->FindEntryByResourceIdSync(
-        resource_id, base::Bind(&ReadOnlyFindEntryCallback, &entry));
-
-    std::string file_name;
-    if (entry && entry->AsGDataFile())
-      file_name = entry->AsGDataFile()->file_name();
-
-    *url = gdata::util::GetFileResourceUrl(resource_id, file_name);
-    DVLOG(1) << "ModifyGDataFileResourceUrl " << *url;
+    file_system->FindEntryByResourceId(resource_id,
+                                       base::Bind(&OnFindEntryByResourceId,
+                                                  profile,
+                                                  resource_id));
+    *url = GURL();
+    return;

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

this return seems unnecessary

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

Done.

   }
 }
 
 bool IsUnderGDataMountPoint(const FilePath& path) {
   return GetGDataMountPointPath() == path ||
          GetGDataMountPointPath().IsParent(path);
 }
 
 GDataSearchPathType GetSearchPathStatus(const FilePath& path) {
   std::vector<std::string> components;
@@ skipping 173 matching lines @@
   // Assign the extracted extensions to md5 and extra_extension.
   int extension_count = extensions.size();
   *md5 = (extension_count > 0) ? extensions[extension_count - 1] :
                                  std::string();
   *extra_extension = (extension_count > 1) ? extensions[extension_count - 2] :
                                              std::string();
 }
 
 }  // namespace util
 }  // namespace gdata