diff options
7 files changed, 233 insertions, 199 deletions
diff --git a/chrome/browser/chromeos/drive/drive_file_system_util.cc b/chrome/browser/chromeos/drive/drive_file_system_util.cc index d390f71..c5ed5c7 100644 --- a/chrome/browser/chromeos/drive/drive_file_system_util.cc +++ b/chrome/browser/chromeos/drive/drive_file_system_util.cc @@ -5,34 +5,23 @@ #include "chrome/browser/chromeos/drive/drive_file_system_util.h" #include <string> -#include <utility> -#include <vector> #include "base/basictypes.h" #include "base/bind.h" #include "base/bind_helpers.h" -#include "base/file_util.h" #include "base/files/file_path.h" -#include "base/json/json_reader.h" #include "base/logging.h" #include "base/message_loop_proxy.h" #include "base/string_util.h" #include "base/stringprintf.h" #include "base/strings/string_number_conversions.h" #include "chrome/browser/chromeos/drive/drive.pb.h" -#include "chrome/browser/chromeos/drive/drive_cache.h" #include "chrome/browser/chromeos/drive/drive_file_system_interface.h" #include "chrome/browser/chromeos/drive/drive_system_service.h" #include "chrome/browser/chromeos/drive/file_write_helper.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/ui/browser.h" -#include "chrome/browser/ui/browser_finder.h" -#include "chrome/common/chrome_version_info.h" -#include "chrome/common/pref_names.h" #include "chrome/common/url_constants.h" #include "content/public/browser/browser_thread.h" #include "net/base/escape.h" -#include "net/base/network_change_notifier.h" #include "webkit/fileapi/file_system_url.h" using content::BrowserThread; @@ -66,68 +55,12 @@ DriveFileSystemInterface* GetDriveFileSystem(Profile* profile) { return system_service ? system_service->file_system() : NULL; } -DriveCache* GetDriveCache(Profile* profile) { - DriveSystemService* system_service = - DriveSystemServiceFactory::GetForProfile(profile); - return system_service ? system_service->cache() : NULL; -} - FileWriteHelper* GetFileWriteHelper(Profile* profile) { DriveSystemService* system_service = DriveSystemServiceFactory::GetForProfile(profile); return system_service ? system_service->file_write_helper() : NULL; } -GURL GetHostedDocumentURLBlockingThread( - const base::FilePath& drive_cache_path) { - std::string json; - if (!file_util::ReadFileToString(drive_cache_path, &json)) { - NOTREACHED() << "Unable to read file " << drive_cache_path.value(); - return GURL(); - } - DVLOG(1) << "Hosted doc content " << json; - scoped_ptr<base::Value> val(base::JSONReader::Read(json)); - base::DictionaryValue* dict_val; - if (!val.get() || !val->GetAsDictionary(&dict_val)) { - NOTREACHED() << "Parse failure for " << json; - return GURL(); - } - std::string edit_url; - if (!dict_val->GetString("url", &edit_url)) { - NOTREACHED() << "url field doesn't exist in " << json; - return GURL(); - } - GURL url(edit_url); - DVLOG(1) << "edit url " << url; - return url; -} - -void OpenEditURLUIThread(Profile* profile, const GURL& edit_url) { - Browser* browser = chrome::FindLastActiveWithProfile(profile, - chrome::HOST_DESKTOP_TYPE_ASH); - if (browser) { - browser->OpenURL(content::OpenURLParams(edit_url, content::Referrer(), - CURRENT_TAB, content::PAGE_TRANSITION_TYPED, false)); - } -} - -// Invoked upon completion of GetEntryInfoByResourceId initiated by -// ModifyDriveFileResourceUrl. -void OnGetEntryInfoByResourceId(Profile* profile, - const std::string& resource_id, - DriveFileError error, - const base::FilePath& drive_file_path, - scoped_ptr<DriveEntryProto> entry_proto) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - - if (error != DRIVE_FILE_OK) - return; - - const GURL edit_url = FilePathToDriveURL(drive_file_path); - OpenEditURLUIThread(profile, edit_url); - DVLOG(1) << "OnGetEntryInfoByResourceId " << edit_url; -} - } // namespace @@ -193,42 +126,17 @@ base::FilePath DriveURLToFilePath(const GURL& url) { return base::FilePath::FromUTF8Unsafe(path_string); } -void ModifyDriveFileResourceUrl(Profile* profile, - const base::FilePath& drive_cache_path, - GURL* url) { +void MaybeSetDriveURL(Profile* profile, const base::FilePath& path, GURL* url) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + if (!IsUnderDriveMountPoint(path)) + return; + DriveFileSystemInterface* file_system = GetDriveFileSystem(profile); if (!file_system) return; - DriveCache* cache = GetDriveCache(profile); - if (!cache) - return; - if (cache->GetCacheDirectoryPath(DriveCache::CACHE_TYPE_TMP_DOCUMENTS). - IsParent(drive_cache_path)) { - // Handle hosted documents. The edit url is in the temporary file, so we - // read it on a blocking thread. - base::PostTaskAndReplyWithResult( - content::BrowserThread::GetBlockingPool(), - FROM_HERE, - base::Bind(&GetHostedDocumentURLBlockingThread, drive_cache_path), - base::Bind(&OpenEditURLUIThread, profile)); - *url = GURL(); - } else if (cache->GetCacheDirectoryPath(DriveCache::CACHE_TYPE_TMP). - IsParent(drive_cache_path) || - cache->GetCacheDirectoryPath(DriveCache::CACHE_TYPE_PERSISTENT). - IsParent(drive_cache_path)) { - // Handle all other drive files. - const std::string resource_id = - drive_cache_path.BaseName().RemoveExtension().AsUTF8Unsafe(); - file_system->GetEntryInfoByResourceId( - resource_id, - base::Bind(&OnGetEntryInfoByResourceId, - profile, - resource_id)); - *url = GURL(); - } + *url = FilePathToDriveURL(util::ExtractDrivePath(path)); } bool IsUnderDriveMountPoint(const base::FilePath& path) { diff --git a/chrome/browser/chromeos/drive/drive_file_system_util.h b/chrome/browser/chromeos/drive/drive_file_system_util.h index e95711e..fb41819 100644 --- a/chrome/browser/chromeos/drive/drive_file_system_util.h +++ b/chrome/browser/chromeos/drive/drive_file_system_util.h @@ -97,10 +97,8 @@ 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, - const base::FilePath& drive_cache_path, - GURL* url); +// Overwrites |url| with a Drive URL when appropriate. +void MaybeSetDriveURL(Profile* profile, const base::FilePath& path, GURL* url); // Returns true if the given path is under the Drive mount point. bool IsUnderDriveMountPoint(const base::FilePath& path); diff --git a/chrome/browser/chromeos/drive/drive_url_request_job.cc b/chrome/browser/chromeos/drive/drive_url_request_job.cc index c936c18..8832cde 100644 --- a/chrome/browser/chromeos/drive/drive_url_request_job.cc +++ b/chrome/browser/chromeos/drive/drive_url_request_job.cc @@ -44,6 +44,7 @@ const net::UnescapeRule::Type kUrlPathUnescapeMask = net::UnescapeRule::CONTROL_CHARS; const int kHTTPOk = 200; +const int kHTTPFound = 302; const int kHTTPNotAllowed = 403; const int kHTTPNotFound = 404; const int kHTTPInternalError = 500; @@ -300,6 +301,16 @@ int DriveURLRequestJob::GetResponseCode() const { return response_info_->headers->response_code(); } +bool DriveURLRequestJob::IsRedirectResponse(GURL* location, + int* http_status_code) { + if (redirect_url_.is_empty()) + return false; + + *location = redirect_url_; + *http_status_code = kHTTPFound; + return true; +} + bool DriveURLRequestJob::ReadRawData(net::IOBuffer* dest, int dest_size, int* bytes_read) { @@ -428,6 +439,13 @@ void DriveURLRequestJob::OnGetEntryInfoByPath( return; } DCHECK(entry_proto.get()); + + if (entry_proto->file_specific_info().is_hosted_document()) { + redirect_url_ = GURL(entry_proto->file_specific_info().alternate_url()); + NotifySuccess(); + return; + } + redirect_url_ = GURL(); 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(); diff --git a/chrome/browser/chromeos/drive/drive_url_request_job.h b/chrome/browser/chromeos/drive/drive_url_request_job.h index 2a911d7..5f23216 100644 --- a/chrome/browser/chromeos/drive/drive_url_request_job.h +++ b/chrome/browser/chromeos/drive/drive_url_request_job.h @@ -60,6 +60,8 @@ class DriveURLRequestJob : public net::URLRequestJob { virtual bool GetMimeType(std::string* mime_type) const OVERRIDE; virtual void GetResponseInfo(net::HttpResponseInfo* info) OVERRIDE; virtual int GetResponseCode() const OVERRIDE; + virtual bool IsRedirectResponse(GURL* location, + int* http_status_code) OVERRIDE; virtual bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) OVERRIDE; @@ -126,6 +128,7 @@ class DriveURLRequestJob : public net::URLRequestJob { base::FilePath local_file_path_; base::FilePath drive_file_path_; + GURL redirect_url_; std::string mime_type_; int64 initial_file_size_; int64 remaining_bytes_; diff --git a/chrome/browser/chromeos/drive/drive_url_request_job_unittest.cc b/chrome/browser/chromeos/drive/drive_url_request_job_unittest.cc index 1deb63a..115f573 100644 --- a/chrome/browser/chromeos/drive/drive_url_request_job_unittest.cc +++ b/chrome/browser/chromeos/drive/drive_url_request_job_unittest.cc @@ -7,68 +7,221 @@ #include "base/bind.h" #include "base/memory/scoped_ptr.h" #include "chrome/browser/chromeos/drive/drive_file_system_util.h" +#include "chrome/browser/chromeos/drive/fake_drive_file_system.h" +#include "chrome/browser/google_apis/fake_drive_service.h" +#include "chrome/browser/google_apis/test_util.h" +#include "chrome/common/url_constants.h" #include "content/public/test/test_browser_thread.h" #include "googleurl/src/gurl.h" #include "net/url_request/url_request_test_util.h" #include "testing/gtest/include/gtest/gtest.h" -namespace drive { +using content::BrowserThread; -class DriveFileSystem; +namespace drive { namespace { -DriveFileSystemInterface* GetNullDriveFileSystem() { - return NULL; -} +// A simple URLRequestJobFactory implementation to create DriveURLReuqestJob. +class TestURLRequestJobFactory : public net::URLRequestJobFactory { + public: + TestURLRequestJobFactory( + const DriveURLRequestJob::DriveFileSystemGetter& file_system_getter) + : file_system_getter_(file_system_getter) { + } + + virtual ~TestURLRequestJobFactory() {} + + // net::URLRequestJobFactory override: + virtual net::URLRequestJob* MaybeCreateJobWithProtocolHandler( + const std::string& scheme, + net::URLRequest* request, + net::NetworkDelegate* network_delegate) const OVERRIDE { + return new DriveURLRequestJob(file_system_getter_, + request, + network_delegate); + } + + virtual bool IsHandledProtocol(const std::string& scheme) const OVERRIDE { + return scheme == chrome::kDriveScheme; + } + + virtual bool IsHandledURL(const GURL& url) const OVERRIDE { + return url.is_valid() && IsHandledProtocol(url.scheme()); + } + + private: + DriveURLRequestJob::DriveFileSystemGetter file_system_getter_; + + DISALLOW_COPY_AND_ASSIGN(TestURLRequestJobFactory); +}; + +// URLRequest::Delegate for tests. +class TestDelegate : public net::URLRequest::Delegate { + public: + TestDelegate() {} + + virtual ~TestDelegate() {} + + const net::URLRequestStatus& status() const { return status_; } + const GURL& redirect_url() const { return redirect_url_; } + + // net::URLRequest::Delegate override: + virtual void OnReceivedRedirect(net::URLRequest* request, + const GURL& new_url, + bool* defer_redirect) OVERRIDE { + // Save the new URL and cancel the request. + EXPECT_TRUE(redirect_url_.is_empty()); + redirect_url_ = new_url; + + request->Cancel(); + } + + virtual void OnResponseStarted(net::URLRequest* request) OVERRIDE { + if (!request->status().is_success()) + CompleteRequest(request); + } + + virtual void OnReadCompleted(net::URLRequest* request, + int bytes_read) OVERRIDE {} + + private: + void CompleteRequest(net::URLRequest* request) { + status_ = request->status(); + delete request; + + BrowserThread::PostTask(BrowserThread::UI, + FROM_HERE, + base::MessageLoop::QuitClosure()); + } + + net::URLRequestStatus status_; + GURL redirect_url_; +}; } // namespace class DriveURLRequestJobTest : public testing::Test { protected: - DriveURLRequestJobTest() - : io_thread_(content::BrowserThread::IO, &message_loop_) { + DriveURLRequestJobTest() : ui_thread_(BrowserThread::UI, &message_loop_), + io_thread_(BrowserThread::IO) { } virtual void SetUp() OVERRIDE { - url_request_context_.reset(new net::TestURLRequestContext); - delegate_.reset(new net::TestDelegate); + io_thread_.StartIOThread(); + + // Initialize FakeDriveService. + drive_service_.reset(new google_apis::FakeDriveService); + drive_service_->LoadResourceListForWapi("chromeos/gdata/root_feed.json"); + drive_service_->LoadAccountMetadataForWapi( + "chromeos/gdata/account_metadata.json"); + drive_service_->LoadAppListForDriveApi("chromeos/drive/applist.json"); + + // Initialize FakeDriveFileSystem. + drive_file_system_.reset( + new test_util::FakeDriveFileSystem(drive_service_.get())); + BrowserThread::PostTaskAndReply( + BrowserThread::IO, + FROM_HERE, + base::Bind(&DriveURLRequestJobTest::SetUpOnIOThread, + base::Unretained(this)), + base::MessageLoop::QuitClosure()); + message_loop_.Run(); + } + + virtual void TearDown() OVERRIDE { + BrowserThread::PostTaskAndReply( + BrowserThread::IO, + FROM_HERE, + base::Bind(&DriveURLRequestJobTest::TearDownOnIOThread, + base::Unretained(this)), + base::MessageLoop::QuitClosure()); + message_loop_.Run(); + } + + void SetUpOnIOThread() { + // Initialize URLRequest related objects. + delegate_.reset(new TestDelegate); network_delegate_.reset(new net::TestNetworkDelegate); + url_request_job_factory_.reset(new TestURLRequestJobFactory( + base::Bind(&DriveURLRequestJobTest::GetDriveFileSystem, + base::Unretained(this)))); + url_request_context_.reset(new net::URLRequestContext); + url_request_context_->set_job_factory(url_request_job_factory_.get()); + url_request_context_->set_network_delegate(network_delegate_.get()); + } + + void TearDownOnIOThread() { + url_request_context_.reset(); + url_request_job_factory_.reset(); + network_delegate_.reset(); + delegate_.reset(); + } + + DriveFileSystemInterface* GetDriveFileSystem() const { + return drive_file_system_.get(); + } - // TODO(tedv): Using the NetworkDelegate with the URLRequestContext - // with set_network_delegate() instead of a NULL delegate causes - // unit test failures, which should be fixed. This occurs because - // the TestNetworkDelegate generates a failure if an failed request - // is generated before the OnBeforeURLRequest() method is called, - // and DriveURLRequestJob::Start() does not call OnBeforeURLRequest(). - // There is further discussion of this at: - // https://codereview.chromium.org/13079008/ - //url_request_context_.set_network_delegate(network_delegate_.get()); + void RunRequest(const GURL& url, const std::string& method) { + BrowserThread::PostTask( + BrowserThread::IO, + FROM_HERE, + base::Bind(&DriveURLRequestJobTest::StartRequestOnIOThread, + base::Unretained(this), url, method)); + message_loop_.Run(); } - MessageLoopForIO message_loop_; + void StartRequestOnIOThread(const GURL& url, const std::string& method) { + // |delegate_| will delete the request. + net::URLRequest* request = new net::URLRequest(url, delegate_.get(), + url_request_context_.get(), + network_delegate_.get()); + request->set_method(method); + request->Start(); + } + + MessageLoopForUI message_loop_; + content::TestBrowserThread ui_thread_; content::TestBrowserThread io_thread_; - scoped_ptr<net::TestURLRequestContext> url_request_context_; - scoped_ptr<net::TestDelegate> delegate_; - scoped_ptr<net::TestNetworkDelegate> network_delegate_; + scoped_ptr<google_apis::FakeDriveService> drive_service_; + scoped_ptr<test_util::FakeDriveFileSystem> drive_file_system_; + scoped_ptr<TestDelegate> delegate_; + scoped_ptr<net::NetworkDelegate> network_delegate_; + scoped_ptr<TestURLRequestJobFactory> url_request_job_factory_; + scoped_ptr<net::URLRequestContext> url_request_context_; }; TEST_F(DriveURLRequestJobTest, NonGetMethod) { - net::TestURLRequest request( - util::FilePathToDriveURL(base::FilePath::FromUTF8Unsafe("file")), - delegate_.get(), url_request_context_.get(), NULL); - request.set_method("POST"); // Set non "GET" method. - - 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_METHOD_NOT_SUPPORTED, request.status().error()); + RunRequest(util::FilePathToDriveURL(base::FilePath::FromUTF8Unsafe("file")), + "POST"); // Set non "GET" method. + + EXPECT_EQ(net::URLRequestStatus::FAILED, delegate_->status().status()); + EXPECT_EQ(net::ERR_METHOD_NOT_SUPPORTED, delegate_->status().error()); +} + +TEST_F(DriveURLRequestJobTest, HostedDocument) { + // Get a hosted document's path. + DriveFileError error = DRIVE_FILE_ERROR_FAILED; + scoped_ptr<DriveEntryProto> entry; + base::FilePath file_path; + drive_file_system_->GetEntryInfoByResourceId( + "document:5_document_resource_id", + google_apis::test_util::CreateCopyResultCallback( + &error, &file_path, &entry)); + google_apis::test_util::RunBlockingPoolTask(); + + EXPECT_EQ(DRIVE_FILE_OK, error); + ASSERT_TRUE(entry); + ASSERT_TRUE(entry->file_specific_info().is_hosted_document()); + + // Run request for the document. + RunRequest(util::FilePathToDriveURL(file_path), "GET"); + + // Request was cancelled by the delegate because of redirect. + EXPECT_EQ(net::URLRequestStatus::CANCELED, delegate_->status().status()); + EXPECT_EQ(GURL(entry->file_specific_info().alternate_url()), + delegate_->redirect_url()); } } // namespace drive diff --git a/chrome/browser/chromeos/extensions/file_manager/file_manager_util.cc b/chrome/browser/chromeos/extensions/file_manager/file_manager_util.cc index d4c4f39..f34b9a4 100644 --- a/chrome/browser/chromeos/extensions/file_manager/file_manager_util.cc +++ b/chrome/browser/chromeos/extensions/file_manager/file_manager_util.cc @@ -268,34 +268,6 @@ void ShowWarningMessageBox(Profile* profile, const base::FilePath& path) { chrome::MESSAGE_BOX_TYPE_WARNING); } -// Called when a file on Drive was found. Opens the file found at |file_path| -// in a new tab with a URL computed based on the |file_type| -void OnDriveFileFound(Profile* profile, - const base::FilePath& file_path, - drive::DriveFileType file_type, - drive::DriveFileError error, - scoped_ptr<drive::DriveEntryProto> entry_proto) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - - if (entry_proto.get() && !entry_proto->has_file_specific_info()) - error = drive::DRIVE_FILE_ERROR_NOT_FOUND; - - if (error == drive::DRIVE_FILE_OK) { - GURL page_url; - if (file_type == drive::REGULAR_FILE) { - 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 { - NOTREACHED(); - } - OpenNewTab(page_url, profile); - } else { - ShowWarningMessageBox(profile, file_path); - } -} - void InstallCRX(Browser* browser, const base::FilePath& path) { ExtensionService* service = extensions::ExtensionSystem::Get(browser->profile())->extension_service(); @@ -890,36 +862,21 @@ bool ExecuteBuiltinHandler(Browser* browser, const base::FilePath& path, if (IsSupportedBrowserExtension(file_extension.data()) || ShouldBeOpenedWithPlugin(profile, file_extension.data())) { GURL page_url = net::FilePathToFileURL(path); - // Override gdata resource to point to internal handler instead of file: - // URL. - if (drive::util::GetSpecialRemoteRootPath().IsParent(path)) { - drive::DriveSystemService* system_service = - drive::DriveSystemServiceFactory::GetForProfile(profile); - if (!system_service) - return false; - - // Open the file once the file is found. - system_service->file_system()->GetEntryInfoByPath( - drive::util::ExtractDrivePath(path), - base::Bind(&OnDriveFileFound, profile, path, drive::REGULAR_FILE)); - return true; + // Override drive resource to point to internal handler instead of file URL. + if (drive::util::IsUnderDriveMountPoint(path)) { + page_url = drive::util::FilePathToDriveURL( + drive::util::ExtractDrivePath(path)); } OpenNewTab(page_url, NULL); return true; } if (IsSupportedGDocsExtension(file_extension.data())) { - if (drive::util::GetSpecialRemoteRootPath().IsParent(path)) { - // The file is on Google Docs. Get the Docs from the Drive service. - drive::DriveSystemService* system_service = - drive::DriveSystemServiceFactory::GetForProfile(profile); - if (!system_service) - return false; - - system_service->file_system()->GetEntryInfoByPath( - drive::util::ExtractDrivePath(path), - base::Bind(&OnDriveFileFound, profile, path, - drive::HOSTED_DOCUMENT)); + if (drive::util::IsUnderDriveMountPoint(path)) { + // The file is on Google Docs. Open with drive URL. + GURL url = drive::util::FilePathToDriveURL( + drive::util::ExtractDrivePath(path)); + OpenNewTab(url, NULL); } else { // The file is local (downloaded from an attachment or otherwise copied). // Parse the file to extract the Docs url and open this url. diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc index a31b4b0..2f5b24c 100644 --- a/chrome/browser/ui/browser.cc +++ b/chrome/browser/ui/browser.cc @@ -1705,25 +1705,22 @@ void Browser::FileSelected(const base::FilePath& path, int index, FileSelectedWithExtraInfo(ui::SelectedFileInfo(path, path), index, params); } -void Browser::FileSelectedWithExtraInfo( - const ui::SelectedFileInfo& file_info, - int index, - void* params) { +void Browser::FileSelectedWithExtraInfo(const ui::SelectedFileInfo& file_info, + int index, + void* params) { profile_->set_last_selected_directory(file_info.file_path.DirName()); - const base::FilePath& path = file_info.local_path; - GURL file_url = net::FilePathToFileURL(path); + GURL url = net::FilePathToFileURL(file_info.local_path); #if defined(OS_CHROMEOS) - drive::util::ModifyDriveFileResourceUrl(profile_, path, &file_url); + drive::util::MaybeSetDriveURL(profile_, file_info.file_path, &url); #endif - if (file_url.is_empty()) + if (url.is_empty()) return; OpenURL(OpenURLParams( - file_url, Referrer(), CURRENT_TAB, content::PAGE_TRANSITION_TYPED, - false)); + url, Referrer(), CURRENT_TAB, content::PAGE_TRANSITION_TYPED, false)); } /////////////////////////////////////////////////////////////////////////////// |