diff options
author | sammc@chromium.org <sammc@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-08-29 10:02:25 +0000 |
---|---|---|
committer | sammc@chromium.org <sammc@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-08-29 10:02:25 +0000 |
commit | ffb8706267743f92faa5314e1e61c7c76c65e0dd (patch) | |
tree | 4aa4841c888e7fb0a518850d9b383506a693b75e /apps | |
parent | a39dcb42c9cc46fa6598744d043f3461c542d967 (diff) | |
download | chromium_src-ffb8706267743f92faa5314e1e61c7c76c65e0dd.zip chromium_src-ffb8706267743f92faa5314e1e61c7c76c65e0dd.tar.gz chromium_src-ffb8706267743f92faa5314e1e61c7c76c65e0dd.tar.bz2 |
Use only app permissions to decide if file entries should be writable.
Previously, apps with the fileSystem.write permission could choose to
obtain read-only files and file entries provided as launch data were
always read-only.
With this change, all file entries obtained using the chrome.fileSystem
API or provided as launch data are writable, for apps with the
fileSystem.write permission.
This makes getWritableEntry unnecessary, other than for files obtained
through other means (e.g., drag and drop).
BUG=148486
Review URL: https://chromiumcodereview.appspot.com/23202006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@220254 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'apps')
-rw-r--r-- | apps/app_restore_service_browsertest.cc | 3 | ||||
-rw-r--r-- | apps/launcher.cc | 41 | ||||
-rw-r--r-- | apps/saved_files_service.cc | 27 | ||||
-rw-r--r-- | apps/saved_files_service.h | 7 | ||||
-rw-r--r-- | apps/saved_files_service_unittest.cc | 15 |
5 files changed, 46 insertions, 47 deletions
diff --git a/apps/app_restore_service_browsertest.cc b/apps/app_restore_service_browsertest.cc index 88ceb65..cdde267 100644 --- a/apps/app_restore_service_browsertest.cc +++ b/apps/app_restore_service_browsertest.cc @@ -136,8 +136,7 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, MAYBE_FileAccessIsRestored) { extension_prefs->SetExtensionRunning(extension->id(), true); for (std::vector<SavedFileEntry>::const_iterator it = file_entries.begin(); it != file_entries.end(); ++it) { - saved_files_service->RegisterFileEntry( - extension->id(), it->id, it->path, it->writable); + saved_files_service->RegisterFileEntry(extension->id(), it->id, it->path); } apps::AppRestoreServiceFactory::GetForProfile(browser()->profile())-> diff --git a/apps/launcher.cc b/apps/launcher.cc index e5990a5..d3bc43c 100644 --- a/apps/launcher.cc +++ b/apps/launcher.cc @@ -46,11 +46,13 @@ namespace app_runtime = extensions::api::app_runtime; using content::BrowserThread; +using extensions::app_file_handler_util::CheckWritableFiles; using extensions::app_file_handler_util::FileHandlerForId; using extensions::app_file_handler_util::FileHandlerCanHandleFile; using extensions::app_file_handler_util::FirstFileHandlerForFile; using extensions::app_file_handler_util::CreateFileEntry; using extensions::app_file_handler_util::GrantedFileEntry; +using extensions::app_file_handler_util::HasFileSystemWritePermission; using extensions::Extension; using extensions::ExtensionHost; using extensions::ExtensionSystem; @@ -125,15 +127,18 @@ class PlatformAppPathLauncher DCHECK(file_path_.IsAbsolute()); -#if defined(OS_CHROMEOS) - if (drive::util::IsUnderDriveMountPoint(file_path_)) { - GetMimeTypeAndLaunchForDriveFile(); + if (HasFileSystemWritePermission(extension_)) { + std::vector<base::FilePath> paths; + paths.push_back(file_path_); + CheckWritableFiles( + paths, + profile_, + base::Bind(&PlatformAppPathLauncher::OnFileValid, this), + base::Bind(&PlatformAppPathLauncher::OnFileInvalid, this)); return; } -#endif - BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, base::Bind( - &PlatformAppPathLauncher::GetMimeTypeAndLaunch, this)); + OnFileValid(); } void LaunchWithHandler(const std::string& handler_id) { @@ -146,6 +151,24 @@ class PlatformAppPathLauncher virtual ~PlatformAppPathLauncher() {} + void OnFileValid() { +#if defined(OS_CHROMEOS) + if (drive::util::IsUnderDriveMountPoint(file_path_)) { + PlatformAppPathLauncher::GetMimeTypeAndLaunchForDriveFile(); + return; + } +#endif + + BrowserThread::PostTask( + BrowserThread::FILE, + FROM_HERE, + base::Bind(&PlatformAppPathLauncher::GetMimeTypeAndLaunch, this)); + } + + void OnFileInvalid(const base::FilePath& /* error_path */) { + LaunchWithNoLaunchData(); + } + void GetMimeTypeAndLaunch() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); @@ -261,11 +284,7 @@ class PlatformAppPathLauncher } GrantedFileEntry file_entry = CreateFileEntry( - profile_, - extension_->id(), - host->render_process_host()->GetID(), - file_path_, - false); + profile_, extension_, host->render_process_host()->GetID(), file_path_); extensions::AppEventRouter::DispatchOnLaunchedEventWithFileEntry( profile_, extension_, handler_id_, mime_type, file_entry); } diff --git a/apps/saved_files_service.cc b/apps/saved_files_service.cc index fcf0248..aa64af9 100644 --- a/apps/saved_files_service.cc +++ b/apps/saved_files_service.cc @@ -36,9 +36,6 @@ const char kFileEntries[] = "file_entries"; // The path to a file entry that the app had permission to access. const char kFileEntryPath[] = "path"; -// Whether or not the app had write access to a file entry. -const char kFileEntryWritable[] = "writable"; - // The sequence number in the LRU of the file entry. const char kFileEntrySequenceNumber[] = "sequence_number"; @@ -62,7 +59,6 @@ void AddSavedFileEntry(ExtensionPrefs* prefs, DictionaryValue* file_entry_dict = new DictionaryValue(); file_entry_dict->Set(kFileEntryPath, CreateFilePathValue(file_entry.path)); - file_entry_dict->SetBoolean(kFileEntryWritable, file_entry.writable); file_entry_dict->SetInteger(kFileEntrySequenceNumber, file_entry.sequence_number); file_entries->SetWithoutPathExpansion(file_entry.id, file_entry_dict); @@ -122,31 +118,25 @@ std::vector<SavedFileEntry> GetSavedFileEntries( base::FilePath file_path; if (!GetValueAsFilePath(*path_value, &file_path)) continue; - bool writable = false; - if (!file_entry->GetBoolean(kFileEntryWritable, &writable)) - continue; int sequence_number = 0; if (!file_entry->GetInteger(kFileEntrySequenceNumber, &sequence_number)) continue; if (!sequence_number) continue; - result.push_back( - SavedFileEntry(it.key(), file_path, writable, sequence_number)); + result.push_back(SavedFileEntry(it.key(), file_path, sequence_number)); } return result; } } // namespace -SavedFileEntry::SavedFileEntry() : writable(false), sequence_number(0) {} +SavedFileEntry::SavedFileEntry() : sequence_number(0) {} SavedFileEntry::SavedFileEntry(const std::string& id, const base::FilePath& path, - bool writable, int sequence_number) : id(id), path(path), - writable(writable), sequence_number(sequence_number) {} class SavedFilesService::SavedFiles { @@ -155,8 +145,7 @@ class SavedFilesService::SavedFiles { ~SavedFiles(); void RegisterFileEntry(const std::string& id, - const base::FilePath& file_path, - bool writable); + const base::FilePath& file_path); void EnqueueFileEntry(const std::string& id); bool IsRegistered(const std::string& id) const; const SavedFileEntry* GetFileEntry(const std::string& id) const; @@ -230,9 +219,8 @@ void SavedFilesService::Observe(int type, void SavedFilesService::RegisterFileEntry(const std::string& extension_id, const std::string& id, - const base::FilePath& file_path, - bool writable) { - GetOrInsert(extension_id)->RegisterFileEntry(id, file_path, writable); + const base::FilePath& file_path) { + GetOrInsert(extension_id)->RegisterFileEntry(id, file_path); } void SavedFilesService::EnqueueFileEntry(const std::string& extension_id, @@ -315,13 +303,12 @@ SavedFilesService::SavedFiles::~SavedFiles() {} void SavedFilesService::SavedFiles::RegisterFileEntry( const std::string& id, - const base::FilePath& file_path, - bool writable) { + const base::FilePath& file_path) { if (ContainsKey(registered_file_entries_, id)) return; registered_file_entries_.insert( - std::make_pair(id, new SavedFileEntry(id, file_path, writable, 0))); + std::make_pair(id, new SavedFileEntry(id, file_path, 0))); } void SavedFilesService::SavedFiles::EnqueueFileEntry(const std::string& id) { diff --git a/apps/saved_files_service.h b/apps/saved_files_service.h index 48a9a42..25b96d7 100644 --- a/apps/saved_files_service.h +++ b/apps/saved_files_service.h @@ -37,7 +37,6 @@ struct SavedFileEntry { SavedFileEntry(const std::string& id, const base::FilePath& path, - bool writable, int sequence_number); // The opaque id of this file entry. @@ -46,9 +45,6 @@ struct SavedFileEntry { // The path to a file entry that the app had permission to access. base::FilePath path; - // Whether or not the app had write access to a file entry. - bool writable; - // The sequence number in the LRU of the file entry. The value 0 indicates // that the entry is not in the LRU. int sequence_number; @@ -69,8 +65,7 @@ class SavedFilesService : public BrowserContextKeyedService, // object construction are automatically registered. void RegisterFileEntry(const std::string& extension_id, const std::string& id, - const base::FilePath& file_path, - bool writable); + const base::FilePath& file_path); // If the file with |id| is not in the queue of files to be retained // permanently, adds the file to the back of the queue, evicting the least diff --git a/apps/saved_files_service_unittest.cc b/apps/saved_files_service_unittest.cc index cc66c35..336049b 100644 --- a/apps/saved_files_service_unittest.cc +++ b/apps/saved_files_service_unittest.cc @@ -71,7 +71,6 @@ class SavedFilesServiceUnitTest : public testing::Test { ASSERT_TRUE(entry); EXPECT_EQ(id_string, entry->id); EXPECT_EQ(path_, entry->path); - EXPECT_TRUE(entry->writable); EXPECT_EQ(sequence_number, entry->sequence_number); } @@ -90,9 +89,9 @@ class SavedFilesServiceUnitTest : public testing::Test { }; TEST_F(SavedFilesServiceUnitTest, RetainTwoFilesTest) { - service_->RegisterFileEntry(extension_->id(), GenerateId(1), path_, true); - service_->RegisterFileEntry(extension_->id(), GenerateId(2), path_, true); - service_->RegisterFileEntry(extension_->id(), GenerateId(3), path_, true); + service_->RegisterFileEntry(extension_->id(), GenerateId(1), path_); + service_->RegisterFileEntry(extension_->id(), GenerateId(2), path_); + service_->RegisterFileEntry(extension_->id(), GenerateId(3), path_); // Test that no entry has a sequence number. TRACE_CALL(CheckEntrySequenceNumber(1, 0)); @@ -152,7 +151,7 @@ TEST_F(SavedFilesServiceUnitTest, NoRetainEntriesPermissionTest) { extension_ = env_.MakeExtension(*base::test::ParseJson( "{\"app\": {\"background\": {\"scripts\": [\"background.js\"]}}," "\"permissions\": [\"fileSystem\"]}")); - service_->RegisterFileEntry(extension_->id(), GenerateId(1), path_, true); + service_->RegisterFileEntry(extension_->id(), GenerateId(1), path_); TRACE_CALL(CheckEntrySequenceNumber(1, 0)); SavedFileEntry entry; service_->EnqueueFileEntry(extension_->id(), GenerateId(1)); @@ -171,10 +170,10 @@ TEST_F(SavedFilesServiceUnitTest, NoRetainEntriesPermissionTest) { TEST_F(SavedFilesServiceUnitTest, EvictionTest) { SavedFilesService::SetLruSizeForTest(10); for (int i = 0; i < 10; i++) { - service_->RegisterFileEntry(extension_->id(), GenerateId(i), path_, true); + service_->RegisterFileEntry(extension_->id(), GenerateId(i), path_); service_->EnqueueFileEntry(extension_->id(), GenerateId(i)); } - service_->RegisterFileEntry(extension_->id(), GenerateId(10), path_, true); + service_->RegisterFileEntry(extension_->id(), GenerateId(10), path_); // Expect that entries 0 to 9 are in the queue, but 10 is not. TRACE_CALL(CheckRangeEnqueuedInOrder(0, 10)); @@ -209,7 +208,7 @@ TEST_F(SavedFilesServiceUnitTest, SequenceNumberCompactionTest) { SavedFilesService::SetMaxSequenceNumberForTest(8); SavedFilesService::SetLruSizeForTest(8); for (int i = 0; i < 4; i++) { - service_->RegisterFileEntry(extension_->id(), GenerateId(i), path_, true); + service_->RegisterFileEntry(extension_->id(), GenerateId(i), path_); service_->EnqueueFileEntry(extension_->id(), GenerateId(i)); } service_->EnqueueFileEntry(extension_->id(), GenerateId(2)); |