chromeos: Replace resource ID in drive URL with path

BUG=175948
TEST=Open a .txt file in Google Drive directory from both Files.app and Ctrl-O


Review URL: https://chromiumcodereview.appspot.com/13401003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@192541 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/drive/drive_file_system_util.cc

@@ -115,18 +115,16 @@ void OpenEditURLUIThread(Profile* profile, const GURL& edit_url) {
 void OnGetEntryInfoByResourceId(Profile* profile,
                                 const std::string& resource_id,
                                 DriveFileError error,
-                                const base::FilePath& /* drive_file_path */,
+                                const base::FilePath& drive_file_path,
                                 scoped_ptr<DriveEntryProto> entry_proto) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (error != DRIVE_FILE_OK)
     return;
 
-  DCHECK(entry_proto.get());
-  const std::string& base_name = entry_proto->base_name();
-  const GURL edit_url = GetFileResourceUrl(resource_id, base_name);
+  const GURL edit_url = FilePathToDriveURL(drive_file_path);
   OpenEditURLUIThread(profile, edit_url);
-  DVLOG(1) << "OnFindEntryByResourceId " << edit_url;
+  DVLOG(1) << "OnGetEntryInfoByResourceId " << edit_url;
 }
 
 }  // namespace
@@ -179,15 +177,21 @@ const base::FilePath& GetDriveMyDriveMountPointPath() {
   return drive_mydrive_mount_path;
 }
 
-GURL GetFileResourceUrl(const std::string& resource_id,
-                        const std::string& file_name) {
-  std::string url(base::StringPrintf(
-      "%s:%s",
+GURL FilePathToDriveURL(const base::FilePath& path) {
+  std::string url(base::StringPrintf("%s:%s",
                                      chrome::kDriveScheme,
-      net::EscapePath(resource_id).c_str()));
+                                     path.AsUTF8Unsafe().c_str()));
   return GURL(url);
 }
 
+base::FilePath DriveURLToFilePath(const GURL& url) {
+  if (!url.is_valid() || url.scheme() != chrome::kDriveScheme)
+    return base::FilePath();
+  std::string path_string = net::UnescapeURLComponent(
+      url.path(), net::UnescapeRule::NORMAL);
+  return base::FilePath::FromUTF8Unsafe(path_string);
+}
+
 void ModifyDriveFileResourceUrl(Profile* profile,
                                 const base::FilePath& drive_cache_path,
                                 GURL* url) {

chrome/browser/chromeos/drive/drive_file_system_util.h

@@ -89,10 +89,11 @@ const std::string& GetDriveMountPointPathAsString();
 // Returns the 'local' root of remote file system as "/special".
 const base::FilePath& GetSpecialRemoteRootPath();
 
-// Returns the gdata file resource url formatted as
-// chrome://drive/<resource_id>/<file_name>.
-GURL GetFileResourceUrl(const std::string& resource_id,
-                        const std::string& file_name);
+// Returns the gdata file resource url formatted as "drive:<path>"
+GURL FilePathToDriveURL(const base::FilePath& path);
+
+// Converts a drive: URL back to a path that can be passed to DriveFileSystem.
+base::FilePath DriveURLToFilePath(const GURL& url);
 
 // Given a profile and a drive_cache_path, return the file resource url.
 void ModifyDriveFileResourceUrl(Profile* profile,

chrome/browser/chromeos/drive/drive_file_system_util_unittest.cc

@@ -5,11 +5,37 @@
 #include "chrome/browser/chromeos/drive/drive_file_system_util.h"
 
 #include "base/files/file_path.h"
+#include "base/utf_string_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace drive {
 namespace util {
 
+TEST(DriveFileSystemUtilTest, FilePathToDriveURL) {
+  base::FilePath path;
+
+  // Path with alphabets and numbers.
+  path = GetDriveMyDriveRootPath().AppendASCII("foo/bar012.txt");
+  EXPECT_EQ(path, DriveURLToFilePath(FilePathToDriveURL(path)));
+
+  // Path with symbols.
+  path = GetDriveMyDriveRootPath().AppendASCII(
+      " !\"#$%&'()*+,-.:;<=>?@[\\]^_`{|}~");
+  EXPECT_EQ(path, DriveURLToFilePath(FilePathToDriveURL(path)));
+
+  // Path with '%'.
+  path = GetDriveMyDriveRootPath().AppendASCII("%19%20%21.txt");
+  EXPECT_EQ(path, DriveURLToFilePath(FilePathToDriveURL(path)));
+
+  // Path with multi byte characters.
+  string16 utf16_string;
+  utf16_string.push_back(0x307b);  // HIRAGANA_LETTER_HO
+  utf16_string.push_back(0x3052);  // HIRAGANA_LETTER_GE
+  path = GetDriveMyDriveRootPath().Append(
+      base::FilePath::FromUTF8Unsafe(UTF16ToUTF8(utf16_string) + ".txt"));
+  EXPECT_EQ(path, DriveURLToFilePath(FilePathToDriveURL(path)));
+}
+
 TEST(DriveFileSystemUtilTest, IsUnderDriveMountPoint) {
   EXPECT_FALSE(IsUnderDriveMountPoint(
       base::FilePath::FromUTF8Unsafe("/wherever/foo.txt")));

chrome/browser/chromeos/drive/drive_url_request_job.cc

@@ -17,6 +17,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/chromeos/drive/drive.pb.h"
 #include "chrome/browser/chromeos/drive/drive_file_system_interface.h"
+#include "chrome/browser/chromeos/drive/drive_file_system_util.h"
 #include "chrome/browser/chromeos/drive/drive_system_service.h"
 #include "chrome/browser/google_apis/base_operations.h"
 #include "chrome/browser/google_apis/task_util.h"
@@ -69,20 +70,6 @@ std::string FixupMimeType(const std::string& type) {
   return type;
 }
 
-// Helper function that extracts and unescapes resource_id from drive urls
-// (drive:<resource_id>).
-bool ParseDriveUrl(const std::string& path, std::string* resource_id) {
-  const std::string drive_schema(chrome::kDriveScheme + std::string(":"));
-  if (!StartsWithASCII(path, drive_schema, false) ||
-      path.size() <= drive_schema.size()) {
-    return false;
-  }
-
-  std::string id = path.substr(drive_schema.size());
-  *resource_id = net::UnescapeURLComponent(id, kUrlPathUnescapeMask);
-  return resource_id->size();
-}
-
 // Helper function to cancel Drive download operation on UI thread.
 void CancelDriveDownloadOnUIThread(
     const DriveURLRequestJob::DriveFileSystemGetter& file_system_getter,
@@ -109,39 +96,37 @@ void CancelDriveDownload(
                  file_system_getter, drive_file_path));
 }
 
-// Helper function to call DriveFileSystem::GetEntryInfoByResourceId.
-void GetEntryInfoByResourceIdOnUIThread(
+// Helper function to call DriveFileSystem::GetEntryInfoByPath.
+void GetEntryInfoByPathOnUIThread(
     const DriveURLRequestJob::DriveFileSystemGetter& file_system_getter,
-    const std::string& resource_id,
-    const drive::GetEntryInfoWithFilePathCallback& callback) {
+    const base::FilePath& path,
+    const drive::GetEntryInfoCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DriveFileSystemInterface* file_system = file_system_getter.Run();
   if (!file_system) {
-    callback.Run(DRIVE_FILE_ERROR_FAILED,
-                 base::FilePath(),
-                 scoped_ptr<DriveEntryProto>());
+    callback.Run(DRIVE_FILE_ERROR_FAILED, scoped_ptr<DriveEntryProto>());
     return;
   }
-  file_system->GetEntryInfoByResourceId(resource_id, callback);
+  file_system->GetEntryInfoByPath(path, callback);
 }
 
-// Returns the entry info for the |resource_id| on DriveFileSystem returned by
+// Returns the entry info for the |path| on DriveFileSystem returned by
 // |file_system_getter| via |callback|.
 // The main task will be done on UI thread, but this method itself is designed
 // to be run on IO thread. Also the |callback| will be run on IO thread, too.
-void GetEntryInfoByResourceId(
+void GetEntryInfoByPath(
     const DriveURLRequestJob::DriveFileSystemGetter& file_system_getter,
-    const std::string& resource_id,
-    const drive::GetEntryInfoWithFilePathCallback& callback) {
+    const base::FilePath& path,
+    const drive::GetEntryInfoCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
-      base::Bind(&GetEntryInfoByResourceIdOnUIThread,
+      base::Bind(&GetEntryInfoByPathOnUIThread,
                  file_system_getter,
-                 resource_id,
+                 path,
                  google_apis::CreateRelayCallback(callback)));
 }
 
@@ -262,19 +247,10 @@ void DriveURLRequestJob::Start() {
                                            net::ERR_METHOD_NOT_SUPPORTED));
     return;
   }
-
-  std::string resource_id;
-  if (!ParseDriveUrl(request_->url().spec(), &resource_id)) {
-    NotifyStartError(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                           net::ERR_INVALID_URL));
-    return;
-  }
-
-  GetEntryInfoByResourceId(
-      file_system_getter_,
-      resource_id,
-      base::Bind(&DriveURLRequestJob::OnGetEntryInfoByResourceId,
-                 weak_ptr_factory_.GetWeakPtr(), resource_id));
+  GetEntryInfoByPath(file_system_getter_,
+                     util::DriveURLToFilePath(request_->url()),
+                     base::Bind(&DriveURLRequestJob::OnGetEntryInfoByPath,
+                                weak_ptr_factory_.GetWeakPtr()));
 }
 
 void DriveURLRequestJob::Kill() {
@@ -435,32 +411,32 @@ DriveURLRequestJob::~DriveURLRequestJob() {
 
 //======================= DriveURLRequestJob private methods ===================
 
-void DriveURLRequestJob::OnGetEntryInfoByResourceId(
-    const std::string& resource_id,
+void DriveURLRequestJob::OnGetEntryInfoByPath(
     DriveFileError error,
-    const base::FilePath& drive_file_path,
     scoped_ptr<DriveEntryProto> entry_proto) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   if (entry_proto.get() && !entry_proto->has_file_specific_info())
     error = DRIVE_FILE_ERROR_NOT_FOUND;
 
-  if (error == DRIVE_FILE_OK) {
-    DCHECK(entry_proto.get());
-    mime_type_ = entry_proto->file_specific_info().content_mime_type();
-    drive_file_path_ = drive_file_path;
-    initial_file_size_ = entry_proto->file_info().size();
-  } else {
+  if (error != DRIVE_FILE_OK) {
     mime_type_.clear();
     drive_file_path_.clear();
     initial_file_size_ = 0;
+    NotifyStartError(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                           net::ERR_FILE_NOT_FOUND));
+    return;
   }
+  DCHECK(entry_proto.get());
+  mime_type_ = entry_proto->file_specific_info().content_mime_type();
+  drive_file_path_ = util::DriveURLToFilePath(request_->url());
+  initial_file_size_ = entry_proto->file_info().size();
   remaining_bytes_ = initial_file_size_;
 
   DVLOG(1) << "Getting file for resource id";
   GetFileByResourceId(
       file_system_getter_,
-      resource_id,
+      entry_proto->resource_id(),
       base::Bind(&DriveURLRequestJob::OnGetFileByResourceId,
                  weak_ptr_factory_.GetWeakPtr()),
       base::Bind(&DriveURLRequestJob::OnUrlFetchDownloadData,

chrome/browser/chromeos/drive/drive_url_request_job.h

@@ -90,10 +90,8 @@ class DriveURLRequestJob : public net::URLRequestJob {
   // Helper callback for handling result of file_util::GetFileSize().
   void OnGetFileSize(int64 *file_size, bool success);
 
-  // Helper callback for GetEntryInfoByResourceId invoked by Start().
-  void OnGetEntryInfoByResourceId(const std::string& resource_id,
-                                  DriveFileError error,
-                                  const base::FilePath& drive_file_path,
+  // Helper callback for GetEntryInfoByPath invoked by Start().
+  void OnGetEntryInfoByPath(DriveFileError error,
                             scoped_ptr<DriveEntryProto> entry_proto);
 
   // Helper methods for ReadRawData to open file and read from its corresponding

chrome/browser/chromeos/drive/drive_url_request_job_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
+#include "chrome/browser/chromeos/drive/drive_file_system_util.h"
 #include "content/public/test/test_browser_thread.h"
 #include "googleurl/src/gurl.h"
 #include "net/url_request/url_request_test_util.h"
@@ -45,7 +46,8 @@ class DriveURLRequestJobTest : public testing::Test {
 
 TEST_F(DriveURLRequestJobTest, NonGetMethod) {
   net::TestURLRequest request(
-      GURL("drive:file_id"), delegate_.get(), url_request_context_.get());
+      util::FilePathToDriveURL(base::FilePath::FromUTF8Unsafe("file")),
+      delegate_.get(), url_request_context_.get());
   request.set_method("POST");  // Set non "GET" method.
 
   scoped_refptr<DriveURLRequestJob> job(
@@ -59,20 +61,4 @@ TEST_F(DriveURLRequestJobTest, NonGetMethod) {
   EXPECT_EQ(net::ERR_METHOD_NOT_SUPPORTED, request.status().error());
 }
 
-TEST_F(DriveURLRequestJobTest, NonDriveScheme) {
-  net::TestURLRequest request(
-      GURL("http://www.google.com"),
-      delegate_.get(), url_request_context_.get());
-
-  scoped_refptr<DriveURLRequestJob> job(
-      new DriveURLRequestJob(
-          base::Bind(&GetNullDriveFileSystem),
-          &request, network_delegate_.get()));
-  job->Start();
-  MessageLoop::current()->RunUntilIdle();
-
-  EXPECT_EQ(net::URLRequestStatus::FAILED, request.status().status());
-  EXPECT_EQ(net::ERR_INVALID_URL, request.status().error());
-}
-
 }  // namespace drive

chrome/browser/chromeos/extensions/file_manager_util.cc

@@ -283,9 +283,8 @@ void OnDriveFileFound(Profile* profile,
   if (error == drive::DRIVE_FILE_OK) {
     GURL page_url;
     if (file_type == drive::REGULAR_FILE) {
-      page_url = drive::util::GetFileResourceUrl(
-          entry_proto->resource_id(),
-          entry_proto->base_name());
+      page_url = drive::util::FilePathToDriveURL(
+          drive::util::ExtractDrivePath(file_path));
     } else if (file_type == drive::HOSTED_DOCUMENT) {
       page_url = GURL(entry_proto->file_specific_info().alternate_url());
     } else {