diff options
16 files changed, 359 insertions, 225 deletions
diff --git a/chrome/browser/chromeos/extensions/file_browser_handler_api.cc b/chrome/browser/chromeos/extensions/file_browser_handler_api.cc index b59cbd9..0bd0ee5 100644 --- a/chrome/browser/chromeos/extensions/file_browser_handler_api.cc +++ b/chrome/browser/chromeos/extensions/file_browser_handler_api.cc @@ -17,7 +17,7 @@ #include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/select_file_dialog.h" #include "chrome/browser/ui/tab_contents/tab_contents.h" -#include "chrome/common/extensions/api/file_browser_handler.h" +#include "chrome/common/extensions/api/file_browser_handler_internal.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/child_process_security_policy.h" #include "content/public/browser/render_process_host.h" @@ -27,8 +27,11 @@ #include "webkit/fileapi/file_system_mount_point_provider.h" using content::BrowserContext; +using extensions::api::file_browser_handler_internal::FileEntryInfo; +using file_handler::FileSelector; -namespace SelectFile = extensions::api::file_browser_handler::SelectFile; +namespace SelectFile = + extensions::api::file_browser_handler_internal::SelectFile; namespace { @@ -41,7 +44,7 @@ const char kNoUserGestureError[] = // action. When user selects the file (or closes the dialog), the function's // |OnFilePathSelected| method will be called with the result. // When |SelectFile| is called, the class instance takes ownership of itself. -class FileSelectorImpl : public file_handler::FileSelector, +class FileSelectorImpl : public FileSelector, public SelectFileDialog::Listener { public: explicit FileSelectorImpl(FileHandlerSelectFileFunction* function); @@ -55,6 +58,8 @@ class FileSelectorImpl : public file_handler::FileSelector, // SelectFielDialog. virtual void SelectFile(const FilePath& suggested_name, Browser* browser) OVERRIDE; + virtual void set_function_for_test( + FileHandlerSelectFileFunction* function) OVERRIDE; // SelectFileDialog::Listener overrides. virtual void FileSelected(const FilePath& path, @@ -73,6 +78,8 @@ class FileSelectorImpl : public file_handler::FileSelector, // Extension function that uses the selector. scoped_refptr<FileHandlerSelectFileFunction> function_; + + DISALLOW_COPY_AND_ASSIGN(FileSelectorImpl); }; FileSelectorImpl::FileSelectorImpl(FileHandlerSelectFileFunction* function) @@ -98,6 +105,12 @@ void FileSelectorImpl::SelectFile(const FilePath& suggested_name, } } +// This should be used in testing only. +void FileSelectorImpl::set_function_for_test( + FileHandlerSelectFileFunction* function) { + NOTREACHED(); +} + bool FileSelectorImpl::DoSelectFile(const FilePath& suggested_name, Browser* browser) { DCHECK(!dialog_.get()); @@ -147,64 +160,91 @@ void FileSelectorImpl::SendResponse(bool success, function_ = NULL; } +typedef base::Callback<void (bool success, + const std::string& file_system_name, + const GURL& file_system_root)> + FileSystemOpenCallback; + +void RelayOpenFileSystemCallbackToFileThread( + const FileSystemOpenCallback& callback, + base::PlatformFileError error, + const std::string& file_system_name, + const GURL& file_system_root) { + bool success = (error == base::PLATFORM_FILE_OK); + content::BrowserThread::PostTask(content::BrowserThread::FILE, FROM_HERE, + base::Bind(callback, success, file_system_name, file_system_root)); +} + } // namespace -FileHandlerSelectFileFunction::FileHandlerSelectFileFunction() - : file_selector_(NULL), - render_process_host_id_(0) { -} +FileHandlerSelectFileFunction::FileHandlerSelectFileFunction() {} -FileHandlerSelectFileFunction::~FileHandlerSelectFileFunction() { -} +FileHandlerSelectFileFunction::~FileHandlerSelectFileFunction() {} bool FileHandlerSelectFileFunction::RunImpl() { scoped_ptr<SelectFile::Params> params(SelectFile::Params::Create(*args_)); FilePath suggested_name(params->selection_params.suggested_name); - if (!user_gesture()) { + if (!user_gesture() && !gesture_check_disabled_for_test_) { error_ = kNoUserGestureError; return false; } // If |file_selector_| is set (e.g. in test), use it instesad of creating new // file selector. - file_handler::FileSelector* file_selector = file_selector_; - file_selector_ = NULL; - if (!file_selector) - file_selector = new FileSelectorImpl(this); - + FileSelector* file_selector = GetFileSelector(); file_selector->SelectFile(suggested_name.BaseName(), GetCurrentBrowser()); return true; } +// static +void FileHandlerSelectFileFunction::set_file_selector_for_test( + FileSelector* file_selector) { + FileHandlerSelectFileFunction::file_selector_for_test_ = file_selector; +} + +// static +void FileHandlerSelectFileFunction::set_gesture_check_disabled_for_test( + bool disabled) { + FileHandlerSelectFileFunction::gesture_check_disabled_for_test_ = disabled; +} + void FileHandlerSelectFileFunction::OnFilePathSelected( bool success, const FilePath& full_path) { if (!success) { - Respond(false, FilePath()); + Respond(false, std::string(), GURL(), FilePath()); return; } - content::BrowserThread::PostTask(content::BrowserThread::FILE, FROM_HERE, - base::Bind(&FileHandlerSelectFileFunction::CreateFileOnFileThread, - this, full_path, - base::Bind(&FileHandlerSelectFileFunction::OnFileCreated, this))); + full_path_ = full_path; + + BrowserContext::GetFileSystemContext(profile_)->OpenFileSystem( + source_url_.GetOrigin(), fileapi::kFileSystemTypeExternal, false, + base::Bind(&RelayOpenFileSystemCallbackToFileThread, + base::Bind(&FileHandlerSelectFileFunction::CreateFileOnFileThread, + this))); }; void FileHandlerSelectFileFunction::CreateFileOnFileThread( - const FilePath& full_path, - const CreateFileCallback& callback) { - bool success = DoCreateFile(full_path); + bool success, + const std::string& file_system_name, + const GURL& file_system_root) { + DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE)); + + if (success) + success = DoCreateFile(); content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE, - base::Bind(callback, success, full_path)); + base::Bind(&FileHandlerSelectFileFunction::OnFileCreated, this, + success, file_system_name, file_system_root)); } -bool FileHandlerSelectFileFunction::DoCreateFile(const FilePath& full_path) { +bool FileHandlerSelectFileFunction::DoCreateFile() { DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE)); // Don't allow links. - if (file_util::PathExists(full_path) && file_util::IsLink(full_path)) + if (file_util::PathExists(full_path_) && file_util::IsLink(full_path_)) return false; bool created = false; @@ -212,58 +252,73 @@ bool FileHandlerSelectFileFunction::DoCreateFile(const FilePath& full_path) { int creation_flags = base::PLATFORM_FILE_CREATE_ALWAYS | base::PLATFORM_FILE_READ | base::PLATFORM_FILE_WRITE; - base::CreatePlatformFile(full_path, creation_flags, &created, &error); + base::CreatePlatformFile(full_path_, creation_flags, &created, &error); return error == base::PLATFORM_FILE_OK; } -void FileHandlerSelectFileFunction::OnFileCreated(bool success, - const FilePath& full_path) { +void FileHandlerSelectFileFunction::OnFileCreated( + bool success, + const std::string& file_system_name, + const GURL& file_system_root) { DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + FilePath virtual_path; if (success) - GrantPermissions(full_path); - Respond(success, full_path); + virtual_path = GrantPermissions(); + Respond(success, file_system_name, file_system_root, virtual_path); } -void FileHandlerSelectFileFunction::GrantPermissions( - const FilePath& full_path) { +FilePath FileHandlerSelectFileFunction::GrantPermissions() { fileapi::ExternalFileSystemMountPointProvider* external_provider = BrowserContext::GetFileSystemContext(profile_)->external_provider(); DCHECK(external_provider); FilePath virtual_path; - external_provider->GetVirtualPath(full_path, &virtual_path); + external_provider->GetVirtualPath(full_path_, &virtual_path); DCHECK(!virtual_path.empty()); - // |render_process_host_id_| might have been previously setup in api test. - if (!render_process_host_id_) - render_process_host_id_ = render_view_host()->GetProcess()->GetID(); - // Grant access to this particular file to target extension. This will // ensure that the target extension can access only this FS entry and // prevent from traversing FS hierarchy upward. external_provider->GrantFileAccessToExtension(extension_id(), virtual_path); content::ChildProcessSecurityPolicy::GetInstance()->GrantPermissionsForFile( - render_process_host_id_, - full_path, + render_view_host()->GetProcess()->GetID(), + full_path_, file_handler_util::GetReadWritePermissions()); -} -void FileHandlerSelectFileFunction::Respond(bool success, - const FilePath& full_path) { - GURL file_url; - if (success) { - file_manager_util::ConvertFileToFileSystemUrl(profile_, full_path, - source_url_.GetOrigin(), &file_url); - DCHECK(!file_url.is_empty()); - } + return virtual_path; +} +void FileHandlerSelectFileFunction::Respond( + bool success, + const std::string& file_system_name, + const GURL& file_system_root, + const FilePath& virtual_path) { scoped_ptr<SelectFile::Result::Result> result( new SelectFile::Result::Result()); result->success = success; - result->file_url = file_url.spec(); + if (success) { + result->entry.reset(new FileEntryInfo()); + result->entry->file_system_name = file_system_name; + result->entry->file_system_root = file_system_root.spec(); + result->entry->file_full_path = "/" + virtual_path.value(); + result->entry->file_is_directory = false; + } result_.reset(SelectFile::Result::Create(*result)); SendResponse(true); } +FileSelector* FileHandlerSelectFileFunction::GetFileSelector() { + FileSelector* result = file_selector_for_test_; + if (result) { + result->set_function_for_test(this); + return result; + } + return new FileSelectorImpl(this); +} + +FileSelector* FileHandlerSelectFileFunction::file_selector_for_test_ = NULL; + +bool FileHandlerSelectFileFunction::gesture_check_disabled_for_test_ = false; + diff --git a/chrome/browser/chromeos/extensions/file_browser_handler_api.h b/chrome/browser/chromeos/extensions/file_browser_handler_api.h index 36787ec..407811c 100644 --- a/chrome/browser/chromeos/extensions/file_browser_handler_api.h +++ b/chrome/browser/chromeos/extensions/file_browser_handler_api.h @@ -6,10 +6,13 @@ #define CHROME_BROWSER_CHROMEOS_EXTENSIONS_FILE_BROWSER_HANDLER_API_H_ #pragma once +#include <string> + +#include "base/file_path.h" #include "chrome/browser/extensions/extension_function.h" class Browser; -class FilePath; +class FileHandlerSelectFileFunction; namespace file_handler { @@ -19,6 +22,10 @@ class FileSelector { // Initiate file selection. virtual void SelectFile(const FilePath& suggested_name, Browser* browser) = 0; + + // Used in testing. + virtual void set_function_for_test( + FileHandlerSelectFileFunction* function) = 0; }; } // namespace file_handler @@ -33,45 +40,55 @@ class FileHandlerSelectFileFunction : public AsyncExtensionFunction { void OnFilePathSelected(bool success, const FilePath& full_path); // Used in test. - void set_file_selector_for_test(file_handler::FileSelector* file_selector) { - file_selector_ = file_selector; - } + static void set_file_selector_for_test( + file_handler::FileSelector* file_selector); // Used in test. - void set_render_process_host_id_for_test(int render_process_host_id) { - render_process_host_id_ = render_process_host_id; - } + static void set_gesture_check_disabled_for_test(bool disabled); protected: virtual bool RunImpl() OVERRIDE; private: - typedef base::Callback<void(bool, const FilePath&)> CreateFileCallback; - // Calls |DoCreateFile| on file thread and invokes the callback. - void CreateFileOnFileThread(const FilePath& full_path, - const CreateFileCallback& callback); + void CreateFileOnFileThread(bool success, + const std::string& file_system_name, + const GURL& file_system_root); // Creates file on provided file path. - bool DoCreateFile(const FilePath& full_path); + bool DoCreateFile(); // Called on UI thread after the file gets created. - void OnFileCreated(bool success, const FilePath& full_path); + void OnFileCreated(bool success, + const std::string& file_system_name, + const GURL& file_system_root); // Grants file access permissions for the created file to the extension with // cros mount point provider and child process security policy. - void GrantPermissions(const FilePath& full_path); + // Returns virtual path for which has been given permission. + FilePath GrantPermissions(); // Sends response to the extension. - void Respond(bool success, const FilePath& full_path); - - // |file_selector_| and |render_process_host_id_| are used primary in testing - // to override file selector and render process id that would normally be - // created (or determined) by the extension function. - file_handler::FileSelector* file_selector_; - int render_process_host_id_; - - DECLARE_EXTENSION_FUNCTION_NAME("fileBrowserHandler.selectFile"); + void Respond(bool success, + const std::string& file_system_name, + const GURL& file_system_root, + const FilePath& virtual_path); + + // Gets file selector that should be used to select the file. + // We should not take the ownership of the selector. + file_handler::FileSelector* GetFileSelector(); + + // Full file system path of the selected file. + FilePath full_path_; + + // |file_selector_for_test_| and |disable_geture_check_for_test_| are used + // primary in testing to override file selector to be used in the function + // implementation and disable user gesture check. + // Once set they will be used for every extension function call. + static file_handler::FileSelector* file_selector_for_test_; + static bool gesture_check_disabled_for_test_; + + DECLARE_EXTENSION_FUNCTION_NAME("fileBrowserHandlerInternal.selectFile"); }; #endif // CHROME_BROWSER_CHROMEOS_EXTENSIONS_FILE_BROWSER_HANDLER_API_H_ diff --git a/chrome/browser/chromeos/extensions/file_browser_handler_api_test.cc b/chrome/browser/chromeos/extensions/file_browser_handler_api_test.cc index 44cd94c..569d481 100644 --- a/chrome/browser/chromeos/extensions/file_browser_handler_api_test.cc +++ b/chrome/browser/chromeos/extensions/file_browser_handler_api_test.cc @@ -10,15 +10,10 @@ #include "base/values.h" #include "chrome/browser/extensions/extension_apitest.h" #include "chrome/browser/extensions/extension_function_test_utils.h" -#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" -#include "chrome/common/chrome_notification_types.h" #include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/ui_test_utils.h" -#include "content/public/browser/notification_service.h" -#include "content/public/browser/render_process_host.h" -#include "content/public/browser/site_instance.h" #include "testing/gmock/include/gmock/gmock.h" #include "webkit/fileapi/file_system_context.h" #include "webkit/fileapi/file_system_mount_point_provider.h" @@ -26,51 +21,12 @@ namespace utils = extension_function_test_utils; using ::testing::_; -using ::testing::Return; +using ::testing::Invoke; using content::BrowserContext; -using content::SiteInstance; using extensions::Extension; namespace { -int ExtractProcessFromExtensionId(const std::string& extension_id, - Profile* profile) { - GURL extension_url = - Extension::GetBaseURLFromExtensionId(extension_id); - ExtensionProcessManager* manager = profile->GetExtensionProcessManager(); - - SiteInstance* site_instance = manager->GetSiteInstanceForURL(extension_url); - if (!site_instance || !site_instance->HasProcess()) - return -1; - content::RenderProcessHost* process = site_instance->GetProcess(); - - return process->GetID(); -} - -GURL GetSourceURLForExtension(const std::string& extension_id) { - return GURL("chrome-extension://" + extension_id + "/"); -} - -GURL GetFileSystemURLForExtension(const std::string& extension_id, - const std::string& virtual_path) { - return GURL("filesystem:chrome-extension://" + extension_id + "/external/" + - virtual_path); -} - -class BackgroundObserver { - public: - BackgroundObserver() - : page_created_(chrome::NOTIFICATION_EXTENSION_BACKGROUND_PAGE_READY, - content::NotificationService::AllSources()) { - } - - void WaitUntilLoaded() { - page_created_.Wait(); - } - private: - ui_test_utils::WindowedNotificationObserver page_created_; -}; - class FileBrowserHandlerTest : public InProcessBrowserTest { }; @@ -96,17 +52,6 @@ class FileBrowserHandlerExtensionTest : public ExtensionApiTest { provider->AddLocalMountPoint(tmp_mount_point_); } - // Loads the extension under path derived from |test_name|, and waits until - // it's loaded. - const Extension* LoadExtensionAndWait(const std::string& test_name) { - BackgroundObserver page_complete; - FilePath extdir = test_data_dir_.AppendASCII(test_name); - const Extension* extension = LoadExtension(extdir); - if (extension) - page_complete.WaitUntilLoaded(); - return extension; - } - FilePath GetFullPathOnTmpMountPoint(const FilePath& relative_path) { return tmp_mount_point_.Append(relative_path); } @@ -116,20 +61,50 @@ class FileBrowserHandlerExtensionTest : public ExtensionApiTest { FilePath tmp_mount_point_; }; +// Action used to set mock expectations for SelectFile(). +ACTION_P3(MockFileSelectorResponse, fun, success, selected_path) { + base::MessageLoopProxy::current()->PostTask(FROM_HERE, + base::Bind(&FileHandlerSelectFileFunction::OnFilePathSelected, + base::Unretained(fun), success, selected_path)); +} + class MockFileSelector : public file_handler::FileSelector { public: MockFileSelector() {} virtual ~MockFileSelector() {} MOCK_METHOD2(SelectFile, void(const FilePath&, Browser*)); -}; -// Action used to set mock expectations for SelectFile(). -ACTION_P3(MockFileSelectorResponse, fun, success, selected_path) { - base::MessageLoopProxy::current()->PostTask(FROM_HERE, - base::Bind(&FileHandlerSelectFileFunction::OnFilePathSelected, - base::Unretained(fun), success, selected_path)); -} + virtual void set_function_for_test( + FileHandlerSelectFileFunction* function) OVERRIDE { + ASSERT_TRUE(!function_); + function_ = function; + } + + void InitStubSelectFile(bool success, const FilePath& selected_path) { + success_ = success; + selected_path_ = selected_path; + } + + void StubSelectFile(const FilePath& suggested_name, Browser* browser) { + base::MessageLoopProxy::current()->PostTask(FROM_HERE, + base::Bind(&FileHandlerSelectFileFunction::OnFilePathSelected, + function_, success_, selected_path_)); + function_ = NULL; + } + + void StubFail(const FilePath& suggested_name, Browser* browser) { + base::MessageLoopProxy::current()->PostTask(FROM_HERE, + base::Bind(&FileHandlerSelectFileFunction::OnFilePathSelected, + function_, false, FilePath())); + function_ = NULL; + } + + private: + scoped_refptr<FileHandlerSelectFileFunction> function_; + bool success_; + FilePath selected_path_; +}; void CheckSelectedFileContents(const FilePath& selected_path, const std::string& expected_contents) { @@ -140,77 +115,45 @@ void CheckSelectedFileContents(const FilePath& selected_path, } // namespace -IN_PROC_BROWSER_TEST_F(FileBrowserHandlerExtensionTest, Simple) { +IN_PROC_BROWSER_TEST_F(FileBrowserHandlerExtensionTest, EndToEnd) { AddTmpMountPoint(); - // Test extension that will try to write test file after we simulate - // selectFileForSave fucntion call. - const Extension* extension = - LoadExtensionAndWait("file_browser/filehandler_create"); - - ASSERT_TRUE(extension); - // Path that will be "selected" by file selector. - FilePath selected_path = + const FilePath selected_path = GetFullPathOnTmpMountPoint(FilePath("test_file.txt")); - scoped_refptr<FileHandlerSelectFileFunction> select_file_function( - new FileHandlerSelectFileFunction()); - // Setup file selector to return |selected_path|. scoped_ptr<MockFileSelector> mock_file_selector(new MockFileSelector()); + mock_file_selector->InitStubSelectFile(true, selected_path); EXPECT_CALL(*mock_file_selector, SelectFile(FilePath("some_file_name.txt"), _)) - .WillOnce(MockFileSelectorResponse(select_file_function.get(), true, - selected_path)); - select_file_function->set_file_selector_for_test( - mock_file_selector.get()); - - // Let's set the extension function as it was called with user gesture from - // the |extension|. - select_file_function->set_has_callback(true); - select_file_function->set_user_gesture(true); - select_file_function->set_extension(extension); + .WillOnce(Invoke(mock_file_selector.get(), + &MockFileSelector::StubSelectFile)); - int render_process_host_id = - ExtractProcessFromExtensionId(extension->id(), browser()->profile()); - select_file_function->set_render_process_host_id_for_test( - render_process_host_id); + // Let's make file selector fail for suggested name "fail". + EXPECT_CALL(*mock_file_selector, + SelectFile(FilePath("fail"), _)) + .WillOnce(Invoke(mock_file_selector.get(), &MockFileSelector::StubFail)); - GURL source_url = GetSourceURLForExtension(extension->id()); - select_file_function->set_source_url(source_url); + // Setup extension function parameters for the test. + FileHandlerSelectFileFunction::set_file_selector_for_test( + mock_file_selector.get()); + FileHandlerSelectFileFunction::set_gesture_check_disabled_for_test(true); // Selected path should still not exist. ASSERT_FALSE(file_util::PathExists(selected_path)); - // Invoke the method. - scoped_ptr<base::DictionaryValue> result(utils::ToDictionary( - utils::RunFunctionAndReturnResult( - select_file_function.get(), - "[{\"suggestedName\": \"some_file_name.txt\"}]", - browser()))); - - // Check that function results are as expected. - EXPECT_TRUE(utils::GetBoolean(result.get(), "success")); - EXPECT_EQ( - GetFileSystemURLForExtension(extension->id(), "tmp/test_file.txt").spec(), - utils::GetString(result.get(), "fileURL")); + ASSERT_TRUE(RunExtensionTest("file_browser/filehandler_create")) << message_; // Path should have been created by function call. ASSERT_TRUE(file_util::PathExists(selected_path)); - // Run test page in test extension that will verify that the extension can - // actually access and write the file. - ResultCatcher catcher; - GURL url = extension->GetResourceURL("test.html"); - ui_test_utils::NavigateToURL(browser(), url); - ASSERT_TRUE(catcher.GetNextResult()) << catcher.message(); - - std::string expected_contents = "hello from test extension."; + const std::string expected_contents = "hello from test extension."; content::BrowserThread::PostTask(content::BrowserThread::FILE, FROM_HERE, base::Bind(&CheckSelectedFileContents, selected_path, expected_contents)); - ui_test_utils::RunAllPendingInMessageLoop(); + FileHandlerSelectFileFunction::set_file_selector_for_test(NULL); + FileHandlerSelectFileFunction::set_gesture_check_disabled_for_test(false); } IN_PROC_BROWSER_TEST_F(FileBrowserHandlerTest, NoUserGesture) { @@ -221,7 +164,7 @@ IN_PROC_BROWSER_TEST_F(FileBrowserHandlerTest, NoUserGesture) { EXPECT_CALL(*mock_file_selector, SelectFile(_, _)) .Times(0); - select_file_function->set_file_selector_for_test( + FileHandlerSelectFileFunction::set_file_selector_for_test( mock_file_selector.get()); std::string error = @@ -230,7 +173,7 @@ IN_PROC_BROWSER_TEST_F(FileBrowserHandlerTest, NoUserGesture) { "[{\"suggestedName\": \"foo\"}]", browser()); - std::string expected_error = + const std::string expected_error = "This method can only be called in response to user gesture, such as a " "mouse click or key press."; EXPECT_EQ(expected_error, error); @@ -246,9 +189,9 @@ IN_PROC_BROWSER_TEST_F(FileBrowserHandlerTest, SelectionFailed) { .WillOnce(MockFileSelectorResponse(select_file_function.get(), false, FilePath())); - select_file_function->set_file_selector_for_test( + FileHandlerSelectFileFunction::set_file_selector_for_test( mock_file_selector.get()); - // Without a callback the function will not generate a result. + select_file_function->set_has_callback(true); select_file_function->set_user_gesture(true); @@ -259,7 +202,8 @@ IN_PROC_BROWSER_TEST_F(FileBrowserHandlerTest, SelectionFailed) { browser()))); EXPECT_FALSE(utils::GetBoolean(result.get(), "success")); - EXPECT_EQ("", utils::GetString(result.get(), "fileURL")); + DictionaryValue* entry_info; + EXPECT_FALSE(result->GetDictionary("entry", &entry_info)); } IN_PROC_BROWSER_TEST_F(FileBrowserHandlerTest, SuggestedFullPath) { @@ -272,9 +216,9 @@ IN_PROC_BROWSER_TEST_F(FileBrowserHandlerTest, SuggestedFullPath) { .WillOnce(MockFileSelectorResponse(select_file_function.get(), false, FilePath())); - select_file_function->set_file_selector_for_test( + FileHandlerSelectFileFunction::set_file_selector_for_test( mock_file_selector.get()); - // Without a callback the function will not generate a result. + select_file_function->set_has_callback(true); select_file_function->set_user_gesture(true); @@ -285,6 +229,7 @@ IN_PROC_BROWSER_TEST_F(FileBrowserHandlerTest, SuggestedFullPath) { browser()))); EXPECT_FALSE(utils::GetBoolean(result.get(), "success")); - EXPECT_EQ("", utils::GetString(result.get(), "fileURL")); + DictionaryValue* entry_info; + EXPECT_FALSE(result->GetDictionary("entry", &entry_info)); } diff --git a/chrome/common/extensions/api/api.gyp b/chrome/common/extensions/api/api.gyp index 86b98df..b87826c 100644 --- a/chrome/common/extensions/api/api.gyp +++ b/chrome/common/extensions/api/api.gyp @@ -21,7 +21,7 @@ 'events.json', 'experimental_font_settings.json', 'experimental_record.json', - 'file_browser_handler.json', + 'file_browser_handler_internal.json', 'permissions.json', 'storage.json', 'tabs.json', @@ -51,7 +51,7 @@ }], ['OS!="chromeos"', { 'json_schema_files!': [ - 'file_browser_handler.json', + 'file_browser_handler_internal.json', ], }], ], diff --git a/chrome/common/extensions/api/extension_api.cc b/chrome/common/extensions/api/extension_api.cc index 49ebd94..bbfeac4 100644 --- a/chrome/common/extensions/api/extension_api.cc +++ b/chrome/common/extensions/api/extension_api.cc @@ -389,6 +389,8 @@ void ExtensionAPI::InitDefaultConfiguration() { IDR_EXTENSION_API_JSON_EXTENSION)); RegisterSchema("fileBrowserHandler", ReadFromResource( IDR_EXTENSION_API_JSON_FILEBROWSERHANDLER)); + RegisterSchema("fileBrowserHandlerInternal", ReadFromResource( + IDR_EXTENSION_API_JSON_FILEBROWSERHANDLERINTERNAL)); RegisterSchema("fileBrowserPrivate", ReadFromResource( IDR_EXTENSION_API_JSON_FILEBROWSERPRIVATE)); RegisterSchema("history", ReadFromResource( diff --git a/chrome/common/extensions/api/file_browser_handler.json b/chrome/common/extensions/api/file_browser_handler.json index 539a1a7..56b27a6 100644 --- a/chrome/common/extensions/api/file_browser_handler.json +++ b/chrome/common/extensions/api/file_browser_handler.json @@ -13,7 +13,8 @@ "description": "Event details payload for fileBrowserHandler.onExecute event.", "properties": { "entries": { - "type": "any", + "type": "array", + "items": { "type": "object", "constructor": "Entry" }, "description": "Array of Entry instances representing files that are targets of this action (selected in ChromeOS file browser)." }, "tab_id" : { @@ -49,12 +50,12 @@ { "name": "selectFile", "type": "function", - "description": "Prompts user to select file path under which a new file will be created. If the user selects file, the file gets created or, if it already exists, truncated. The function has to be called with user gesture.", + "description": "Prompts user to select file path under which a new file will be created. When the user selects file, the file gets created or, if it already existed, truncated. The function has to be called with a user gesture.", "parameters": [ { "name": "selectionParams", "type": "object", - "description": "Parameters that will be used to create new file.", + "description": "Parameters that will be used to create a new file.", "properties": { "suggestedName": { "type": "string", @@ -76,9 +77,11 @@ "type": "boolean", "description": "Has the file been selected." }, - "fileURL": { - "type": "string", - "description": "Filesystem URL of the selected file." + "entry": { + "type": "object", + "constructor": "Entry", + "optional": true, + "description": "Selected file entry. It will be null if a file hasn't been selected." } } } diff --git a/chrome/common/extensions/api/file_browser_handler_internal.json b/chrome/common/extensions/api/file_browser_handler_internal.json new file mode 100644 index 0000000..455bce8 --- /dev/null +++ b/chrome/common/extensions/api/file_browser_handler_internal.json @@ -0,0 +1,77 @@ +// 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. + +[ + { + "namespace": "fileBrowserHandlerInternal", + "nodoc": true, + "internal": true, + "platforms": ["chromeos"], + "types": [ + { + "id": "FileEntryInfo", + "type": "object", + "description": "Information needed to build a file entry that will be returned through the API.", + "properties": { + "fileSystemName": { + "type": "string" + }, + "fileSystemRoot": { + "type": "string" + }, + "fileFullPath": { + "type": "string" + }, + "fileIsDirectory": { + "type": "boolean" + } + } + } + ], + + "functions": [ + { + "name": "selectFile", + "type": "function", + "description": "Prompts user to select file path under which a new file will be created. If the user selects file, the file gets created or, if it already exists, truncated. The function has to be called with user gesture.", + "parameters": [ + { + "name": "selectionParams", + "type": "object", + "description": "Parameters that will be used to create new file.", + "properties": { + "suggestedName": { + "type": "string", + "description": "Suggested name for the new file." + } + } + }, + { + "name": "callback", + "type": "function", + "description": "Function called upon completion.", + "parameters": [ + { + "name": "result", + "description": "Result of the method.", + "type": "object", + "properties": { + "success": { + "type": "boolean", + "description": "Has the file been selected." + }, + "entry": { + "$ref": "FileEntryInfo", + "optional": true, + "description": "Selected file entry." + } + } + } + ] + } + ] + } + ] + } +] diff --git a/chrome/common/extensions/extension_permission_set.cc b/chrome/common/extensions/extension_permission_set.cc index c62b9ba..1380f9e 100644 --- a/chrome/common/extensions/extension_permission_set.cc +++ b/chrome/common/extensions/extension_permission_set.cc @@ -255,6 +255,8 @@ void ExtensionAPIPermission::RegisterAllPermissions( // Register private permissions. { kChromeosInfoPrivate, "chromeosInfoPrivate", kFlagCannotBeOptional }, + { kFileBrowserHandlerInternal, "fileBrowserHandlerInternal", + kFlagCannotBeOptional }, { kFileBrowserPrivate, "fileBrowserPrivate", kFlagCannotBeOptional }, { kManagedModePrivate, "managedModePrivate", kFlagCannotBeOptional }, { kMediaPlayerPrivate, "mediaPlayerPrivate", kFlagCannotBeOptional }, @@ -846,6 +848,10 @@ void ExtensionPermissionSet::InitImplicitExtensionPermissions( if (apis_.find(ExtensionAPIPermission::kWebRequest) != apis_.end()) apis_.insert(ExtensionAPIPermission::kWebRequestInternal); + // The fileBrowserHandler permission implies the internal version as well. + if (apis_.find(ExtensionAPIPermission::kFileBrowserHandler) != apis_.end()) + apis_.insert(ExtensionAPIPermission::kFileBrowserHandlerInternal); + // Add the scriptable hosts. for (UserScriptList::const_iterator content_script = extension->content_scripts().begin(); diff --git a/chrome/common/extensions/extension_permission_set.h b/chrome/common/extensions/extension_permission_set.h index 2b247df..239d266 100644 --- a/chrome/common/extensions/extension_permission_set.h +++ b/chrome/common/extensions/extension_permission_set.h @@ -122,6 +122,7 @@ class ExtensionAPIPermission { kEchoPrivate, kExperimental, kFileBrowserHandler, + kFileBrowserHandlerInternal, kFileBrowserPrivate, kFileSystem, kGeolocation, diff --git a/chrome/common/extensions/extension_permission_set_unittest.cc b/chrome/common/extensions/extension_permission_set_unittest.cc index a5e0473..24efd54 100644 --- a/chrome/common/extensions/extension_permission_set_unittest.cc +++ b/chrome/common/extensions/extension_permission_set_unittest.cc @@ -619,6 +619,7 @@ TEST(ExtensionPermissionsTest, PermissionMessages) { // These are private. skip.insert(ExtensionAPIPermission::kChromeAuthPrivate); skip.insert(ExtensionAPIPermission::kChromeosInfoPrivate); + skip.insert(ExtensionAPIPermission::kFileBrowserHandlerInternal); skip.insert(ExtensionAPIPermission::kFileBrowserPrivate); skip.insert(ExtensionAPIPermission::kInputMethodPrivate); skip.insert(ExtensionAPIPermission::kManagedModePrivate); diff --git a/chrome/common/extensions_api_resources.grd b/chrome/common/extensions_api_resources.grd index 904a92a..e45c446 100644 --- a/chrome/common/extensions_api_resources.grd +++ b/chrome/common/extensions_api_resources.grd @@ -40,6 +40,7 @@ <include name="IDR_EXTENSION_API_JSON_EXPERIMENTAL_SPEECHINPUT" file="extensions\api\experimental_speech_input.json" type="BINDATA" /> <include name="IDR_EXTENSION_API_JSON_EXTENSION" file="extensions\api\extension.json" type="BINDATA" /> <include name="IDR_EXTENSION_API_JSON_FILEBROWSERHANDLER" file="extensions\api\file_browser_handler.json" type="BINDATA" /> + <include name="IDR_EXTENSION_API_JSON_FILEBROWSERHANDLERINTERNAL" file="extensions\api\file_browser_handler_internal.json" type="BINDATA" /> <include name="IDR_EXTENSION_API_JSON_FILEBROWSERPRIVATE" file="extensions\api\file_browser_private.json" type="BINDATA" /> <include name="IDR_EXTENSION_API_JSON_HISTORY" file="extensions\api\history.json" type="BINDATA" /> <include name="IDR_EXTENSION_API_JSON_I18N" file="extensions\api\i18n.json" type="BINDATA" /> diff --git a/chrome/renderer/resources/extensions/file_browser_handler_custom_bindings.js b/chrome/renderer/resources/extensions/file_browser_handler_custom_bindings.js index 44d1539..4ce96be 100644 --- a/chrome/renderer/resources/extensions/file_browser_handler_custom_bindings.js +++ b/chrome/renderer/resources/extensions/file_browser_handler_custom_bindings.js @@ -22,3 +22,26 @@ chromeHidden.Event.registerArgumentMassager('fileBrowserHandler.onExecute', for (var i = 0; i < fileList.length; i++) fileList[i] = GetExternalFileEntry(fileList[i]); }); + +chromeHidden.registerCustomHook('fileBrowserHandler', function(bindingsAPI) { + var apiFunctions = bindingsAPI.apiFunctions; + + apiFunctions.setHandleRequest('selectFile', + function(selectionParams, callback) { + function internalCallback(externalCallback, internalResult) { + if (!externalCallback) + return; + var result = undefined; + if (internalResult) { + result = { success: internalResult.success, entry: null }; + if (internalResult.success) + result.entry = GetExternalFileEntry(internalResult.entry); + } + + externalCallback(result); + } + + return chromeHidden.internalAPIs.fileBrowserHandlerInternal.selectFile( + selectionParams, internalCallback.bind(null, callback)); + }); +}); diff --git a/chrome/test/data/extensions/api_test/file_browser/filehandler_create/background.js b/chrome/test/data/extensions/api_test/file_browser/filehandler_create/background.js deleted file mode 100644 index bb27ac3..0000000 --- a/chrome/test/data/extensions/api_test/file_browser/filehandler_create/background.js +++ /dev/null @@ -1,7 +0,0 @@ -// 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. - -chrome.fileBrowserHandler.onExecute.addListener(function(tasks) { - console.log('do nothing'); -}); diff --git a/chrome/test/data/extensions/api_test/file_browser/filehandler_create/manifest.json b/chrome/test/data/extensions/api_test/file_browser/filehandler_create/manifest.json index c1f98f8..cef8403 100644 --- a/chrome/test/data/extensions/api_test/file_browser/filehandler_create/manifest.json +++ b/chrome/test/data/extensions/api_test/file_browser/filehandler_create/manifest.json @@ -5,7 +5,7 @@ "manifest_version": 2, "description": "Tests of chrome.fileBrowserHandler.selectFile method", "background": { - "scripts": ["background.js"] + "scripts": ["test.js"] }, "permissions": [ "fileBrowserHandler", diff --git a/chrome/test/data/extensions/api_test/file_browser/filehandler_create/test.html b/chrome/test/data/extensions/api_test/file_browser/filehandler_create/test.html deleted file mode 100644 index 9bd1bb7..0000000 --- a/chrome/test/data/extensions/api_test/file_browser/filehandler_create/test.html +++ /dev/null @@ -1,3 +0,0 @@ -<html> -<script src='test.js'></script> -</html> diff --git a/chrome/test/data/extensions/api_test/file_browser/filehandler_create/test.js b/chrome/test/data/extensions/api_test/file_browser/filehandler_create/test.js index 4821aeb..ce67deb 100644 --- a/chrome/test/data/extensions/api_test/file_browser/filehandler_create/test.js +++ b/chrome/test/data/extensions/api_test/file_browser/filehandler_create/test.js @@ -35,10 +35,6 @@ function errorCallback(error) { chrome.test.fail(msg); } -function getFileSystemUrlForPath(path) { - return 'filesystem:chrome-extension://' + EXTENSION_ID + '/external/' + path; -} - function writeToFile(entry) { entry.createWriter(function(writer) { writer.onerror = function(e) { @@ -49,14 +45,31 @@ function writeToFile(entry) { var bb = new WebKitBlobBuilder(); bb.append(FILE_CONTENTS); writer.write(bb.getBlob('text/plain')); - }); + }, errorCallback); } -chrome.test.runTests([function getFile() { - // The test will try to open and write to a file for which - // fileBrowserHandler.selectFile has previously been called (on C++ side of - // the test). It verifies that the permissions given by the method allow the - // extension to read/write to selected file. - var entryUrl = getFileSystemUrlForPath('tmp/test_file.txt'); - window.webkitResolveLocalFileSystemURL(entryUrl, writeToFile, errorCallback); -}]); +chrome.test.runTests([ + function selectionSuccessful() { + // The test will call selectFile function and expect it to succeed. + // When it gets the file entry, it verifies that the permissions given in + // the method allow the extension to read/write to selected file. + chrome.fileBrowserHandler.selectFile({ suggestedName: 'some_file_name.txt'}, + function(result) { + chrome.test.assertTrue(!!result); + chrome.test.assertTrue(result.success); + chrome.test.assertTrue(!!result.entry); + + writeToFile(result.entry); + }); + }, + function selectionFails() { + // The test expects that selectFile returns failure with an empty entry. + chrome.fileBrowserHandler.selectFile({ suggestedName: 'fail' }, + function(result) { + chrome.test.assertTrue(!!result); + // Entry should be set iff operation succeeded. + chrome.test.assertEq(false, result.success); + chrome.test.assertTrue(result.entry == null); + chrome.test.succeed(); + }); + }]); |