diff options
author | mpearson@chromium.org <mpearson@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-02-27 19:50:13 +0000 |
---|---|---|
committer | mpearson@chromium.org <mpearson@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-02-27 19:50:13 +0000 |
commit | b897ff873abc2344b8f46a2df7b4fad3881cf116 (patch) | |
tree | 36a79fb905a5cd4f03d60d9051dce47d725ece1c /apps | |
parent | 7550b5a1d1fc1d4e62f06a935d2d615cdd8494e4 (diff) | |
download | chromium_src-b897ff873abc2344b8f46a2df7b4fad3881cf116.zip chromium_src-b897ff873abc2344b8f46a2df7b4fad3881cf116.tar.gz chromium_src-b897ff873abc2344b8f46a2df7b4fad3881cf116.tar.bz2 |
Revert 184900
I think this revert, oddly, caused a series of failures on chromeos bots
http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20x64/builds/30936
like the top of the failures that it was supposed to have solved.
browser_tests browser_tests failed 3
Flakiness dashboard
( 44 mins, 15 secs )
stdio
FileSystemApiOpenBackgroundTest
FileSystemApiSaveBackgroundTest
FileAccessIsSavedToPrefs
Perhaps it wasn't the root cause after all.
This re-landing is an attempt to investigate what's going on here.
I reverted another change in the original blamelist an attempt to investigate further.
https://codereview.chromium.org/12334129/
I also tried disabling these tests to get more green in the tree. That hasn't kicked in yet, oddly.
https://codereview.chromium.org/12334128/
> Revert "Track and persist what file entries an extension has access to."
>
> This reverts commit 4fe61bc24f343a6a1381a782dec4fb45b2f6c437 (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
TBR=iannucci@chromium.org
Review URL: https://codereview.chromium.org/12317156
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@185007 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'apps')
-rw-r--r-- | apps/app_restore_service.cc | 11 | ||||
-rw-r--r-- | apps/app_restore_service.h | 15 | ||||
-rw-r--r-- | apps/app_restore_service_browsertest.cc | 46 |
3 files changed, 67 insertions, 5 deletions
diff --git a/apps/app_restore_service.cc b/apps/app_restore_service.cc index 554f873..f1b3fd3 100644 --- a/apps/app_restore_service.cc +++ b/apps/app_restore_service.cc @@ -5,6 +5,7 @@ #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" @@ -47,9 +48,11 @@ 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); + RestoreApp(*it, file_entries); } } } @@ -94,9 +97,13 @@ 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) { +void AppRestoreService::RestoreApp( + const Extension* extension, + const std::vector<SavedFileEntry>& file_entries) { + // TODO(koz): Make |file_entries| available to the newly restarted app. AppEventRouter::DispatchOnRestartedEvent(profile_, extension); } diff --git a/apps/app_restore_service.h b/apps/app_restore_service.h index f919a7a..0217f60 100644 --- a/apps/app_restore_service.h +++ b/apps/app_restore_service.h @@ -11,12 +11,19 @@ #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. @@ -37,7 +44,9 @@ class AppRestoreService : public ProfileKeyedService, void RecordAppStart(const std::string& extension_id); void RecordAppStop(const std::string& extension_id); - void RestoreApp(const extensions::Extension* extension); + void RestoreApp( + const extensions::Extension* extension, + const std::vector<SavedFileEntry>& file_entries); content::NotificationRegistrar registrar_; Profile* profile_; diff --git a/apps/app_restore_service_browsertest.cc b/apps/app_restore_service_browsertest.cc index 094812d..3eaf3ff 100644 --- a/apps/app_restore_service_browsertest.cc +++ b/apps/app_restore_service_browsertest.cc @@ -4,6 +4,8 @@ #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" @@ -12,9 +14,12 @@ #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; @@ -52,4 +57,45 @@ 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 |