diff options
54 files changed, 1424 insertions, 450 deletions
diff --git a/apps/app_restore_service.cc b/apps/app_restore_service.cc index a567811..88ac811 100644 --- a/apps/app_restore_service.cc +++ b/apps/app_restore_service.cc @@ -4,8 +4,8 @@ #include "apps/app_restore_service.h" +#include "apps/saved_files_service.h" #include "chrome/browser/extensions/api/app_runtime/app_runtime_api.h" -#include "chrome/browser/extensions/api/file_handlers/app_file_handler_util.h" #include "chrome/browser/extensions/event_router.h" #include "chrome/browser/extensions/extension_host.h" #include "chrome/browser/extensions/extension_prefs.h" @@ -70,13 +70,16 @@ void AppRestoreService::HandleStartup(bool should_restore_apps) { it != extensions->end(); ++it) { const Extension* extension = *it; if (extension_prefs->IsExtensionRunning(extension->id())) { - std::vector<SavedFileEntry> file_entries; - extensions::app_file_handler_util::GetSavedFileEntries(extension_prefs, - extension->id(), - &file_entries); RecordAppStop(extension->id()); - if (should_restore_apps) - RestoreApp(*it, file_entries); + // If we are not restoring apps (e.g., because it is a clean restart), and + // the app does not have retain permission, explicitly clear the retained + // entries queue. + if (should_restore_apps) { + RestoreApp(*it); + } else { + SavedFilesService::Get(profile_)->ClearQueueIfNoRetainPermission( + extension); + } } } } @@ -138,8 +141,6 @@ void AppRestoreService::RecordAppStop(const std::string& extension_id) { ExtensionPrefs* extension_prefs = ExtensionSystem::Get(profile_)->extension_service()->extension_prefs(); extension_prefs->SetExtensionRunning(extension_id, false); - extensions::app_file_handler_util::ClearSavedFileEntries( - extension_prefs, extension_id); } void AppRestoreService::RecordIfAppHasWindows( @@ -162,12 +163,8 @@ void AppRestoreService::RecordIfAppHasWindows( extension_prefs->SetHasWindows(id, has_windows); } -void AppRestoreService::RestoreApp( - const Extension* extension, - const std::vector<SavedFileEntry>& file_entries) { - extensions::RestartPlatformAppWithFileEntries(profile_, - extension, - file_entries); +void AppRestoreService::RestoreApp(const Extension* extension) { + extensions::RestartPlatformApp(profile_, extension); } void AppRestoreService::StartObservingShellWindows() { diff --git a/apps/app_restore_service.h b/apps/app_restore_service.h index 97dcd88..65d3de2 100644 --- a/apps/app_restore_service.h +++ b/apps/app_restore_service.h @@ -15,17 +15,10 @@ namespace extensions { class Extension; - -namespace app_file_handler_util { -struct SavedFileEntry; -} - } class Profile; -using extensions::app_file_handler_util::SavedFileEntry; - namespace apps { // Tracks what apps need to be restarted when the browser restarts. @@ -61,9 +54,7 @@ class AppRestoreService : public BrowserContextKeyedService, void RecordAppStop(const std::string& extension_id); void RecordIfAppHasWindows(const std::string& id); - void RestoreApp( - const extensions::Extension* extension, - const std::vector<SavedFileEntry>& file_entries); + void RestoreApp(const extensions::Extension* extension); void StartObservingShellWindows(); void StopObservingShellWindows(); diff --git a/apps/app_restore_service_browsertest.cc b/apps/app_restore_service_browsertest.cc index ce800a4..cf832fe 100644 --- a/apps/app_restore_service_browsertest.cc +++ b/apps/app_restore_service_browsertest.cc @@ -4,7 +4,7 @@ #include "apps/app_restore_service.h" #include "apps/app_restore_service_factory.h" -#include "chrome/browser/extensions/api/file_handlers/app_file_handler_util.h" +#include "apps/saved_files_service.h" #include "chrome/browser/extensions/api/file_system/file_system_api.h" #include "chrome/browser/extensions/extension_prefs.h" #include "chrome/browser/extensions/extension_test_message_listener.h" @@ -12,7 +12,6 @@ #include "chrome/common/extensions/extension.h" #include "content/public/test/test_utils.h" -using extensions::app_file_handler_util::SavedFileEntry; using extensions::Extension; using extensions::ExtensionPrefs; using extensions::ExtensionSystem; @@ -78,21 +77,15 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, FileAccessIsSavedToPrefs) { ASSERT_TRUE(extension); file_written_listener.WaitUntilSatisfied(); - ExtensionPrefs* extension_prefs = - ExtensionPrefs::Get(browser()->profile()); + SavedFilesService* saved_files_service = SavedFilesService::Get(profile()); - // Record the file entries in prefs because when the app gets suspended it - // will have them all cleared. - std::vector<SavedFileEntry> file_entries; - extensions::app_file_handler_util::GetSavedFileEntries( - extension_prefs, extension->id(), &file_entries); + std::vector<SavedFileEntry> file_entries = + saved_files_service->GetAllFileEntries(extension->id()); // One for the read-only file entry and one for the writable file entry. ASSERT_EQ(2u, file_entries.size()); extension_suspended.Wait(); - file_entries.clear(); - extensions::app_file_handler_util::GetSavedFileEntries( - extension_prefs, extension->id(), &file_entries); + file_entries = saved_files_service->GetAllFileEntries(extension->id()); // File entries should be cleared when the extension is suspended. ASSERT_TRUE(file_entries.empty()); } @@ -124,12 +117,9 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, FileAccessIsRestored) { ExtensionPrefs* extension_prefs = ExtensionPrefs::Get(browser()->profile()); - // Record the file entries in prefs because when the app gets suspended it - // will have them all cleared. - std::vector<SavedFileEntry> file_entries; - extensions::app_file_handler_util::GetSavedFileEntries(extension_prefs, - extension->id(), - &file_entries); + SavedFilesService* saved_files_service = SavedFilesService::Get(profile()); + std::vector<SavedFileEntry> file_entries = + saved_files_service->GetAllFileEntries(extension->id()); extension_suspended.Wait(); // Simulate a restart by populating the preferences as if the browser didn't @@ -137,8 +127,8 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, FileAccessIsRestored) { extension_prefs->SetExtensionRunning(extension->id(), true); for (std::vector<SavedFileEntry>::const_iterator it = file_entries.begin(); it != file_entries.end(); ++it) { - extensions::app_file_handler_util::AddSavedFileEntry( - extension_prefs, extension->id(), it->id, it->path, it->writable); + saved_files_service->RegisterFileEntry( + extension->id(), it->id, it->path, it->writable); } apps::AppRestoreServiceFactory::GetForProfile(browser()->profile())-> diff --git a/apps/apps.gypi b/apps/apps.gypi index 6ba1011..f6bfa6c 100644 --- a/apps/apps.gypi +++ b/apps/apps.gypi @@ -42,6 +42,10 @@ 'pref_names.h', 'prefs.cc', 'prefs.h', + 'saved_files_service.cc', + 'saved_files_service.h', + 'saved_files_service_factory.cc', + 'saved_files_service_factory.h', 'shell_window_geometry_cache.cc', 'shell_window_geometry_cache.h', 'shortcut_manager.cc', diff --git a/apps/saved_files_service.cc b/apps/saved_files_service.cc new file mode 100644 index 0000000..c509c08 --- /dev/null +++ b/apps/saved_files_service.cc @@ -0,0 +1,441 @@ +// Copyright 2013 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 "apps/saved_files_service.h" + +#include <algorithm> + +#include "apps/saved_files_service_factory.h" +#include "base/basictypes.h" +#include "base/hash_tables.h" +#include "base/value_conversions.h" +#include "chrome/browser/extensions/extension_host.h" +#include "chrome/browser/extensions/extension_prefs.h" +#include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/extension_system.h" +#include "chrome/common/extensions/permissions/api_permission.h" +#include "chrome/common/extensions/permissions/permission_set.h" + +namespace apps { + +using extensions::APIPermission; +using extensions::Extension; +using extensions::ExtensionHost; +using extensions::ExtensionPrefs; + +namespace { + +// Preference keys + +// The file entries that the app has permission to access. +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"; + +const size_t kMaxSavedFileEntries = 500; +const int kMaxSequenceNumber = kint32max; + +// These might be different to the constant values in tests. +size_t g_max_saved_file_entries = kMaxSavedFileEntries; +int g_max_sequence_number = kMaxSequenceNumber; + +// Persists a SavedFileEntry in ExtensionPrefs. +void AddSavedFileEntry(ExtensionPrefs* prefs, + const std::string& extension_id, + const SavedFileEntry& file_entry) { + ExtensionPrefs::ScopedDictionaryUpdate update( + prefs, extension_id, kFileEntries); + DictionaryValue* file_entries = update.Get(); + if (!file_entries) + file_entries = update.Create(); + DCHECK(!file_entries->GetDictionaryWithoutPathExpansion(file_entry.id, NULL)); + + 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); +} + +// Updates the sequence_number of a SavedFileEntry persisted in ExtensionPrefs. +void UpdateSavedFileEntry(ExtensionPrefs* prefs, + const std::string& extension_id, + const SavedFileEntry& file_entry) { + ExtensionPrefs::ScopedDictionaryUpdate update( + prefs, extension_id, kFileEntries); + DictionaryValue* file_entries = update.Get(); + DCHECK(file_entries); + DictionaryValue* file_entry_dict = NULL; + file_entries->GetDictionaryWithoutPathExpansion(file_entry.id, + &file_entry_dict); + DCHECK(file_entry_dict); + file_entry_dict->SetInteger(kFileEntrySequenceNumber, + file_entry.sequence_number); +} + +// Removes a SavedFileEntry from ExtensionPrefs. +void RemoveSavedFileEntry(ExtensionPrefs* prefs, + const std::string& extension_id, + const std::string& file_entry_id) { + ExtensionPrefs::ScopedDictionaryUpdate update( + prefs, extension_id, kFileEntries); + DictionaryValue* file_entries = update.Get(); + if (!file_entries) + file_entries = update.Create(); + file_entries->RemoveWithoutPathExpansion(file_entry_id, NULL); +} + +// Clears all SavedFileEntry for the app from ExtensionPrefs. +void ClearSavedFileEntries(ExtensionPrefs* prefs, + const std::string& extension_id) { + prefs->UpdateExtensionPref(extension_id, kFileEntries, NULL); +} + +// Returns all SavedFileEntries for the app. +std::vector<SavedFileEntry> GetSavedFileEntries( + ExtensionPrefs* prefs, + const std::string& extension_id) { + std::vector<SavedFileEntry> result; + const DictionaryValue* file_entries = NULL; + if (!prefs->ReadPrefAsDictionary(extension_id, kFileEntries, &file_entries)) + return result; + + for (DictionaryValue::Iterator it(*file_entries); !it.IsAtEnd(); + it.Advance()) { + const DictionaryValue* file_entry = NULL; + if (!it.value().GetAsDictionary(&file_entry)) + continue; + const base::Value* path_value; + if (!file_entry->Get(kFileEntryPath, &path_value)) + continue; + 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)); + } + return result; +} + +} // namespace + +SavedFileEntry::SavedFileEntry() : writable(false), 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 { + public: + SavedFiles(Profile* profile, const std::string& extension_id); + ~SavedFiles(); + + void RegisterFileEntry(const std::string& id, + const base::FilePath& file_path, + bool writable); + void EnqueueFileEntry(const std::string& id); + bool IsRegistered(const std::string& id) const; + const SavedFileEntry* GetFileEntry(const std::string& id) const; + std::vector<SavedFileEntry> GetAllFileEntries() const; + + private: + // Compacts sequence numbers if the largest sequence number is + // g_max_sequence_number. Outside of testing, it is set to kint32max, so this + // will almost never do any real work. + void MaybeCompactSequenceNumbers(); + + void LoadSavedFileEntriesFromPreferences(); + + Profile* profile_; + const std::string extension_id_; + + // Contains all file entries that have been registered, keyed by ID. Owns + // values. + base::hash_map<std::string, SavedFileEntry*> registered_file_entries_; + STLValueDeleter<base::hash_map<std::string, SavedFileEntry*> > + registered_file_entries_deleter_; + + // The queue of file entries that have been retained, keyed by + // sequence_number. Values are a subset of values in registered_file_entries_. + // This should be kept in sync with file entries stored in extension prefs. + std::map<int, SavedFileEntry*> saved_file_lru_; + + DISALLOW_COPY_AND_ASSIGN(SavedFiles); +}; + +// static +SavedFilesService* SavedFilesService::Get(Profile* profile) { + return SavedFilesServiceFactory::GetForProfile(profile); +} + +SavedFilesService::SavedFilesService(Profile* profile) + : extension_id_to_saved_files_deleter_(&extension_id_to_saved_files_), + profile_(profile) { + registrar_.Add(this, + chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED, + content::NotificationService::AllSources()); + registrar_.Add(this, + chrome::NOTIFICATION_APP_TERMINATING, + content::NotificationService::AllSources()); +} + +SavedFilesService::~SavedFilesService() {} + +void SavedFilesService::Observe(int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) { + switch (type) { + case chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED: { + ExtensionHost* host = content::Details<ExtensionHost>(details).ptr(); + const Extension* extension = host->extension(); + if (extension) { + ClearQueueIfNoRetainPermission(extension); + Clear(extension->id()); + } + break; + } + + case chrome::NOTIFICATION_APP_TERMINATING: { + // Stop listening to NOTIFICATION_EXTENSION_HOST_DESTROYED in particular + // as all extension hosts will be destroyed as a result of shutdown. + registrar_.RemoveAll(); + break; + } + } +} + +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); +} + +void SavedFilesService::EnqueueFileEntry(const std::string& extension_id, + const std::string& id) { + GetOrInsert(extension_id)->EnqueueFileEntry(id); +} + +std::vector<SavedFileEntry> SavedFilesService::GetAllFileEntries( + const std::string& extension_id) { + return GetOrInsert(extension_id)->GetAllFileEntries(); +} + +bool SavedFilesService::IsRegistered(const std::string& extension_id, + const std::string& id) { + return GetOrInsert(extension_id)->IsRegistered(id); +} + +const SavedFileEntry* SavedFilesService::GetFileEntry( + const std::string& extension_id, + const std::string& id) { + return GetOrInsert(extension_id)->GetFileEntry(id); +} + +void SavedFilesService::ClearQueueIfNoRetainPermission( + const Extension* extension) { + if (!extension->GetActivePermissions()->HasAPIPermission( + APIPermission::kFileSystemRetainFiles)) { + ClearSavedFileEntries(ExtensionPrefs::Get(profile_), extension->id()); + Clear(extension->id()); + } +} + +SavedFilesService::SavedFiles* SavedFilesService::GetOrInsert( + const std::string& extension_id) { + std::map<std::string, SavedFiles*>::iterator it = + extension_id_to_saved_files_.find(extension_id); + if (it != extension_id_to_saved_files_.end()) + return it->second; + + SavedFiles* saved_files = new SavedFiles(profile_, extension_id); + extension_id_to_saved_files_.insert( + std::make_pair(extension_id, saved_files)); + return saved_files; +} + +void SavedFilesService::Clear(const std::string& extension_id) { + std::map<std::string, SavedFiles*>::iterator it = + extension_id_to_saved_files_.find(extension_id); + if (it != extension_id_to_saved_files_.end()) { + delete it->second; + extension_id_to_saved_files_.erase(it); + } +} + +SavedFilesService::SavedFiles::SavedFiles(Profile* profile, + const std::string& extension_id) + : profile_(profile), + extension_id_(extension_id), + registered_file_entries_deleter_(®istered_file_entries_) { + LoadSavedFileEntriesFromPreferences(); +} + +SavedFilesService::SavedFiles::~SavedFiles() {} + +void SavedFilesService::SavedFiles::RegisterFileEntry( + const std::string& id, + const base::FilePath& file_path, + bool writable) { + if (ContainsKey(registered_file_entries_, id)) + return; + + registered_file_entries_.insert( + std::make_pair(id, new SavedFileEntry(id, file_path, writable, 0))); +} + +void SavedFilesService::SavedFiles::EnqueueFileEntry(const std::string& id) { + base::hash_map<std::string, SavedFileEntry*>::iterator it = + registered_file_entries_.find(id); + DCHECK(it != registered_file_entries_.end()); + + SavedFileEntry* file_entry = it->second; + int old_sequence_number = file_entry->sequence_number; + if (!saved_file_lru_.empty()) { + // Get the sequence number after the last file entry in the LRU. + std::map<int, SavedFileEntry*>::reverse_iterator it = + saved_file_lru_.rbegin(); + if (it->second == file_entry) + return; + + file_entry->sequence_number = it->first + 1; + } else { + // The first sequence number is 1, as 0 means the entry is not in the LRU. + file_entry->sequence_number = 1; + } + saved_file_lru_.insert( + std::make_pair(file_entry->sequence_number, file_entry)); + ExtensionPrefs* prefs = ExtensionPrefs::Get(profile_); + if (old_sequence_number) { + saved_file_lru_.erase(old_sequence_number); + UpdateSavedFileEntry(prefs, extension_id_, *file_entry); + } else { + AddSavedFileEntry(prefs, extension_id_, *file_entry); + if (saved_file_lru_.size() > g_max_saved_file_entries) { + std::map<int, SavedFileEntry*>::iterator it = saved_file_lru_.begin(); + it->second->sequence_number = 0; + RemoveSavedFileEntry(prefs, extension_id_, it->second->id); + saved_file_lru_.erase(it); + } + } + MaybeCompactSequenceNumbers(); +} + +bool SavedFilesService::SavedFiles::IsRegistered(const std::string& id) const { + return ContainsKey(registered_file_entries_, id); +} + +const SavedFileEntry* SavedFilesService::SavedFiles::GetFileEntry( + const std::string& id) const { + base::hash_map<std::string, SavedFileEntry*>::const_iterator it = + registered_file_entries_.find(id); + if (it == registered_file_entries_.end()) + return NULL; + + return it->second; +} + +std::vector<SavedFileEntry> SavedFilesService::SavedFiles::GetAllFileEntries() + const { + std::vector<SavedFileEntry> result; + for (base::hash_map<std::string, SavedFileEntry*>::const_iterator it = + registered_file_entries_.begin(); + it != registered_file_entries_.end(); + ++it) { + result.push_back(*it->second); + } + return result; +} + +void SavedFilesService::SavedFiles::MaybeCompactSequenceNumbers() { + DCHECK_GE(g_max_sequence_number, 0); + DCHECK_GE(static_cast<size_t>(g_max_sequence_number), + g_max_saved_file_entries); + std::map<int, SavedFileEntry*>::reverse_iterator it = + saved_file_lru_.rbegin(); + if (it == saved_file_lru_.rend()) + return; + + // Only compact sequence numbers if the last entry's sequence number is the + // maximum value. This should almost never be the case. + if (it->first < g_max_sequence_number) + return; + + int sequence_number = 0; + ExtensionPrefs* prefs = ExtensionPrefs::Get(profile_); + for (std::map<int, SavedFileEntry*>::iterator it = saved_file_lru_.begin(); + it != saved_file_lru_.end(); + ++it) { + sequence_number++; + if (it->second->sequence_number == sequence_number) + continue; + + SavedFileEntry* file_entry = it->second; + file_entry->sequence_number = sequence_number; + UpdateSavedFileEntry(prefs, extension_id_, *file_entry); + saved_file_lru_.erase(it++); + // Provide the following element as an insert hint. While optimized + // insertion time with the following element as a hint is only supported by + // the spec in C++11, the implementations do support this. + it = saved_file_lru_.insert( + it, std::make_pair(file_entry->sequence_number, file_entry)); + } +} + +void SavedFilesService::SavedFiles::LoadSavedFileEntriesFromPreferences() { + ExtensionPrefs* prefs = ExtensionPrefs::Get(profile_); + std::vector<SavedFileEntry> saved_entries = + GetSavedFileEntries(prefs, extension_id_); + for (std::vector<SavedFileEntry>::iterator it = saved_entries.begin(); + it != saved_entries.end(); + ++it) { + SavedFileEntry* file_entry = new SavedFileEntry(*it); + registered_file_entries_.insert(std::make_pair(file_entry->id, file_entry)); + saved_file_lru_.insert( + std::make_pair(file_entry->sequence_number, file_entry)); + } +} + +// static +void SavedFilesService::SetMaxSequenceNumberForTest(int max_value) { + g_max_sequence_number = max_value; +} + +// static +void SavedFilesService::ClearMaxSequenceNumberForTest() { + g_max_sequence_number = kMaxSequenceNumber; +} + +// static +void SavedFilesService::SetLruSizeForTest(int size) { + g_max_saved_file_entries = size; +} + +// static +void SavedFilesService::ClearLruSizeForTest() { + g_max_saved_file_entries = kMaxSavedFileEntries; +} + +} // namespace apps diff --git a/apps/saved_files_service.h b/apps/saved_files_service.h new file mode 100644 index 0000000..613764a --- /dev/null +++ b/apps/saved_files_service.h @@ -0,0 +1,135 @@ +// Copyright 2013 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. + +#ifndef APPS_SAVED_FILES_SERVICE_H_ +#define APPS_SAVED_FILES_SERVICE_H_ + +#include <map> +#include <set> +#include <string> +#include <vector> + +#include "base/files/file_path.h" +#include "base/gtest_prod_util.h" +#include "base/stl_util.h" +#include "components/browser_context_keyed_service/browser_context_keyed_service.h" +#include "content/public/browser/notification_observer.h" +#include "content/public/browser/notification_registrar.h" + +class Profile; +class SavedFilesServiceUnitTest; +FORWARD_DECLARE_TEST(SavedFilesServiceUnitTest, RetainTwoFilesTest); +FORWARD_DECLARE_TEST(SavedFilesServiceUnitTest, EvictionTest); +FORWARD_DECLARE_TEST(SavedFilesServiceUnitTest, SequenceNumberCompactionTest); + +namespace extensions { +class Extension; +} + +namespace apps { + +// Represents a file entry that a user has given an app permission to +// access. Will be persisted to disk (in the Preferences file), so should remain +// serializable. +struct SavedFileEntry { + SavedFileEntry(); + + SavedFileEntry(const std::string& id, + const base::FilePath& path, + bool writable, + int sequence_number); + + // The opaque id of this file entry. + std::string id; + + // 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; +}; + +// Tracks the files that apps have retained access to both while running and +// when suspended. +class SavedFilesService : public BrowserContextKeyedService, + public content::NotificationObserver { + public: + explicit SavedFilesService(Profile* profile); + virtual ~SavedFilesService(); + + static SavedFilesService* Get(Profile* profile); + + // Registers a file entry with the saved files service, making it eligible to + // be put into the queue. File entries that are in the retained files queue at + // object construction are automatically registered. + void RegisterFileEntry(const std::string& extension_id, + const std::string& id, + const base::FilePath& file_path, + bool writable); + + // 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 + // recently used entry at the front of the queue if it is full. If it is + // already present, moves it to the back of the queue. The |id| must have been + // registered. + void EnqueueFileEntry(const std::string& extension_id, const std::string& id); + + // Returns whether the file entry with the given |id| has been registered. + bool IsRegistered(const std::string& extension_id, const std::string& id); + + // Gets a borrowed pointer to the file entry with the specified |id|. Returns + // NULL if the file entry has not been registered. + const SavedFileEntry* GetFileEntry(const std::string& extension_id, + const std::string& id); + + // Returns all registered file entries. + std::vector<SavedFileEntry> GetAllFileEntries( + const std::string& extension_id); + + // Clears all retained files if the app does not have the + // fileSystem.retainFiles permission. + void ClearQueueIfNoRetainPermission(const extensions::Extension* extension); + + private: + FRIEND_TEST_ALL_PREFIXES(::SavedFilesServiceUnitTest, RetainTwoFilesTest); + FRIEND_TEST_ALL_PREFIXES(::SavedFilesServiceUnitTest, EvictionTest); + FRIEND_TEST_ALL_PREFIXES(::SavedFilesServiceUnitTest, + SequenceNumberCompactionTest); + friend class ::SavedFilesServiceUnitTest; + + // A container for the registered files for an app. + class SavedFiles; + + // content::NotificationObserver. + virtual void Observe(int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) OVERRIDE; + + // Returns the SavedFiles for |extension_id|, creating it if necessary. + SavedFiles* GetOrInsert(const std::string& extension_id); + + // Clears the SavedFiles for |extension_id|. + void Clear(const std::string& extension_id); + + static void SetMaxSequenceNumberForTest(int max_value); + static void ClearMaxSequenceNumberForTest(); + static void SetLruSizeForTest(int size); + static void ClearLruSizeForTest(); + + std::map<std::string, SavedFiles*> extension_id_to_saved_files_; + STLValueDeleter<std::map<std::string, SavedFiles*> > + extension_id_to_saved_files_deleter_; + content::NotificationRegistrar registrar_; + Profile* profile_; + + DISALLOW_COPY_AND_ASSIGN(SavedFilesService); +}; + +} // namespace apps + +#endif // APPS_SAVED_FILES_SERVICE_H_ diff --git a/apps/saved_files_service_factory.cc b/apps/saved_files_service_factory.cc new file mode 100644 index 0000000..c904770 --- /dev/null +++ b/apps/saved_files_service_factory.cc @@ -0,0 +1,36 @@ +// Copyright 2013 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 "apps/saved_files_service_factory.h" + +#include "apps/saved_files_service.h" +#include "chrome/browser/profiles/profile.h" +#include "components/browser_context_keyed_service/browser_context_dependency_manager.h" + +namespace apps { + +// static +SavedFilesService* SavedFilesServiceFactory::GetForProfile(Profile* profile) { + return static_cast<SavedFilesService*>( + GetInstance()->GetServiceForBrowserContext(profile, true)); +} + +// static +SavedFilesServiceFactory* SavedFilesServiceFactory::GetInstance() { + return Singleton<SavedFilesServiceFactory>::get(); +} + +SavedFilesServiceFactory::SavedFilesServiceFactory() + : BrowserContextKeyedServiceFactory( + "SavedFilesService", + BrowserContextDependencyManager::GetInstance()) {} + +SavedFilesServiceFactory::~SavedFilesServiceFactory() {} + +BrowserContextKeyedService* SavedFilesServiceFactory::BuildServiceInstanceFor( + content::BrowserContext* profile) const { + return new SavedFilesService(static_cast<Profile*>(profile)); +} + +} // namespace apps diff --git a/apps/saved_files_service_factory.h b/apps/saved_files_service_factory.h new file mode 100644 index 0000000..502010f --- /dev/null +++ b/apps/saved_files_service_factory.h @@ -0,0 +1,35 @@ +// Copyright 2013 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. + +#ifndef APPS_SAVED_FILES_SERVICE_FACTORY_H_ +#define APPS_SAVED_FILES_SERVICE_FACTORY_H_ + +#include "base/memory/singleton.h" +#include "components/browser_context_keyed_service/browser_context_keyed_service_factory.h" + +class Profile; + +namespace apps { + +class SavedFilesService; + +// BrowserContextKeyedServiceFactory for SavedFilesService. +class SavedFilesServiceFactory : public BrowserContextKeyedServiceFactory { + public: + static SavedFilesService* GetForProfile(Profile* profile); + + static SavedFilesServiceFactory* GetInstance(); + + private: + SavedFilesServiceFactory(); + virtual ~SavedFilesServiceFactory(); + friend struct DefaultSingletonTraits<SavedFilesServiceFactory>; + + virtual BrowserContextKeyedService* BuildServiceInstanceFor( + content::BrowserContext* profile) const OVERRIDE; +}; + +} // namespace apps + +#endif // APPS_SAVED_FILES_SERVICE_FACTORY_H_ diff --git a/apps/saved_files_service_unittest.cc b/apps/saved_files_service_unittest.cc new file mode 100644 index 0000000..4d31485 --- /dev/null +++ b/apps/saved_files_service_unittest.cc @@ -0,0 +1,239 @@ +// Copyright 2013 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 <algorithm> + +#include "apps/saved_files_service.h" +#include "base/files/file_path.h" +#include "base/string_number_conversions.h" +#include "base/test/values_test_util.h" +#include "base/values.h" +#include "chrome/browser/extensions/extension_prefs.h" +#include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/extension_system.h" +#include "chrome/browser/extensions/test_extension_environment.h" +#include "chrome/common/extensions/extension.h" +#include "chrome/test/base/testing_profile.h" +#include "testing/gtest/include/gtest/gtest.h" + +#if !defined(OS_ANDROID) + +#define TRACE_CALL(expression) \ + do { \ + SCOPED_TRACE(#expression); \ + expression; \ + } while (0) + +using apps::SavedFileEntry; +using apps::SavedFilesService; + +namespace { + +std::string GenerateId(int i) { + return base::IntToString(i) + ":filename.ext"; +} + +} // namespace + +class SavedFilesServiceUnitTest : public testing::Test { + protected: + virtual void SetUp() OVERRIDE { + testing::Test::SetUp(); + extension_ = env_.MakeExtension(*base::test::ParseJson( + "{" + " \"app\": {" + " \"background\": {" + " \"scripts\": [\"background.js\"]" + " }" + " }," + " \"permissions\": [" + " {\"fileSystem\": [\"retainFiles\"]}" + " ]" + "}")); + service_ = SavedFilesService::Get(env_.profile()); + path_ = base::FilePath(FILE_PATH_LITERAL("filename.ext")); + } + + virtual void TearDown() OVERRIDE { + SavedFilesService::ClearMaxSequenceNumberForTest(); + SavedFilesService::ClearLruSizeForTest(); + testing::Test::TearDown(); + } + + // Check that a registered file entry has the correct value. + void CheckEntrySequenceNumber(int id, int sequence_number) { + std::string id_string = GenerateId(id); + SCOPED_TRACE(id_string); + EXPECT_TRUE(service_->IsRegistered(extension_->id(), id_string)); + const SavedFileEntry* entry = + service_->GetFileEntry(extension_->id(), id_string); + 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); + } + + // Check that a range of registered file entries have the correct values. + void CheckRangeEnqueuedInOrder(int start, int end) { + SavedFileEntry entry; + for (int i = start; i < end; i++) { + CheckEntrySequenceNumber(i, i + 1); + } + } + + extensions::TestExtensionEnvironment env_; + const extensions::Extension* extension_; + SavedFilesService* service_; + base::FilePath path_; +}; + +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); + + // Test that no entry has a sequence number. + TRACE_CALL(CheckEntrySequenceNumber(1, 0)); + TRACE_CALL(CheckEntrySequenceNumber(2, 0)); + TRACE_CALL(CheckEntrySequenceNumber(3, 0)); + + // Test that only entry #1 has a sequence number. + service_->EnqueueFileEntry(extension_->id(), GenerateId(1)); + TRACE_CALL(CheckEntrySequenceNumber(1, 1)); + TRACE_CALL(CheckEntrySequenceNumber(2, 0)); + + // Test that entry #1 has not changed sequence number because it is the most + // recently enqueued entry. + service_->EnqueueFileEntry(extension_->id(), GenerateId(1)); + TRACE_CALL(CheckEntrySequenceNumber(1, 1)); + TRACE_CALL(CheckEntrySequenceNumber(2, 0)); + + // Test that entry #1 is unchanged and entry #2 has been assigned the next + // sequence number. + service_->EnqueueFileEntry(extension_->id(), GenerateId(2)); + TRACE_CALL(CheckEntrySequenceNumber(1, 1)); + TRACE_CALL(CheckEntrySequenceNumber(2, 2)); + + // Test that both entries #1 and #2 are unchanged because #2 is the most + // recently enqueued entry. + service_->EnqueueFileEntry(extension_->id(), GenerateId(2)); + TRACE_CALL(CheckEntrySequenceNumber(1, 1)); + TRACE_CALL(CheckEntrySequenceNumber(2, 2)); + + // Test that entry #1 has been assigned the next sequence number. + service_->EnqueueFileEntry(extension_->id(), GenerateId(1)); + TRACE_CALL(CheckEntrySequenceNumber(1, 3)); + TRACE_CALL(CheckEntrySequenceNumber(2, 2)); + TRACE_CALL(CheckEntrySequenceNumber(3, 0)); + + EXPECT_FALSE(service_->IsRegistered(extension_->id(), "another id")); + SavedFileEntry entry; + EXPECT_FALSE(service_->GetFileEntry(extension_->id(), "another id")); + + // ClearQueueIfNoRetainPermission should be a no-op because the app has the + // fileSystem.retainFiles permission. + service_->ClearQueueIfNoRetainPermission(extension_); + TRACE_CALL(CheckEntrySequenceNumber(1, 3)); + TRACE_CALL(CheckEntrySequenceNumber(2, 2)); + TRACE_CALL(CheckEntrySequenceNumber(3, 0)); + + // Test that after a clear, retained file entries are unchanged, but file + // entries that have been registered but not retained are no longer + // registered. + service_->Clear(extension_->id()); + TRACE_CALL(CheckEntrySequenceNumber(1, 3)); + TRACE_CALL(CheckEntrySequenceNumber(2, 2)); + EXPECT_FALSE(service_->IsRegistered(extension_->id(), GenerateId(3))); +} + +TEST_F(SavedFilesServiceUnitTest, NoRetainFilesPermissionTest) { + extension_ = env_.MakeExtension(*base::test::ParseJson( + "{\"app\": {\"background\": {\"scripts\": [\"background.js\"]}}," + "\"permissions\": [\"fileSystem\"]}")); + service_->RegisterFileEntry(extension_->id(), GenerateId(1), path_, true); + TRACE_CALL(CheckEntrySequenceNumber(1, 0)); + SavedFileEntry entry; + service_->EnqueueFileEntry(extension_->id(), GenerateId(1)); + TRACE_CALL(CheckEntrySequenceNumber(1, 1)); + EXPECT_FALSE(service_->IsRegistered(extension_->id(), "another id")); + EXPECT_FALSE(service_->GetFileEntry(extension_->id(), "another id")); + + // ClearQueueIfNoRetainPermission should clear the queue, since the app does + // not have the "retainFiles" permission. + service_->ClearQueueIfNoRetainPermission(extension_); + std::vector<SavedFileEntry> entries = + service_->GetAllFileEntries(extension_->id()); + EXPECT_TRUE(entries.empty()); +} + +TEST_F(SavedFilesServiceUnitTest, EvictionTest) { + SavedFilesService::SetLruSizeForTest(10); + for (int i = 0; i < 10; i++) { + service_->RegisterFileEntry(extension_->id(), GenerateId(i), path_, true); + service_->EnqueueFileEntry(extension_->id(), GenerateId(i)); + } + service_->RegisterFileEntry(extension_->id(), GenerateId(10), path_, true); + + // Expect that entries 0 to 9 are in the queue, but 10 is not. + TRACE_CALL(CheckRangeEnqueuedInOrder(0, 10)); + TRACE_CALL(CheckEntrySequenceNumber(10, 0)); + service_->EnqueueFileEntry(extension_->id(), GenerateId(10)); + + // Expect that entries 1 to 10 are in the queue, but entry 0 is not. + TRACE_CALL(CheckEntrySequenceNumber(0, 0)); + TRACE_CALL(CheckRangeEnqueuedInOrder(1, 11)); + + // Check that retained entries are unchanged after a clear. + service_->Clear(extension_->id()); + SavedFileEntry entry; + EXPECT_FALSE(service_->GetFileEntry(extension_->id(), GenerateId(0))); + TRACE_CALL(CheckRangeEnqueuedInOrder(1, 11)); + + // Expect that entry 2 is now at the back of the queue, and no further entries + // have been evicted. + service_->EnqueueFileEntry(extension_->id(), GenerateId(2)); + TRACE_CALL(CheckEntrySequenceNumber(2, 12)); + TRACE_CALL(CheckRangeEnqueuedInOrder(1, 1)); + TRACE_CALL(CheckRangeEnqueuedInOrder(3, 11)); + + // Check that retained entries are unchanged after a clear. + service_->Clear(extension_->id()); + TRACE_CALL(CheckEntrySequenceNumber(2, 12)); + TRACE_CALL(CheckRangeEnqueuedInOrder(1, 1)); + TRACE_CALL(CheckRangeEnqueuedInOrder(3, 11)); +} + +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_->EnqueueFileEntry(extension_->id(), GenerateId(i)); + } + service_->EnqueueFileEntry(extension_->id(), GenerateId(2)); + service_->EnqueueFileEntry(extension_->id(), GenerateId(3)); + service_->EnqueueFileEntry(extension_->id(), GenerateId(2)); + + // The sequence numbers should be sparse, as they have not gone over the + // limit. + TRACE_CALL(CheckEntrySequenceNumber(0, 1)); + TRACE_CALL(CheckEntrySequenceNumber(1, 2)); + TRACE_CALL(CheckEntrySequenceNumber(2, 7)); + TRACE_CALL(CheckEntrySequenceNumber(3, 6)); + service_->Clear(extension_->id()); + TRACE_CALL(CheckEntrySequenceNumber(0, 1)); + TRACE_CALL(CheckEntrySequenceNumber(1, 2)); + TRACE_CALL(CheckEntrySequenceNumber(2, 7)); + TRACE_CALL(CheckEntrySequenceNumber(3, 6)); + + // This should push the sequence number to the limit of 8, and trigger a + // sequence number compaction. Expect that the sequence numbers are + // contiguous from 1 to 4. + service_->EnqueueFileEntry(extension_->id(), GenerateId(3)); + TRACE_CALL(CheckRangeEnqueuedInOrder(0, 4)); + service_->Clear(extension_->id()); + TRACE_CALL(CheckRangeEnqueuedInOrder(0, 4)); +} +#endif diff --git a/chrome/browser/extensions/api/app_runtime/app_runtime_api.cc b/chrome/browser/extensions/api/app_runtime/app_runtime_api.cc index 146d609..1f411ff 100644 --- a/chrome/browser/extensions/api/app_runtime/app_runtime_api.cc +++ b/chrome/browser/extensions/api/app_runtime/app_runtime_api.cc @@ -52,28 +52,10 @@ void AppEventRouter::DispatchOnLaunchedEvent( DispatchOnLaunchedEventImpl(extension->id(), arguments.Pass(), profile); } -DictionaryValue* DictionaryFromSavedFileEntry( - const app_file_handler_util::GrantedFileEntry& file_entry) { - DictionaryValue* result = new DictionaryValue(); - result->SetString("id", file_entry.id); - result->SetString("fileSystemId", file_entry.filesystem_id); - result->SetString("baseName", file_entry.registered_name); - return result; -} - // static. -void AppEventRouter::DispatchOnRestartedEvent( - Profile* profile, - const Extension* extension, - const std::vector<app_file_handler_util::GrantedFileEntry>& file_entries) { - ListValue* file_entries_list = new ListValue(); - for (std::vector<extensions::app_file_handler_util::GrantedFileEntry> - ::const_iterator it = file_entries.begin(); it != file_entries.end(); - ++it) { - file_entries_list->Append(DictionaryFromSavedFileEntry(*it)); - } +void AppEventRouter::DispatchOnRestartedEvent(Profile* profile, + const Extension* extension) { scoped_ptr<ListValue> arguments(new ListValue()); - arguments->Append(file_entries_list); scoped_ptr<Event> event(new Event(kOnRestarted, arguments.Pass())); event->restrict_to_profile = profile; extensions::ExtensionSystem::Get(profile)->event_router()-> diff --git a/chrome/browser/extensions/api/app_runtime/app_runtime_api.h b/chrome/browser/extensions/api/app_runtime/app_runtime_api.h index c925f38..493c342 100644 --- a/chrome/browser/extensions/api/app_runtime/app_runtime_api.h +++ b/chrome/browser/extensions/api/app_runtime/app_runtime_api.h @@ -28,10 +28,8 @@ class AppEventRouter { // data. static void DispatchOnLaunchedEvent(Profile* profile, const Extension* extension); - static void DispatchOnRestartedEvent( - Profile* profile, - const Extension* extension, - const std::vector<app_file_handler_util::GrantedFileEntry>& file_entries); + static void DispatchOnRestartedEvent(Profile* profile, + const Extension* extension); // TODO(benwells): Update this comment, it is out of date. // Dispatches the onLaunched event to the given app, providing launch data of diff --git a/chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc b/chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc index 3341069..9faa264 100644 --- a/chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc +++ b/chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc @@ -15,16 +15,6 @@ namespace extensions { namespace app_file_handler_util { namespace { -// Preference keys - -// The file entries that an extension has permission to access. -const char kFileEntries[] = "file_entries"; - -// The path to a file entry that an extension had permission to access. -const char kFileEntryPath[] = "path"; - -// Whether or not an extension had write access to a file entry. -const char kFileEntryWritable[] = "writable"; bool FileHandlerCanHandleFileWithExtension( const FileHandlerInfo& handler, @@ -159,72 +149,9 @@ GrantedFileEntry CreateFileEntry( // above if required. if (!policy->CanReadFile(renderer_id, path)) policy->GrantReadFile(renderer_id, path); - - // Save this file entry in the prefs. - AddSavedFileEntry(ExtensionPrefs::Get(profile), - extension_id, - result.id, - path, - writable); return result; } -void AddSavedFileEntry(ExtensionPrefs* prefs, - const std::string& extension_id, - const std::string& file_entry_id, - const base::FilePath& file_path, - bool writable) { - ExtensionPrefs::ScopedDictionaryUpdate update( - prefs, - extension_id, - kFileEntries); - DictionaryValue* file_entries = update.Get(); - if (!file_entries) - file_entries = update.Create(); - - // Once a file's permissions are set, they can't be changed. - DictionaryValue* file_entry_dict = NULL; - if (file_entries->GetDictionary(file_entry_id, &file_entry_dict)) - return; - - file_entry_dict = new DictionaryValue(); - file_entry_dict->SetString(kFileEntryPath, file_path.value()); - file_entry_dict->SetBoolean(kFileEntryWritable, writable); - file_entries->SetWithoutPathExpansion(file_entry_id, file_entry_dict); -} - -void GetSavedFileEntries( - const ExtensionPrefs* prefs, - const std::string& extension_id, - std::vector<SavedFileEntry>* out) { - const DictionaryValue* file_entries = NULL; - if (!prefs || !prefs->ReadPrefAsDictionary(extension_id, - kFileEntries, - &file_entries)) { - return; - } - - for (DictionaryValue::Iterator iter(*file_entries); - !iter.IsAtEnd(); iter.Advance()) { - const DictionaryValue* file_entry = NULL; - if (!iter.value().GetAsDictionary(&file_entry)) - continue; - base::FilePath::StringType path_string; - if (!file_entry->GetString(kFileEntryPath, &path_string)) - continue; - bool writable = false; - if (!file_entry->GetBoolean(kFileEntryWritable, &writable)) - continue; - base::FilePath file_path(path_string); - out->push_back(SavedFileEntry(iter.key(), file_path, writable)); - } -} - -void ClearSavedFileEntries(ExtensionPrefs* prefs, - const std::string& extension_id) { - prefs->UpdateExtensionPref(extension_id, kFileEntries, NULL); -} - } // namespace app_file_handler_util } // namespace extensions diff --git a/chrome/browser/extensions/api/file_handlers/app_file_handler_util.h b/chrome/browser/extensions/api/file_handlers/app_file_handler_util.h index ebfe816..6d9681f 100644 --- a/chrome/browser/extensions/api/file_handlers/app_file_handler_util.h +++ b/chrome/browser/extensions/api/file_handlers/app_file_handler_util.h @@ -47,22 +47,6 @@ bool FileHandlerCanHandleFile( const std::string& mime_type, const base::FilePath& path); -// Represents a file entry that a user has given an extension permission to -// access. Intended to be persisted to disk (in the Preferences file), so should -// remain serializable. -struct SavedFileEntry { - SavedFileEntry(const std::string& id, - const base::FilePath& path, - bool writable) - : id(id), - path(path), - writable(writable) {} - - std::string id; - base::FilePath path; - bool writable; -}; - // Refers to a file entry that a renderer has been given access to. struct GrantedFileEntry { std::string id; @@ -79,19 +63,6 @@ GrantedFileEntry CreateFileEntry( const base::FilePath& path, bool writable); -// Methods to adjust the file entry preferences for a given extension. -void AddSavedFileEntry(ExtensionPrefs* prefs, - const std::string& extension_id, - const std::string& file_entry_id, - const base::FilePath& file_path, - bool writable); -void GetSavedFileEntries( - const ExtensionPrefs* prefs, - const std::string& extension_id, - std::vector<app_file_handler_util::SavedFileEntry>* out); -void ClearSavedFileEntries(ExtensionPrefs* prefs, - const std::string& extension_id); - } // namespace app_file_handler_util } // namespace extensions diff --git a/chrome/browser/extensions/api/file_system/file_system_api.cc b/chrome/browser/extensions/api/file_system/file_system_api.cc index 54bf08d..b4f8252 100644 --- a/chrome/browser/extensions/api/file_system/file_system_api.cc +++ b/chrome/browser/extensions/api/file_system/file_system_api.cc @@ -4,6 +4,7 @@ #include "chrome/browser/extensions/api/file_system/file_system_api.h" +#include "apps/saved_files_service.h" #include "base/bind.h" #include "base/file_util.h" #include "base/files/file_path.h" @@ -47,6 +48,8 @@ #include "chrome/browser/chromeos/drive/file_system_util.h" #endif +using apps::SavedFileEntry; +using apps::SavedFilesService; using fileapi::IsolatedContext; const char kInvalidParameters[] = "Invalid parameters"; @@ -57,6 +60,7 @@ const char kUserCancelled[] = "User cancelled"; const char kWritableFileError[] = "Invalid file for writing"; const char kRequiresFileSystemWriteError[] = "Operation requires fileSystem.write permission"; +const char kUnknownIdError[] = "Unknown id"; namespace file_system = extensions::api::file_system; namespace ChooseEntry = file_system::ChooseEntry; @@ -140,13 +144,14 @@ bool g_skip_picker_for_test = false; bool g_use_suggested_path_for_test = false; base::FilePath* g_path_to_be_picked_for_test; -bool GetFilePathOfFileEntry(const std::string& filesystem_name, - const std::string& filesystem_path, - const content::RenderViewHost* render_view_host, - base::FilePath* file_path, - std::string* error) { - std::string filesystem_id; - if (!fileapi::CrackIsolatedFileSystemName(filesystem_name, &filesystem_id)) { +bool GetFileSystemAndPathOfFileEntry( + const std::string& filesystem_name, + const std::string& filesystem_path, + const content::RenderViewHost* render_view_host, + std::string* filesystem_id, + base::FilePath* file_path, + std::string* error) { + if (!fileapi::CrackIsolatedFileSystemName(filesystem_name, filesystem_id)) { *error = kInvalidParameters; return false; } @@ -156,7 +161,7 @@ bool GetFilePathOfFileEntry(const std::string& filesystem_name, content::ChildProcessSecurityPolicy* policy = content::ChildProcessSecurityPolicy::GetInstance(); if (!policy->CanReadFileSystem(render_view_host->GetProcess()->GetID(), - filesystem_id)) { + *filesystem_id)) { *error = kSecurityError; return false; } @@ -164,10 +169,10 @@ bool GetFilePathOfFileEntry(const std::string& filesystem_name, IsolatedContext* context = IsolatedContext::GetInstance(); base::FilePath relative_path = base::FilePath::FromUTF8Unsafe(filesystem_path); - base::FilePath virtual_path = context->CreateVirtualRootPath(filesystem_id) + base::FilePath virtual_path = context->CreateVirtualRootPath(*filesystem_id) .Append(relative_path); if (!context->CrackVirtualPath(virtual_path, - &filesystem_id, + filesystem_id, NULL, file_path)) { *error = kInvalidParameters; @@ -177,6 +182,20 @@ bool GetFilePathOfFileEntry(const std::string& filesystem_name, return true; } +bool GetFilePathOfFileEntry(const std::string& filesystem_name, + const std::string& filesystem_path, + const content::RenderViewHost* render_view_host, + base::FilePath* file_path, + std::string* error) { + std::string filesystem_id; + return GetFileSystemAndPathOfFileEntry(filesystem_name, + filesystem_path, + render_view_host, + &filesystem_id, + file_path, + error); +} + bool DoCheckWritableFile(const base::FilePath& path) { // Don't allow links. if (file_util::PathExists(path) && file_util::IsLink(path)) @@ -326,6 +345,11 @@ void FileSystemEntryFunction::CheckWritableFile(const base::FilePath& path) { void FileSystemEntryFunction::RegisterFileSystemAndSendResponse( const base::FilePath& path, EntryType entry_type) { + RegisterFileSystemAndSendResponseWithIdOverride(path, entry_type, ""); +} + +void FileSystemEntryFunction::RegisterFileSystemAndSendResponseWithIdOverride( + const base::FilePath& path, EntryType entry_type, const std::string& id) { DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); fileapi::IsolatedContext* isolated_context = @@ -342,7 +366,10 @@ void FileSystemEntryFunction::RegisterFileSystemAndSendResponse( SetResult(dict); dict->SetString("fileSystemId", file_entry.filesystem_id); dict->SetString("baseName", file_entry.registered_name); - dict->SetString("id", file_entry.id); + if (id.empty()) + dict->SetString("id", file_entry.id); + else + dict->SetString("id", id); SendResponse(true); } @@ -701,4 +728,80 @@ bool FileSystemChooseEntryFunction::RunImpl() { return true; } +bool FileSystemRetainEntryFunction::RunImpl() { + std::string entry_id; + EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &entry_id)); + SavedFilesService* saved_files_service = SavedFilesService::Get(profile()); + // Add the file to the retain list if it is not already on there. + if (!saved_files_service->IsRegistered(extension_->id(), entry_id) && + !RetainFileEntry(entry_id)) { + return false; + } + saved_files_service->EnqueueFileEntry(extension_->id(), entry_id); + return true; +} + +bool FileSystemRetainEntryFunction::RetainFileEntry( + const std::string& entry_id) { + std::string filesystem_name; + std::string filesystem_path; + EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &filesystem_name)); + EXTENSION_FUNCTION_VALIDATE(args_->GetString(2, &filesystem_path)); + std::string filesystem_id; + base::FilePath path; + if (!GetFileSystemAndPathOfFileEntry(filesystem_name, + filesystem_path, + render_view_host_, + &filesystem_id, + &path, + &error_)) { + return false; + } + + content::ChildProcessSecurityPolicy* policy = + content::ChildProcessSecurityPolicy::GetInstance(); + bool is_writable = policy->CanReadWriteFileSystem( + render_view_host_->GetProcess()->GetID(), filesystem_id); + SavedFilesService::Get(profile())->RegisterFileEntry( + extension_->id(), entry_id, path, is_writable); + return true; +} + +bool FileSystemIsRestorableFunction::RunImpl() { + std::string entry_id; + EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &entry_id)); + SetResult(new base::FundamentalValue(SavedFilesService::Get( + profile())->IsRegistered(extension_->id(), entry_id))); + return true; +} + +bool FileSystemRestoreEntryFunction::RunImpl() { + std::string entry_id; + bool needs_new_entry; + EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &entry_id)); + EXTENSION_FUNCTION_VALIDATE(args_->GetBoolean(1, &needs_new_entry)); + const SavedFileEntry* file_entry = SavedFilesService::Get( + profile())->GetFileEntry(extension_->id(), entry_id); + if (!file_entry) { + error_ = kUnknownIdError; + return false; + } + + SavedFilesService::Get(profile())->EnqueueFileEntry( + extension_->id(), entry_id); + + // Only create a new file entry if the renderer requests one. + // |needs_new_entry| will be false if the renderer already has an Entry for + // |entry_id|. + if (needs_new_entry) { + // Reuse the ID of the retained file entry so retainEntry returns the same + // ID that was passed to restoreEntry. + RegisterFileSystemAndSendResponseWithIdOverride( + file_entry->path, + file_entry->writable ? WRITABLE : READ_ONLY, + file_entry->id); + } + return true; +} + } // namespace extensions diff --git a/chrome/browser/extensions/api/file_system/file_system_api.h b/chrome/browser/extensions/api/file_system/file_system_api.h index 0dad0c0..9793e57 100644 --- a/chrome/browser/extensions/api/file_system/file_system_api.h +++ b/chrome/browser/extensions/api/file_system/file_system_api.h @@ -44,6 +44,16 @@ class FileSystemEntryFunction : public AsyncExtensionFunction { void RegisterFileSystemAndSendResponse(const base::FilePath& path, EntryType entry_type); + // This will finish the choose file process. This is either called directly + // from FileSelected, or from CreateFileIfNecessary. It is called on the UI + // thread. |id_override| specifies the id to send in the response instead of + // the generated id. This can be useful for creating a file entry with an id + // matching another file entry, e.g. for restoreEntry. + void RegisterFileSystemAndSendResponseWithIdOverride( + const base::FilePath& path, + EntryType entry_type, + const std::string& id_override); + // called on the UI thread if there is a problem checking a writable file. void HandleWritableFileError(); }; @@ -115,6 +125,38 @@ class FileSystemChooseEntryFunction : public FileSystemEntryFunction { base::FilePath initial_path_; }; +class FileSystemRetainEntryFunction : public SyncExtensionFunction { + public: + DECLARE_EXTENSION_FUNCTION("fileSystem.retainEntry", FILESYSTEM_RETAINENTRY) + + protected: + virtual ~FileSystemRetainEntryFunction() {} + virtual bool RunImpl() OVERRIDE; + + private: + // Retains the file entry referenced by |entry_id| in apps::SavedFilesService. + // |entry_id| must refer to an entry in an isolated file system. + bool RetainFileEntry(const std::string& entry_id); +}; + +class FileSystemIsRestorableFunction : public SyncExtensionFunction { + public: + DECLARE_EXTENSION_FUNCTION("fileSystem.isRestorable", FILESYSTEM_ISRESTORABLE) + + protected: + virtual ~FileSystemIsRestorableFunction() {} + virtual bool RunImpl() OVERRIDE; +}; + +class FileSystemRestoreEntryFunction : public FileSystemEntryFunction { + public: + DECLARE_EXTENSION_FUNCTION("fileSystem.restoreEntry", FILESYSTEM_RESTOREENTRY) + + protected: + virtual ~FileSystemRestoreEntryFunction() {} + virtual bool RunImpl() OVERRIDE; +}; + } // namespace extensions #endif // CHROME_BROWSER_EXTENSIONS_API_FILE_SYSTEM_FILE_SYSTEM_API_H_ diff --git a/chrome/browser/extensions/api/file_system/file_system_apitest.cc b/chrome/browser/extensions/api/file_system/file_system_apitest.cc index 018b749..2e67d3e2 100644 --- a/chrome/browser/extensions/api/file_system/file_system_apitest.cc +++ b/chrome/browser/extensions/api/file_system/file_system_apitest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "apps/saved_files_service.h" #include "base/file_util.h" #include "base/path_service.h" #include "build/build_config.h" @@ -18,11 +19,11 @@ namespace { class AppInstallObserver : public content::NotificationObserver { public: - AppInstallObserver(const base::FilePath& choose_entry_directory, - extensions::ExtensionPrefs* prefs) - : path_(choose_entry_directory), - prefs_(prefs) { - registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED, + AppInstallObserver( + base::Callback<void(const extensions::Extension*)> callback) + : callback_(callback) { + registrar_.Add(this, + chrome::NOTIFICATION_EXTENSION_LOADED, content::NotificationService::AllSources()); } @@ -30,18 +31,28 @@ class AppInstallObserver : public content::NotificationObserver { const content::NotificationSource& source, const content::NotificationDetails& details) OVERRIDE { EXPECT_EQ(chrome::NOTIFICATION_EXTENSION_LOADED, type); - std::string extension_id = content::Details<const extensions::Extension>( - details).ptr()->id(); - prefs_->SetLastChooseEntryDirectory(extension_id, path_); + callback_.Run(content::Details<const extensions::Extension>(details).ptr()); } private: content::NotificationRegistrar registrar_; - const base::FilePath path_; - extensions::ExtensionPrefs* prefs_; + base::Callback<void(const extensions::Extension*)> callback_; DISALLOW_COPY_AND_ASSIGN(AppInstallObserver); }; +void SetLastChooseEntryDirectory(const base::FilePath& choose_entry_directory, + extensions::ExtensionPrefs* prefs, + const extensions::Extension* extension) { + prefs->SetLastChooseEntryDirectory(extension->id(), choose_entry_directory); +} + +void AddSavedEntry(const base::FilePath& path_to_save, + apps::SavedFilesService* service, + const extensions::Extension* extension) { + service->RegisterFileEntry( + extension->id(), "magic id", path_to_save, /* writable */ true); +} + } // namespace class FileSystemApiTest : public extensions::PlatformAppBrowserTest { @@ -161,9 +172,10 @@ IN_PROC_BROWSER_TEST_F(FileSystemApiTest, SkipPickerAndSelectSuggestedPathForTest(); { AppInstallObserver observer( - test_file.DirName(), - extensions::ExtensionSystem::Get( - profile())->extension_service()->extension_prefs()); + base::Bind(SetLastChooseEntryDirectory, + test_file.DirName(), + extensions::ExtensionSystem::Get( + profile())->extension_service()->extension_prefs())); ASSERT_TRUE(RunPlatformAppTest("api_test/file_system/open_existing")) << message_; } @@ -179,11 +191,12 @@ IN_PROC_BROWSER_TEST_F(FileSystemApiTest, FileSystemChooseEntryFunction:: SkipPickerAndSelectSuggestedPathForTest(); { - AppInstallObserver observer( - test_file.DirName().Append(base::FilePath::FromUTF8Unsafe( - "fake_directory_does_not_exist")), + AppInstallObserver observer(base::Bind( + SetLastChooseEntryDirectory, + test_file.DirName().Append( + base::FilePath::FromUTF8Unsafe("fake_directory_does_not_exist")), extensions::ExtensionSystem::Get( - profile())->extension_service()->extension_prefs()); + profile())->extension_service()->extension_prefs())); ASSERT_TRUE(RunPlatformAppTest("api_test/file_system/open_existing")) << message_; } @@ -340,11 +353,30 @@ IN_PROC_BROWSER_TEST_F(FileSystemApiTest, FileSystemApiIsWritableTest) { "api_test/file_system/is_writable_file_entry")) << message_; } -IN_PROC_BROWSER_TEST_F(FileSystemApiTest, FileSystemApiGetEntryId) { +IN_PROC_BROWSER_TEST_F(FileSystemApiTest, FileSystemApiRetainEntry) { base::FilePath test_file = TempFilePath("writable.txt", true); ASSERT_FALSE(test_file.empty()); FileSystemChooseEntryFunction::SkipPickerAndAlwaysSelectPathForTest( &test_file); ASSERT_TRUE(RunPlatformAppTest( - "api_test/file_system/get_entry_id")) << message_; + "api_test/file_system/retain_entry")) << message_; + std::vector<apps::SavedFileEntry> file_entries = apps::SavedFilesService::Get( + profile())->GetAllFileEntries(GetSingleLoadedExtension()->id()); + ASSERT_EQ(1u, file_entries.size()); + EXPECT_EQ(test_file, file_entries[0].path); + EXPECT_EQ(1, file_entries[0].sequence_number); + EXPECT_FALSE(file_entries[0].writable); +} + +IN_PROC_BROWSER_TEST_F(FileSystemApiTest, FileSystemApiRestoreEntry) { + base::FilePath test_file = TempFilePath("writable.txt", true); + ASSERT_FALSE(test_file.empty()); + FileSystemChooseEntryFunction::SkipPickerAndAlwaysSelectPathForTest( + &test_file); + { + AppInstallObserver observer(base::Bind( + AddSavedEntry, test_file, apps::SavedFilesService::Get(profile()))); + ASSERT_TRUE(RunPlatformAppTest( + "api_test/file_system/restore_entry")) << message_; + } } diff --git a/chrome/browser/extensions/extension_function_histogram_value.h b/chrome/browser/extensions/extension_function_histogram_value.h index fa916d6..804c34c 100644 --- a/chrome/browser/extensions/extension_function_histogram_value.h +++ b/chrome/browser/extensions/extension_function_histogram_value.h @@ -532,6 +532,9 @@ enum HistogramValue { IDENTITYPRIVATE_GETRESOURCES, NOTIFICATIONS_GET_ALL, USB_LISTINTERFACES, + FILESYSTEM_RETAINENTRY, + FILESYSTEM_ISRESTORABLE, + FILESYSTEM_RESTOREENTRY, ENUM_BOUNDARY // Last entry: Add new entries above. }; diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index 6cfddc3..820322c 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -32,7 +32,6 @@ #include "chrome/browser/extensions/api/app_runtime/app_runtime_api.h" #include "chrome/browser/extensions/api/declarative/rules_registry_service.h" #include "chrome/browser/extensions/api/extension_action/extension_action_api.h" -#include "chrome/browser/extensions/api/file_handlers/app_file_handler_util.h" #include "chrome/browser/extensions/api/profile_keyed_api_factory.h" #include "chrome/browser/extensions/api/runtime/runtime_api.h" #include "chrome/browser/extensions/api/storage/settings_frontend.h" @@ -734,13 +733,6 @@ void ExtensionService::ReloadExtensionWithEvents( } on_load_events_[extension_id] = events; - if (events & EVENT_RESTARTED) { - extensions::app_file_handler_util::GetSavedFileEntries( - extension_prefs_, - extension_id, - &on_restart_file_entries_[extension_id]); - } - if (delayed_updates_for_idle_.Contains(extension_id)) { FinishDelayedInstallation(extension_id); @@ -2907,14 +2899,8 @@ void ExtensionService::DoPostLoadTasks(const Extension* extension) { queue->AddPendingTask(profile(), extension->id(), base::Bind(&ExtensionService::LaunchApplication)); if (events_to_fire & EVENT_RESTARTED) { - SavedFileEntryMap::iterator it = - on_restart_file_entries_.find(extension->id()); - if (it == on_restart_file_entries_.end()) - NOTREACHED(); queue->AddPendingTask(profile(), extension->id(), - base::Bind(&ExtensionService::RestartApplication, - it->second)); - on_restart_file_entries_.erase(it); + base::Bind(&ExtensionService::RestartApplication)); } } @@ -2936,14 +2922,13 @@ void ExtensionService::LaunchApplication( // static void ExtensionService::RestartApplication( - std::vector<extensions::app_file_handler_util::SavedFileEntry> file_entries, extensions::ExtensionHost* extension_host) { if (!extension_host) return; #if !defined(OS_ANDROID) - extensions::RestartPlatformAppWithFileEntries( - extension_host->profile(), extension_host->extension(), file_entries); + extensions::RestartPlatformApp( + extension_host->profile(), extension_host->extension()); #endif } diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h index e11e6cc..ea62a5b 100644 --- a/chrome/browser/extensions/extension_service.h +++ b/chrome/browser/extensions/extension_service.h @@ -67,11 +67,6 @@ class ExtensionSystem; class ExtensionUpdater; class PendingExtensionManager; class SettingsFrontend; - -namespace app_file_handler_util { -struct SavedFileEntry; -} - } namespace syncer { @@ -801,10 +796,7 @@ class ExtensionService // Dispatches a restart event to the platform app associated with // |extension_host|. - static void RestartApplication( - std::vector<extensions::app_file_handler_util::SavedFileEntry> - file_entries, - extensions::ExtensionHost* extension_host); + static void RestartApplication(extensions::ExtensionHost* extension_host); // Helper to inspect an ExtensionHost after it has been loaded. void InspectExtensionHost(extensions::ExtensionHost* host); @@ -928,13 +920,6 @@ class ExtensionService // dispatched to the extension when it is loaded. std::map<std::string, int> on_load_events_; - // Maps extension ids to vectors of saved file entries that the extension - // should be given access to on restart. - typedef std::map<std::string, - std::vector<extensions::app_file_handler_util::SavedFileEntry> > - SavedFileEntryMap; - SavedFileEntryMap on_restart_file_entries_; - content::NotificationRegistrar registrar_; PrefChangeRegistrar pref_change_registrar_; diff --git a/chrome/browser/extensions/platform_app_launcher.cc b/chrome/browser/extensions/platform_app_launcher.cc index 2471693..54289e4 100644 --- a/chrome/browser/extensions/platform_app_launcher.cc +++ b/chrome/browser/extensions/platform_app_launcher.cc @@ -50,7 +50,6 @@ 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::SavedFileEntry; namespace extensions { @@ -279,79 +278,6 @@ class PlatformAppPathLauncher DISALLOW_COPY_AND_ASSIGN(PlatformAppPathLauncher); }; -class SavedFileEntryLauncher - : public base::RefCountedThreadSafe<SavedFileEntryLauncher> { - public: - SavedFileEntryLauncher( - Profile* profile, - const Extension* extension, - const std::vector<SavedFileEntry>& file_entries) - : profile_(profile), - extension_(extension), - file_entries_(file_entries) {} - - void Launch() { - // Access needs to be granted to the file or filesystem for the process - // associated with the extension. To do this the ExtensionHost is needed. - // This might not be available, or it might be in the process of being - // unloaded, in which case the lazy background task queue is used to load - // he extension and then call back to us. - extensions::LazyBackgroundTaskQueue* queue = - ExtensionSystem::Get(profile_)->lazy_background_task_queue(); - if (queue->ShouldEnqueueTask(profile_, extension_)) { - queue->AddPendingTask(profile_, extension_->id(), base::Bind( - &SavedFileEntryLauncher::GrantAccessToFilesAndLaunch, - this)); - return; - } - ExtensionProcessManager* process_manager = - ExtensionSystem::Get(profile_)->process_manager(); - extensions::ExtensionHost* host = - process_manager->GetBackgroundHostForExtension(extension_->id()); - DCHECK(host); - GrantAccessToFilesAndLaunch(host); - } - - private: - friend class base::RefCountedThreadSafe<SavedFileEntryLauncher>; - ~SavedFileEntryLauncher() {} - - void GrantAccessToFilesAndLaunch(ExtensionHost* host) { - // If there was an error loading the app page, |host| will be NULL. - if (!host) { - LOG(ERROR) << "Could not load app page for " << extension_->id(); - return; - } - - int renderer_id = host->render_process_host()->GetID(); - std::vector<GrantedFileEntry> granted_file_entries; - for (std::vector<SavedFileEntry>::const_iterator it = - file_entries_.begin(); it != file_entries_.end(); ++it) { - GrantedFileEntry file_entry = CreateFileEntry( - profile_, extension_->id(), renderer_id, it->path, it->writable); - file_entry.id = it->id; - granted_file_entries.push_back(file_entry); - - // Record that we have granted this file permission. - app_file_handler_util::AddSavedFileEntry( - ExtensionPrefs::Get(profile_), - host->extension()->id(), - it->id, - it->path, - it->writable); - } - extensions::AppEventRouter::DispatchOnRestartedEvent( - profile_, extension_, granted_file_entries); - } - - // The profile the app should be run in. - Profile* profile_; - // The extension providing the app. - const Extension* extension_; - - std::vector<SavedFileEntry> file_entries_; -}; - } // namespace void LaunchPlatformApp(Profile* profile, @@ -400,19 +326,14 @@ void LaunchPlatformAppWithFileHandler(Profile* profile, launcher->LaunchWithHandler(handler_id); } -void RestartPlatformAppWithFileEntries( - Profile* profile, - const Extension* extension, - const std::vector<SavedFileEntry>& file_entries) { +void RestartPlatformApp(Profile* profile, const Extension* extension) { extensions::EventRouter* event_router = ExtensionSystem::Get(profile)->event_router(); bool listening_to_restart = event_router-> ExtensionHasEventListener(extension->id(), event_names::kOnRestarted); if (listening_to_restart) { - scoped_refptr<SavedFileEntryLauncher> launcher = new SavedFileEntryLauncher( - profile, extension, file_entries); - launcher->Launch(); + extensions::AppEventRouter::DispatchOnRestartedEvent(profile, extension); return; } diff --git a/chrome/browser/extensions/platform_app_launcher.h b/chrome/browser/extensions/platform_app_launcher.h index fc7965a..1180f1c 100644 --- a/chrome/browser/extensions/platform_app_launcher.h +++ b/chrome/browser/extensions/platform_app_launcher.h @@ -23,10 +23,6 @@ namespace extensions { class Extension; -namespace app_file_handler_util { -struct SavedFileEntry; -} - // Launches the platform app |extension|. Creates appropriate launch data for // the |command_line| fields present. |extension| and |profile| must not be // NULL. A NULL |command_line| means there is no launch data. If non-empty, @@ -49,11 +45,7 @@ void LaunchPlatformAppWithFileHandler(Profile* profile, const std::string& handler_id, const base::FilePath& file_path); -void RestartPlatformAppWithFileEntries( - Profile* profile, - const Extension* extension, - const std::vector<app_file_handler_util::SavedFileEntry>& - saved_file_entries); +void RestartPlatformApp(Profile* profile, const Extension* extension); } // namespace extensions diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 406b03c..a18128f 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -463,6 +463,7 @@ }, 'sources': [ '../apps/app_shim/app_shim_host_mac_unittest.cc', + '../apps/saved_files_service_unittest.cc', '../apps/shell_window_geometry_cache_unittest.cc', '../extensions/browser/file_reader_unittest.cc', '../extensions/common/event_filter_unittest.cc', diff --git a/chrome/common/extensions/api/_permission_features.json b/chrome/common/extensions/api/_permission_features.json index e750a7f..08fadef 100644 --- a/chrome/common/extensions/api/_permission_features.json +++ b/chrome/common/extensions/api/_permission_features.json @@ -208,6 +208,10 @@ "extension_types": ["extension"], "whitelist": [ "2FC374607C2DF285634B67C64A2E356C607091C3" ] }], + "fileSystem.retainFiles": [{ + "channel": "trunk", + "extension_types": ["platform_app"] + }], "fileSystem.write": [{ "channel": "stable", "extension_types": ["platform_app"] diff --git a/chrome/common/extensions/api/file_system.idl b/chrome/common/extensions/api/file_system.idl index 06e2be2..89db7df 100644 --- a/chrome/common/extensions/api/file_system.idl +++ b/chrome/common/extensions/api/file_system.idl @@ -55,6 +55,7 @@ namespace fileSystem { callback GetDisplayPathCallback = void (DOMString displayPath); callback FileEntryCallback = void ([instanceOf=FileEntry] object fileEntry); callback IsWritableCallback = void (boolean isWritable); + callback IsRestorableCallback = void (boolean isRestorable); interface Functions { // Get the display path of a FileEntry object. The display path is based on @@ -76,14 +77,17 @@ namespace fileSystem { static void chooseEntry(optional ChooseEntryOptions options, FileEntryCallback callback); - // Returns the file entry with the given id. - [nocompile] static FileEntry getEntryById(DOMString id); + // Returns the file entry with the given id if it can be restored. This call + // will fail otherwise. + static void restoreEntry(DOMString id, FileEntryCallback callback); - // Returns the id of the given file entry. This can be used to retrieve file - // entries with getEntryById(). When an app is restarted (ie: it is sent the - // onRestarted event) it can regain access to the file entries it had by - // remembering their ids and calling getEntryById(). - [nocompile] static DOMString getEntryId( - [instanceOf=FileEntry] object fileEntry); + // Returns whether a file entry for the given id can be restored, i.e. + // whether restoreEntry would succeed with this id now. + static void isRestorable(DOMString id, IsRestorableCallback callback); + + // Returns an id that can be passed to restoreEntry to regain access to a + // given file entry. Only the 500 most recently used entries are retained, + // where calls to retainEntry and restoreEntry count as use. + static DOMString retainEntry([instanceOf=FileEntry] object fileEntry); }; }; diff --git a/chrome/common/extensions/permissions/api_permission.h b/chrome/common/extensions/permissions/api_permission.h index c8777d8..eb9ec78 100644 --- a/chrome/common/extensions/permissions/api_permission.h +++ b/chrome/common/extensions/permissions/api_permission.h @@ -74,6 +74,7 @@ class APIPermission { kFileBrowserHandlerInternal, kFileBrowserPrivate, kFileSystem, + kFileSystemRetainFiles, kFileSystemWrite, kFontSettings, kFullscreen, diff --git a/chrome/common/extensions/permissions/chrome_api_permissions.cc b/chrome/common/extensions/permissions/chrome_api_permissions.cc index 37f4a47..e6e8564 100644 --- a/chrome/common/extensions/permissions/chrome_api_permissions.cc +++ b/chrome/common/extensions/permissions/chrome_api_permissions.cc @@ -226,6 +226,7 @@ std::vector<APIPermissionInfo*> ChromeAPIPermissions::GetAllPermissions() // a file chooser dialog and selected a file. Selecting the file is // considered consent to read it. { APIPermission::kFileSystem, "fileSystem" }, + { APIPermission::kFileSystemRetainFiles, "fileSystem.retainFiles" }, { APIPermission::kFileSystemWrite, "fileSystem.write", APIPermissionInfo::kFlagNone, IDS_EXTENSION_PROMPT_WARNING_FILE_SYSTEM_WRITE, diff --git a/chrome/common/extensions/permissions/permission_set_unittest.cc b/chrome/common/extensions/permissions/permission_set_unittest.cc index 7dae356..128b069 100644 --- a/chrome/common/extensions/permissions/permission_set_unittest.cc +++ b/chrome/common/extensions/permissions/permission_set_unittest.cc @@ -709,6 +709,7 @@ TEST(PermissionsTest, PermissionMessages) { // Platform apps. skip.insert(APIPermission::kFileSystem); + skip.insert(APIPermission::kFileSystemRetainFiles); skip.insert(APIPermission::kSocket); skip.insert(APIPermission::kUsbDevice); diff --git a/chrome/renderer/extensions/file_system_natives.cc b/chrome/renderer/extensions/file_system_natives.cc index fd356e4..984c7bf 100644 --- a/chrome/renderer/extensions/file_system_natives.cc +++ b/chrome/renderer/extensions/file_system_natives.cc @@ -28,6 +28,9 @@ FileSystemNatives::FileSystemNatives(v8::Handle<v8::Context> context) RouteFunction("GetIsolatedFileSystem", base::Bind(&FileSystemNatives::GetIsolatedFileSystem, base::Unretained(this))); + RouteFunction("CrackIsolatedFileSystemName", + base::Bind(&FileSystemNatives::CrackIsolatedFileSystemName, + base::Unretained(this))); } v8::Handle<v8::Value> FileSystemNatives::GetIsolatedFileSystem( @@ -98,4 +101,16 @@ v8::Handle<v8::Value> FileSystemNatives::GetFileEntry( is_directory); } +v8::Handle<v8::Value> FileSystemNatives::CrackIsolatedFileSystemName( + const v8::Arguments& args) { + DCHECK_EQ(args.Length(), 1); + DCHECK(args[0]->IsString()); + std::string filesystem_name = *v8::String::Utf8Value(args[0]->ToString()); + std::string filesystem_id; + if (!fileapi::CrackIsolatedFileSystemName(filesystem_name, &filesystem_id)) + return v8::Undefined(); + + return v8::String::New(filesystem_id.c_str(), filesystem_id.size()); +} + } // namespace extensions diff --git a/chrome/renderer/extensions/file_system_natives.h b/chrome/renderer/extensions/file_system_natives.h index 6768eae..46a6f67 100644 --- a/chrome/renderer/extensions/file_system_natives.h +++ b/chrome/renderer/extensions/file_system_natives.h @@ -18,6 +18,7 @@ class FileSystemNatives : public ObjectBackedNativeHandler { private: v8::Handle<v8::Value> GetFileEntry(const v8::Arguments& args); v8::Handle<v8::Value> GetIsolatedFileSystem(const v8::Arguments& args); + v8::Handle<v8::Value> CrackIsolatedFileSystemName(const v8::Arguments& args); DISALLOW_COPY_AND_ASSIGN(FileSystemNatives); }; diff --git a/chrome/renderer/resources/extensions/app_runtime_custom_bindings.js b/chrome/renderer/resources/extensions/app_runtime_custom_bindings.js index a490c02..bf1a8f23 100644 --- a/chrome/renderer/resources/extensions/app_runtime_custom_bindings.js +++ b/chrome/renderer/resources/extensions/app_runtime_custom_bindings.js @@ -17,37 +17,6 @@ var SerializeToString = appNatives.SerializeToString; var CreateBlob = appNatives.CreateBlob; var entryIdManager = require('entryIdManager'); -chromeHidden.Event.registerArgumentMassager('app.runtime.onRestarted', - function(args, dispatch) { - // These file entries don't get dispatched, we just use this hook to register - // them all with entryIdManager. - var fileEntries = args[0]; - - var pendingCallbacks = fileEntries.length; - - var dispatchIfNoPendingCallbacks = function() { - if (pendingCallbacks == 0) - dispatch([]); - }; - - for (var i = 0; i < fileEntries.length; i++) { - var fe = fileEntries[i]; - var fs = GetIsolatedFileSystem(fe.fileSystemId); - (function(fe, fs) { - fs.root.getFile(fe.baseName, {}, function(fileEntry) { - entryIdManager.registerEntry(fe.id, fileEntry); - pendingCallbacks--; - dispatchIfNoPendingCallbacks(); - }, function(err) { - console.error('Error getting fileEntry, code: ' + err.code); - pendingCallbacks--; - dispatchIfNoPendingCallbacks(); - }); - })(fe, fs); - } - dispatchIfNoPendingCallbacks(); -}); - chromeHidden.Event.registerArgumentMassager('app.runtime.onLaunched', function(args, dispatch) { var launchData = args[0]; diff --git a/chrome/renderer/resources/extensions/entry_id_manager.js b/chrome/renderer/resources/extensions/entry_id_manager.js index 7431650..8933239 100644 --- a/chrome/renderer/resources/extensions/entry_id_manager.js +++ b/chrome/renderer/resources/extensions/entry_id_manager.js @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +var fileSystemNatives = requireNative('file_system_natives'); + var nameToIds = {}; var idsToEntries = {}; @@ -9,11 +11,20 @@ function computeName(entry) { return entry.filesystem.name + ':' + entry.fullPath; } +function computeId(entry) { + var fileSystemId = fileSystemNatives.CrackIsolatedFileSystemName( + entry.filesystem.name); + if (!fileSystemId) + return null; + // Strip the leading '/' from the path. + return fileSystemId + ':' + entry.fullPath.slice(1); +} + function registerEntry(id, entry) { var name = computeName(entry); nameToIds[name] = id; idsToEntries[id] = entry; -}; +} function getEntryId(entry) { var name = null; @@ -22,7 +33,14 @@ function getEntryId(entry) { } catch(e) { return null; } - return nameToIds[name]; + var id = nameToIds[name]; + if (id != null) + return id; + + // If an entry has not been registered, compute its id and register it. + id = computeId(entry); + registerEntry(id, entry); + return id; } function getEntryById(id) { diff --git a/chrome/renderer/resources/extensions/file_system_custom_bindings.js b/chrome/renderer/resources/extensions/file_system_custom_bindings.js index 8df8200..95bcb42 100644 --- a/chrome/renderer/resources/extensions/file_system_custom_bindings.js +++ b/chrome/renderer/resources/extensions/file_system_custom_bindings.js @@ -10,6 +10,7 @@ var fileSystemNatives = requireNative('file_system_natives'); var forEach = require('utils').forEach; var GetIsolatedFileSystem = fileSystemNatives.GetIsolatedFileSystem; var lastError = require('lastError'); +var sendRequest = require('sendRequest').sendRequest; var GetModuleSystem = requireNative('v8_context').GetModuleSystem; // TODO(sammc): Don't require extension. See http://crbug.com/235689. var GetExtensionViews = requireNative('extension').GetExtensionViews; @@ -81,16 +82,46 @@ binding.registerCustomHook(function(bindingsAPI) { forEach(['getDisplayPath', 'getWritableEntry', 'isWritableEntry'], bindFileEntryFunction); - forEach(['getWritableEntry', 'chooseEntry'], function(i, functionName) { + forEach(['getWritableEntry', 'chooseEntry', 'restoreEntry'], + function(i, functionName) { bindFileEntryCallback(functionName, apiFunctions); }); - apiFunctions.setHandleRequest('getEntryId', function(fileEntry) { - return entryIdManager.getEntryId(fileEntry); + apiFunctions.setHandleRequest('retainEntry', function(fileEntry) { + var id = entryIdManager.getEntryId(fileEntry); + if (!id) + return ''; + var fileSystemName = fileEntry.filesystem.name; + var relativePath = fileEntry.fullPath.slice(1); + + sendRequest(this.name, [id, fileSystemName, relativePath], + this.definition.parameters, {}); + return id; + }); + + apiFunctions.setHandleRequest('isRestorable', + function(id, callback) { + var savedEntry = entryIdManager.getEntryById(id); + if (savedEntry) { + callback(true); + } else { + sendRequest(this.name, [id, callback], this.definition.parameters, {}); + } }); - apiFunctions.setHandleRequest('getEntryById', function(id) { - return entryIdManager.getEntryById(id); + apiFunctions.setUpdateArgumentsPostValidate('restoreEntry', + function(id, callback) { + var savedEntry = entryIdManager.getEntryById(id); + if (savedEntry) { + // We already have a file entry for this id so pass it to the callback and + // send a request to the browser to move it to the back of the LRU. + callback(savedEntry); + return [id, false, null]; + } else { + // Ask the browser process for a new file entry for this id, to be passed + // to |callback|. + return [id, true, callback]; + } }); // TODO(benwells): Remove these deprecated versions of the functions. diff --git a/chrome/test/data/extensions/api_test/file_system/get_entry_id/test.js b/chrome/test/data/extensions/api_test/file_system/get_entry_id/test.js deleted file mode 100644 index 0bc03b9..0000000 --- a/chrome/test/data/extensions/api_test/file_system/get_entry_id/test.js +++ /dev/null @@ -1,26 +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.test.runTests([ - function getEntryIdWorks() { - chrome.runtime.getBackgroundPage( - chrome.test.callbackPass(function(backgroundPage) { - backgroundPage.callback = chrome.test.callbackPass( - function(other_window, id, entry) { - other_window.close(); - chrome.test.assertEq(chrome.fileSystem.getEntryId(entry), id); - chrome.test.assertEq(chrome.fileSystem.getEntryById(id), entry); - chrome.test.assertEq( - chrome.fileSystem.getEntryId(chrome.fileSystem.getEntryById(id)), - id); - chrome.test.assertEq( - chrome.fileSystem.getEntryById(chrome.fileSystem.getEntryId(entry)), - entry); - checkEntry(entry, 'writable.txt', false /* isNew */, - false /*shouldBeWritable */); - }); - chrome.app.window.create('test_other_window.html'); - })); - } -]); diff --git a/chrome/test/data/extensions/api_test/file_system/get_entry_id/test_other_window.js b/chrome/test/data/extensions/api_test/file_system/get_entry_id/test_other_window.js deleted file mode 100644 index 875a10e..0000000 --- a/chrome/test/data/extensions/api_test/file_system/get_entry_id/test_other_window.js +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright (c) 2013 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.runtime.getBackgroundPage( - chrome.test.callbackPass(function(backgroundPage) { - chrome.fileSystem.chooseEntry({type: 'openFile'}, - chrome.test.callbackPass(function(entry) { - var id = chrome.fileSystem.getEntryId(entry); - chrome.test.assertTrue(id != null); - var e = chrome.fileSystem.getEntryById(id); - chrome.test.assertEq(e, entry); - backgroundPage.callback(window, id, entry); - })); -})); diff --git a/chrome/test/data/extensions/api_test/file_system/get_entry_id/background.js b/chrome/test/data/extensions/api_test/file_system/restore_entry/background.js index bc4dc47..bc4dc47 100644 --- a/chrome/test/data/extensions/api_test/file_system/get_entry_id/background.js +++ b/chrome/test/data/extensions/api_test/file_system/restore_entry/background.js diff --git a/chrome/test/data/extensions/api_test/file_system/restore_entry/manifest.json b/chrome/test/data/extensions/api_test/file_system/restore_entry/manifest.json new file mode 100644 index 0000000..0596fb2 --- /dev/null +++ b/chrome/test/data/extensions/api_test/file_system/restore_entry/manifest.json @@ -0,0 +1,11 @@ +{ + "name": "chrome.fileSystem.restoreEntry test", + "version": "0.1", + "description": "Test for chrome.fileSystem.restoreEntry", + "app": { + "background": { + "scripts": ["background.js"] + } + }, + "permissions": ["fileSystem"] +} diff --git a/chrome/test/data/extensions/api_test/file_system/get_entry_id/test.html b/chrome/test/data/extensions/api_test/file_system/restore_entry/test.html index d89c528..d89c528 100644 --- a/chrome/test/data/extensions/api_test/file_system/get_entry_id/test.html +++ b/chrome/test/data/extensions/api_test/file_system/restore_entry/test.html diff --git a/chrome/test/data/extensions/api_test/file_system/restore_entry/test.js b/chrome/test/data/extensions/api_test/file_system/restore_entry/test.js new file mode 100644 index 0000000..ff1e66f --- /dev/null +++ b/chrome/test/data/extensions/api_test/file_system/restore_entry/test.js @@ -0,0 +1,27 @@ +// Copyright 2013 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.test.runTests([ + function restoreEntryWorks() { + var id = 'magic id'; + chrome.fileSystem.isRestorable(id, chrome.test.callbackPass( + function(isRestorable) { + chrome.test.assertTrue(isRestorable); + })); + chrome.fileSystem.restoreEntry(id, chrome.test.callbackPass( + function(restoredEntry) { + chrome.test.assertTrue(restoredEntry != null); + chrome.test.assertEq( + chrome.fileSystem.retainEntry(restoredEntry), id); + checkEntry(restoredEntry, 'writable.txt', false /* isNew */, + true /*shouldBeWritable */); + })); + chrome.fileSystem.isRestorable('wrong id', chrome.test.callbackPass( + function(isRestorable) { + chrome.test.assertFalse(isRestorable); + })); + chrome.fileSystem.restoreEntry('wrong id', chrome.test.callbackFail( + 'Unknown id')); + } +]); diff --git a/chrome/test/data/extensions/api_test/file_system/get_entry_id/test_util.js b/chrome/test/data/extensions/api_test/file_system/restore_entry/test_util.js index c4845b2..c4845b2 100644 --- a/chrome/test/data/extensions/api_test/file_system/get_entry_id/test_util.js +++ b/chrome/test/data/extensions/api_test/file_system/restore_entry/test_util.js diff --git a/chrome/test/data/extensions/api_test/file_system/retain_entry/background.js b/chrome/test/data/extensions/api_test/file_system/retain_entry/background.js new file mode 100644 index 0000000..bc4dc47 --- /dev/null +++ b/chrome/test/data/extensions/api_test/file_system/retain_entry/background.js @@ -0,0 +1,7 @@ +// 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.app.runtime.onLaunched.addListener(function () { + chrome.app.window.create('test.html'); +}); diff --git a/chrome/test/data/extensions/api_test/file_system/get_entry_id/manifest.json b/chrome/test/data/extensions/api_test/file_system/retain_entry/manifest.json index f45434a..77f237d 100644 --- a/chrome/test/data/extensions/api_test/file_system/get_entry_id/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system/retain_entry/manifest.json @@ -1,7 +1,7 @@ { - "name": "chrome.fileSystem.getEntryId test", + "name": "chrome.fileSystem.retainEntry test", "version": "0.1", - "description": "Test for chrome.fileSystem.getEntryId", + "description": "Test for chrome.fileSystem.retainEntry", "app": { "background": { "scripts": ["background.js"] diff --git a/chrome/test/data/extensions/api_test/file_system/retain_entry/test.html b/chrome/test/data/extensions/api_test/file_system/retain_entry/test.html new file mode 100644 index 0000000..d89c528 --- /dev/null +++ b/chrome/test/data/extensions/api_test/file_system/retain_entry/test.html @@ -0,0 +1,4 @@ +<html> +<script src="test_util.js"></script> +<script src="test.js"></script> +</html> diff --git a/chrome/test/data/extensions/api_test/file_system/retain_entry/test.js b/chrome/test/data/extensions/api_test/file_system/retain_entry/test.js new file mode 100644 index 0000000..9555e8e --- /dev/null +++ b/chrome/test/data/extensions/api_test/file_system/retain_entry/test.js @@ -0,0 +1,28 @@ +// Copyright 2013 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.test.runTests([ + function retainEntryWorks() { + chrome.app.window.create('test_other_window.html', chrome.test.callbackPass( + function(otherWindow) { + otherWindow.contentWindow.callback = chrome.test.callbackPass( + function(id, entry) { + otherWindow.close(); + chrome.fileSystem.isRestorable(id, chrome.test.callbackPass( + function(isRestorable) { + chrome.test.assertTrue(isRestorable); + })); + chrome.test.assertEq(chrome.fileSystem.retainEntry(entry), id); + chrome.fileSystem.restoreEntry(id, chrome.test.callbackPass( + function(restoredEntry) { + chrome.test.assertEq(restoredEntry, entry); + chrome.test.assertEq( + chrome.fileSystem.retainEntry(restoredEntry), id); + checkEntry(restoredEntry, 'writable.txt', false /* isNew */, + false /*shouldBeWritable */); + })); + }); + })); + } +]); diff --git a/chrome/test/data/extensions/api_test/file_system/get_entry_id/test_other_window.html b/chrome/test/data/extensions/api_test/file_system/retain_entry/test_other_window.html index a85a652..a85a652 100644 --- a/chrome/test/data/extensions/api_test/file_system/get_entry_id/test_other_window.html +++ b/chrome/test/data/extensions/api_test/file_system/retain_entry/test_other_window.html diff --git a/chrome/test/data/extensions/api_test/file_system/retain_entry/test_other_window.js b/chrome/test/data/extensions/api_test/file_system/retain_entry/test_other_window.js new file mode 100644 index 0000000..18a76a05 --- /dev/null +++ b/chrome/test/data/extensions/api_test/file_system/retain_entry/test_other_window.js @@ -0,0 +1,18 @@ +// Copyright (c) 2013 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.fileSystem.chooseEntry({type: 'openFile'}, + chrome.test.callbackPass(function(entry) { + var id = chrome.fileSystem.retainEntry(entry); + chrome.test.assertTrue(id != null); + chrome.fileSystem.isRestorable(id, chrome.test.callbackPass( + function(isRestorable) { + chrome.test.assertTrue(isRestorable); + })); + chrome.fileSystem.restoreEntry(id, chrome.test.callbackPass( + function(restoredEntry) { + chrome.test.assertEq(restoredEntry, entry); + })); + callback(id, entry); +})); diff --git a/chrome/test/data/extensions/api_test/file_system/retain_entry/test_util.js b/chrome/test/data/extensions/api_test/file_system/retain_entry/test_util.js new file mode 100644 index 0000000..c4845b2 --- /dev/null +++ b/chrome/test/data/extensions/api_test/file_system/retain_entry/test_util.js @@ -0,0 +1,62 @@ +// 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. + +// This is a duplicate of the file test_util in +// chrome/test/data/extensions/api_test/file_system + +function checkEntry(entry, expectedName, isNew, shouldBeWritable) { + chrome.test.assertEq(expectedName, entry.name); + // Test that the file can be read. + entry.file(chrome.test.callback(function(file) { + var reader = new FileReader(); + reader.onloadend = chrome.test.callbackPass(function(e) { + if (isNew) + chrome.test.assertEq(reader.result, ""); + else + chrome.test.assertEq(reader.result.indexOf("Can you see me?"), 0); + // Test that we can write to the file, or not, depending on + // |shouldBeWritable|. + entry.createWriter(function(fileWriter) { + fileWriter.onwriteend = chrome.test.callback(function(e) { + if (fileWriter.error) { + if (shouldBeWritable) { + chrome.test.fail("Error writing to file: " + + fileWriter.error.toString()); + } else { + chrome.test.succeed(); + } + } else { + if (shouldBeWritable) { + // Get a new entry and check the data got to disk. + chrome.fileSystem.chooseEntry(chrome.test.callbackPass( + function(readEntry) { + readEntry.file(chrome.test.callback(function(readFile) { + var readReader = new FileReader(); + readReader.onloadend = function(e) { + chrome.test.assertEq(readReader.result.indexOf("HoHoHo!"), + 0); + chrome.test.succeed(); + }; + readReader.onerror = function(e) { + chrome.test.fail("Failed to read file after write."); + }; + readReader.readAsText(readFile); + })); + })); + } else { + chrome.test.fail( + "'Could write to file that should not be writable."); + } + } + }); + var blob = new Blob(["HoHoHo!"], {type: "text/plain"}); + fileWriter.write(blob); + }); + }); + reader.onerror = chrome.test.callback(function(e) { + chrome.test.fail("Error reading file contents."); + }); + reader.readAsText(file); + })); +} diff --git a/chrome/test/data/extensions/platform_apps/file_access_restored_test/test.js b/chrome/test/data/extensions/platform_apps/file_access_restored_test/test.js index 07ebb0b..32a744a 100644 --- a/chrome/test/data/extensions/platform_apps/file_access_restored_test/test.js +++ b/chrome/test/data/extensions/platform_apps/file_access_restored_test/test.js @@ -26,7 +26,7 @@ chrome.app.runtime.onLaunched.addListener(function() { var fs = win.contentWindow.chrome.fileSystem; fs.chooseEntry({type: 'openFile'}, function(entry) { fs.getWritableEntry(entry, function(writableEntry) { - var id = fs.getEntryId(entry); + var id = fs.retainEntry(entry); chrome.storage.local.set({id:id}, function() { truncateAndWriteToFile(writableEntry, function() { chrome.test.sendMessage('fileWritten'); @@ -40,35 +40,36 @@ chrome.app.runtime.onLaunched.addListener(function() { chrome.app.runtime.onRestarted.addListener(function() { chrome.storage.local.get(null, function(data) { - var entry = chrome.fileSystem.getEntryById(data.id); - if (!entry) { - console.error("couldn't get file entry " + data.id); - return; - } - entry.file(function(file) { - var fr = new FileReader(); - fr.onload = function(e) { - if (e.target.result != expectedText) { - console.error( - "expected '" + expectedText + "', got '" + e.target.result + "'"); - return; - } - entry.createWriter(function(fileWriter) { - fileWriter.onwriteend = function(e) { - chrome.test.sendMessage('restartedFileAccessOK'); - win.close(); - }; - fileWriter.onerror = function(e) { - console.error('Write failed: ' + e.toString()); - }; - var blob = new Blob(["doesn't matter"], {type: 'text/plain'}); - fileWriter.write(blob); - }); - }; - fr.onerror = function(e) { - chrome.test.fail("error reading file"); - }; - fr.readAsText(file); + chrome.fileSystem.restoreEntry(data.id, function(entry) { + if (!entry) { + console.error("couldn't get file entry " + data.id); + return; + } + entry.file(function(file) { + var fr = new FileReader(); + fr.onload = function(e) { + if (e.target.result != expectedText) { + console.error("expected '" + expectedText + "', got '" + + e.target.result + "'"); + return; + } + entry.createWriter(function(fileWriter) { + fileWriter.onwriteend = function(e) { + chrome.test.sendMessage('restartedFileAccessOK'); + win.close(); + }; + fileWriter.onerror = function(e) { + console.error('Write failed: ' + e.toString()); + }; + var blob = new Blob(["doesn't matter"], {type: 'text/plain'}); + fileWriter.write(blob); + }); + }; + fr.onerror = function(e) { + chrome.test.fail("error reading file"); + }; + fr.readAsText(file); + }); }); }); }); diff --git a/chrome/test/data/extensions/platform_apps/file_access_saved_to_prefs_test/test.js b/chrome/test/data/extensions/platform_apps/file_access_saved_to_prefs_test/test.js index fe42334..d4ebd7c 100644 --- a/chrome/test/data/extensions/platform_apps/file_access_saved_to_prefs_test/test.js +++ b/chrome/test/data/extensions/platform_apps/file_access_saved_to_prefs_test/test.js @@ -25,7 +25,9 @@ chrome.app.runtime.onLaunched.addListener(function() { function(win) { var fs = win.contentWindow.chrome.fileSystem; fs.chooseEntry({type: 'openFile'}, function(entry) { + chrome.fileSystem.retainEntry(entry); fs.getWritableEntry(entry, function(writableEntry) { + chrome.fileSystem.retainEntry(writableEntry); truncateAndWriteToFile(writableEntry, function() { chrome.test.sendMessage('fileWritten'); win.close(); diff --git a/chrome/test/data/extensions/platform_apps/launch_application_octet_stream/test.js b/chrome/test/data/extensions/platform_apps/launch_application_octet_stream/test.js index 079f79c..6be819f 100644 --- a/chrome/test/data/extensions/platform_apps/launch_application_octet_stream/test.js +++ b/chrome/test/data/extensions/platform_apps/launch_application_octet_stream/test.js @@ -13,7 +13,7 @@ chrome.app.runtime.onLaunched.addListener(function (launchData) { chrome.test.assertEq(launchData.items[0].type, "application/octet-stream"); chrome.test.assertTrue( - chrome.fileSystem.getEntryId(launchData.items[0].entry) != null); + chrome.fileSystem.retainEntry(launchData.items[0].entry) != null); launchData.items[0].entry.file(function(file) { var reader = new FileReader(); diff --git a/chrome/test/data/extensions/platform_apps/launch_file/test.js b/chrome/test/data/extensions/platform_apps/launch_file/test.js index 7538174..115fff8 100644 --- a/chrome/test/data/extensions/platform_apps/launch_file/test.js +++ b/chrome/test/data/extensions/platform_apps/launch_file/test.js @@ -11,7 +11,7 @@ chrome.app.runtime.onLaunched.addListener(function (launchData) { "launchData.id incorrect"); chrome.test.assertEq(launchData.items.length, 1); chrome.test.assertTrue( - chrome.fileSystem.getEntryId(launchData.items[0].entry) != null); + chrome.fileSystem.retainEntry(launchData.items[0].entry) != null); launchData.items[0].entry.file(function(file) { var reader = new FileReader(); diff --git a/chrome/test/data/extensions/platform_apps/launch_file_by_extension/test.js b/chrome/test/data/extensions/platform_apps/launch_file_by_extension/test.js index 7538174..115fff8 100644 --- a/chrome/test/data/extensions/platform_apps/launch_file_by_extension/test.js +++ b/chrome/test/data/extensions/platform_apps/launch_file_by_extension/test.js @@ -11,7 +11,7 @@ chrome.app.runtime.onLaunched.addListener(function (launchData) { "launchData.id incorrect"); chrome.test.assertEq(launchData.items.length, 1); chrome.test.assertTrue( - chrome.fileSystem.getEntryId(launchData.items[0].entry) != null); + chrome.fileSystem.retainEntry(launchData.items[0].entry) != null); launchData.items[0].entry.file(function(file) { var reader = new FileReader(); diff --git a/chrome/test/data/extensions/platform_apps/launch_file_by_extension_and_type/test.js b/chrome/test/data/extensions/platform_apps/launch_file_by_extension_and_type/test.js index 7538174..115fff8 100644 --- a/chrome/test/data/extensions/platform_apps/launch_file_by_extension_and_type/test.js +++ b/chrome/test/data/extensions/platform_apps/launch_file_by_extension_and_type/test.js @@ -11,7 +11,7 @@ chrome.app.runtime.onLaunched.addListener(function (launchData) { "launchData.id incorrect"); chrome.test.assertEq(launchData.items.length, 1); chrome.test.assertTrue( - chrome.fileSystem.getEntryId(launchData.items[0].entry) != null); + chrome.fileSystem.retainEntry(launchData.items[0].entry) != null); launchData.items[0].entry.file(function(file) { var reader = new FileReader(); diff --git a/chrome/test/data/extensions/platform_apps/launch_file_with_any_extension/test.js b/chrome/test/data/extensions/platform_apps/launch_file_with_any_extension/test.js index 7538174..115fff8 100644 --- a/chrome/test/data/extensions/platform_apps/launch_file_with_any_extension/test.js +++ b/chrome/test/data/extensions/platform_apps/launch_file_with_any_extension/test.js @@ -11,7 +11,7 @@ chrome.app.runtime.onLaunched.addListener(function (launchData) { "launchData.id incorrect"); chrome.test.assertEq(launchData.items.length, 1); chrome.test.assertTrue( - chrome.fileSystem.getEntryId(launchData.items[0].entry) != null); + chrome.fileSystem.retainEntry(launchData.items[0].entry) != null); launchData.items[0].entry.file(function(file) { var reader = new FileReader(); diff --git a/chrome/test/data/extensions/platform_apps/launch_file_with_no_extension/test.js b/chrome/test/data/extensions/platform_apps/launch_file_with_no_extension/test.js index 7538174..115fff8 100644 --- a/chrome/test/data/extensions/platform_apps/launch_file_with_no_extension/test.js +++ b/chrome/test/data/extensions/platform_apps/launch_file_with_no_extension/test.js @@ -11,7 +11,7 @@ chrome.app.runtime.onLaunched.addListener(function (launchData) { "launchData.id incorrect"); chrome.test.assertEq(launchData.items.length, 1); chrome.test.assertTrue( - chrome.fileSystem.getEntryId(launchData.items[0].entry) != null); + chrome.fileSystem.retainEntry(launchData.items[0].entry) != null); launchData.items[0].entry.file(function(file) { var reader = new FileReader(); |