Skip to content

Commit

Permalink
Revert "Track and persist what file entries an extension has access to."
Browse files Browse the repository at this point in the history 
This reverts commit 4fe61bc (r184878).

This was causing test failures in FileSystemApiOpenBackgroundTest and
FileSystemApiSaveBackgroundTest on linux (example
http://build.chromium.org/p/chromium.linux/buildstatus?builder=Linux%20Tests%20%28dbg%29%281%29&number=23657 )

TBR=koz@chromium.org
BUG=

Review URL: https://codereview.chromium.org/12319140

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@184900 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
iannucci@chromium.org committed Feb 27, 2013
1 parent 4c1fd7d commit b8816a4
Show file tree
Hide file tree
Showing 13 changed files with 26 additions and 285 deletions.
11 changes: 2 additions & 9 deletions apps/app_restore_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "apps/app_restore_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_service.h"
Expand Down Expand Up @@ -48,11 +47,9 @@ 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;
extension_prefs->GetSavedFileEntries(extension->id(), &file_entries);
RecordAppStop(extension->id());
if (should_restore_apps)
RestoreApp(*it, file_entries);
RestoreApp(*it);
}
}
}
Expand Down Expand Up @@ -97,13 +94,9 @@ void AppRestoreService::RecordAppStop(const std::string& extension_id) {
ExtensionPrefs* extension_prefs =
ExtensionSystem::Get(profile_)->extension_service()->extension_prefs();
extension_prefs->SetExtensionRunning(extension_id, false);
extension_prefs->ClearSavedFileEntries(extension_id);
}

void AppRestoreService::RestoreApp(
const Extension* extension,
const std::vector<SavedFileEntry>& file_entries) {
// TODO(koz): Make |file_entries| available to the newly restarted app.
void AppRestoreService::RestoreApp(const Extension* extension) {
AppEventRouter::DispatchOnRestartedEvent(profile_, extension);
}

Expand Down
15 changes: 3 additions & 12 deletions apps/app_restore_service.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,12 @@
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"

class Profile;

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.
Expand All @@ -44,9 +37,7 @@ class AppRestoreService : public ProfileKeyedService,

void RecordAppStart(const std::string& extension_id);
void RecordAppStop(const std::string& extension_id);
void RestoreApp(
const extensions::Extension* extension,
const std::vector<SavedFileEntry>& file_entries);
void RestoreApp(const extensions::Extension* extension);

content::NotificationRegistrar registrar_;
Profile* profile_;
Expand Down
46 changes: 0 additions & 46 deletions apps/app_restore_service_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

#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 "chrome/browser/extensions/api/file_system/file_system_api.h"
#include "chrome/browser/extensions/extension_prefs.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_system.h"
Expand All @@ -14,12 +12,9 @@
#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;
using extensions::FileSystemChooseEntryFunction;

// TODO(benwells): Move PlatformAppBrowserTest to apps namespace in apps
// component.
using extensions::PlatformAppBrowserTest;
Expand Down Expand Up @@ -57,45 +52,4 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, RunningAppsAreRecorded) {
restart_listener.WaitUntilSatisfied();
}

IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, FileAccessIsSavedToPrefs) {
content::WindowedNotificationObserver extension_suspended(
chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,
content::NotificationService::AllSources());

base::ScopedTempDir temp_directory;
ASSERT_TRUE(temp_directory.CreateUniqueTempDir());
base::FilePath temp_file;
ASSERT_TRUE(file_util::CreateTemporaryFileInDir(temp_directory.path(),
&temp_file));

FileSystemChooseEntryFunction::SkipPickerAndAlwaysSelectPathForTest(
&temp_file);

ExtensionTestMessageListener file_written_listener("fileWritten", false);
ExtensionTestMessageListener access_ok_listener(
"restartedFileAccessOK", false);

const Extension* extension =
LoadAndLaunchPlatformApp("file_access_saved_to_prefs_test");
ASSERT_TRUE(extension);
file_written_listener.WaitUntilSatisfied();

ExtensionService* extension_service =
ExtensionSystem::Get(browser()->profile())->extension_service();
ExtensionPrefs* extension_prefs = extension_service->extension_prefs();

// Record the file entries in prefs because when the app gets suspended it
// will have them all cleared.
std::vector<SavedFileEntry> file_entries;
extension_prefs->GetSavedFileEntries(extension->id(), &file_entries);
// 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();
extension_prefs->GetSavedFileEntries(extension->id(), &file_entries);
// File entries should be cleared when the extension is suspended.
ASSERT_TRUE(file_entries.empty());
}

} // namespace apps
42 changes: 0 additions & 42 deletions chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,7 @@

#include "chrome/browser/extensions/api/file_handlers/app_file_handler_util.h"

#include "chrome/browser/extensions/extension_prefs.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_system.h"
#include "content/public/browser/child_process_security_policy.h"
#include "net/base/mime_util.h"
#include "webkit/fileapi/file_system_types.h"
#include "webkit/fileapi/isolated_context.h"

namespace extensions {

Expand Down Expand Up @@ -88,42 +82,6 @@ bool FileHandlerCanHandleFileWithMimeType(
return false;
}

GrantedFileEntry CreateFileEntry(
Profile* profile,
const std::string& extension_id,
int renderer_id,
const base::FilePath& path,
bool writable) {
GrantedFileEntry result;
fileapi::IsolatedContext* isolated_context =
fileapi::IsolatedContext::GetInstance();
DCHECK(isolated_context);

result.filesystem_id = isolated_context->RegisterFileSystemForPath(
fileapi::kFileSystemTypeNativeLocal, path, &result.registered_name);

content::ChildProcessSecurityPolicy* policy =
content::ChildProcessSecurityPolicy::GetInstance();
policy->GrantReadFileSystem(renderer_id, result.filesystem_id);
if (writable)
policy->GrantWriteFileSystem(renderer_id, result.filesystem_id);

result.id = result.filesystem_id + ":" + result.registered_name;

// We only need file level access for reading FileEntries. Saving FileEntries
// just needs the file system to have read/write access, which is granted
// above if required.
if (!policy->CanReadFile(renderer_id, path))
policy->GrantReadFile(renderer_id, path);

ExtensionPrefs* prefs = extensions::ExtensionSystem::Get(profile)->
extension_service()->extension_prefs();
// Save this file entry in the prefs.
prefs->AddSavedFileEntry(extension_id, result.id, path, writable);

return result;
}

} // namespace app_file_handler_util

} // namespace extensions
34 changes: 0 additions & 34 deletions chrome/browser/extensions/api/file_handlers/app_file_handler_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
#include "chrome/common/extensions/api/file_handlers/file_handlers_parser.h"
#include "chrome/common/extensions/extension.h"

class Profile;

namespace extensions {

// TODO(benwells): move this to platform_apps namespace.
Expand All @@ -36,38 +34,6 @@ FindFileHandlersForMimeTypes(const Extension& extension,
bool FileHandlerCanHandleFileWithMimeType(const FileHandlerInfo& handler,
const std::string& mime_type);

// 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;
std::string filesystem_id;
std::string registered_name;
};

// Creates a new file entry and allows |renderer_id| to access |path|. This
// registers a new file system for |path|.
GrantedFileEntry CreateFileEntry(
Profile* profile,
const std::string& extension_id,
int renderer_id,
const base::FilePath& path,
bool writable);

} // namespace app_file_handler_util

} // namespace extensions
Expand Down
31 changes: 19 additions & 12 deletions chrome/browser/extensions/api/file_system/file_system_api.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "base/string_util.h"
#include "base/sys_string_conversions.h"
#include "base/utf_string_conversions.h"
#include "chrome/browser/extensions/api/file_handlers/app_file_handler_util.h"
#include "chrome/browser/extensions/shell_window_registry.h"
#include "chrome/browser/platform_util.h"
#include "chrome/browser/ui/chrome_select_file_policy.h"
Expand Down Expand Up @@ -41,8 +40,7 @@ using fileapi::IsolatedContext;

const char kInvalidParameters[] = "Invalid parameters";
const char kSecurityError[] = "Security error";
const char kInvalidCallingPage[] = "Invalid calling page. This function can't "
"be called from a background page.";
const char kInvalidCallingPage[] = "Invalid calling page";
const char kUserCancelled[] = "User cancelled";
const char kWritableFileError[] = "Invalid file for writing";
const char kRequiresFileSystemWriteError[] =
Expand Down Expand Up @@ -302,18 +300,27 @@ void FileSystemEntryFunction::RegisterFileSystemAndSendResponse(
fileapi::IsolatedContext::GetInstance();
DCHECK(isolated_context);

bool writable = entry_type == WRITABLE;
extensions::app_file_handler_util::GrantedFileEntry file_entry =
extensions::app_file_handler_util::CreateFileEntry(profile(),
GetExtension()->id(), render_view_host_->GetProcess()->GetID(), path,
writable);
std::string registered_name;
std::string filesystem_id = isolated_context->RegisterFileSystemForPath(
fileapi::kFileSystemTypeNativeLocal, path, &registered_name);

content::ChildProcessSecurityPolicy* policy =
content::ChildProcessSecurityPolicy::GetInstance();
int renderer_id = render_view_host_->GetProcess()->GetID();
policy->GrantReadFileSystem(renderer_id, filesystem_id);
if (entry_type == WRITABLE)
policy->GrantWriteFileSystem(renderer_id, filesystem_id);

// We only need file level access for reading FileEntries. Saving FileEntries
// just needs the file system to have read/write access, which is granted
// above if required.
if (!policy->CanReadFile(renderer_id, path))
policy->GrantReadFile(renderer_id, path);

DictionaryValue* dict = new DictionaryValue();
SetResult(dict);
dict->SetString("fileSystemId", file_entry.filesystem_id);
dict->SetString("baseName", file_entry.registered_name);
dict->SetString("id", file_entry.id);

dict->SetString("fileSystemId", filesystem_id);
dict->SetString("baseName", registered_name);
SendResponse(true);
}

Expand Down
65 changes: 0 additions & 65 deletions chrome/browser/extensions/extension_prefs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "base/utf_string_conversions.h"
#include "base/version.h"
#include "chrome/browser/extensions/admin_policy.h"
#include "chrome/browser/extensions/api/file_handlers/app_file_handler_util.h"
#include "chrome/browser/extensions/api/omnibox/omnibox_api.h"
#include "chrome/browser/extensions/extension_pref_store.h"
#include "chrome/browser/extensions/extension_sorting.h"
Expand Down Expand Up @@ -42,15 +41,6 @@ namespace {

// Additional preferences keys

// The file entries that an extension had 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";

// Whether this extension was running when chrome last shutdown.
const char kPrefRunning[] = "running";

Expand Down Expand Up @@ -1142,61 +1132,6 @@ bool ExtensionPrefs::IsExtensionRunning(const std::string& extension_id) {
return running;
}

void ExtensionPrefs::AddSavedFileEntry(
const std::string& extension_id,
const std::string& file_entry_id,
const base::FilePath& file_path,
bool writable) {
ScopedExtensionPrefUpdate update(prefs_, extension_id);
DictionaryValue* extension_dict = update.Get();
DictionaryValue* file_entries = NULL;
if (!extension_dict->GetDictionary(kFileEntries, &file_entries)) {
file_entries = new DictionaryValue();
extension_dict->SetWithoutPathExpansion(kFileEntries, file_entries);
}
// 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 ExtensionPrefs::ClearSavedFileEntries(
const std::string& extension_id) {
ScopedExtensionPrefUpdate update(prefs_, extension_id);
DictionaryValue* extension_dict = update.Get();
extension_dict->Remove(kFileEntries, NULL);
}

void ExtensionPrefs::GetSavedFileEntries(
const std::string& extension_id,
std::vector<app_file_handler_util::SavedFileEntry>* out) {
const DictionaryValue* prefs = GetExtensionPref(extension_id);
const DictionaryValue* file_entries = NULL;
if (!prefs->GetDictionary(kFileEntries, &file_entries))
return;
for (DictionaryValue::key_iterator it = file_entries->begin_keys();
it != file_entries->end_keys(); ++it) {
std::string id = *it;
const DictionaryValue* file_entry = NULL;
if (!file_entries->GetDictionaryWithoutPathExpansion(id, &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(app_file_handler_util::SavedFileEntry(
id, file_path, writable));
}
}

ExtensionOmniboxSuggestion
ExtensionPrefs::GetOmniboxDefaultSuggestion(const std::string& extension_id) {
ExtensionOmniboxSuggestion suggestion;
Expand Down
Loading

0 comments on commit b8816a4

Please sign in to comment.