diff options
author | calamity@chromium.org <calamity@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-10 06:48:36 +0000 |
---|---|---|
committer | calamity@chromium.org <calamity@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-10 06:48:36 +0000 |
commit | a9f74a6b78ecfb8f868d19d99e43a5679bb95ad4 (patch) | |
tree | 2d51f251c016ad809bd225771e8e90129779abde /chrome/browser | |
parent | 9eedbeeee1feeb6a3bbdae8826f21822835f9b30 (diff) | |
download | chromium_src-a9f74a6b78ecfb8f868d19d99e43a5679bb95ad4.zip chromium_src-a9f74a6b78ecfb8f868d19d99e43a5679bb95ad4.tar.gz chromium_src-a9f74a6b78ecfb8f868d19d99e43a5679bb95ad4.tar.bz2 |
Sync the launch type pref for apps.
This CL syncs the launch type pref for apps by adding a property to
app_specifics.proto. The sync data is processed in ExtensionSyncService.
extensions::SetLaunchType has been changed to take an ExtensionService
so that it is able to sync the pref change when a launch type is updated.
A value has been added to the LaunchType enum to represent an invalid
launch type preference.
BUG=181576
TBR=sky@chromium.org
Review URL: https://codereview.chromium.org/93883004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@244112 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
14 files changed, 160 insertions, 30 deletions
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc index 42a4b1e..b0293f4 100644 --- a/chrome/browser/automation/testing_automation_provider.cc +++ b/chrome/browser/automation/testing_automation_provider.cc @@ -4346,8 +4346,7 @@ void TestingAutomationProvider::SetAppLaunchType( return; } - extensions::SetLaunchType( - service->extension_prefs(), extension->id(), launch_type); + extensions::SetLaunchType(service, extension->id(), launch_type); reply.SendSuccess(NULL); } diff --git a/chrome/browser/extensions/api/management/management_apitest.cc b/chrome/browser/extensions/api/management/management_apitest.cc index 7d28505..3710b3a 100644 --- a/chrome/browser/extensions/api/management/management_apitest.cc +++ b/chrome/browser/extensions/api/management/management_apitest.cc @@ -220,8 +220,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementApiTest, DISABLED_LaunchPanelApp) { // Set a pref indicating that the user wants to launch in a regular tab. // This should be ignored, because panel apps always load in a popup. - extensions::SetLaunchType(service->extension_prefs(), - app_id, extensions::LAUNCH_TYPE_REGULAR); + extensions::SetLaunchType(service, app_id, extensions::LAUNCH_TYPE_REGULAR); // Load the extension again. std::string app_id_new; @@ -280,8 +279,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementApiTest, MAYBE_LaunchTabApp) { ASSERT_FALSE(service->GetExtensionById(app_id, true)); // Set a pref indicating that the user wants to launch in a window. - extensions::SetLaunchType(service->extension_prefs(), - app_id, extensions::LAUNCH_TYPE_WINDOW); + extensions::SetLaunchType(service, app_id, extensions::LAUNCH_TYPE_WINDOW); std::string app_id_new; LoadAndWaitForLaunch("management/launch_app_tab", &app_id_new); diff --git a/chrome/browser/extensions/app_sync_data.cc b/chrome/browser/extensions/app_sync_data.cc index f3a817d..ac0fb07 100644 --- a/chrome/browser/extensions/app_sync_data.cc +++ b/chrome/browser/extensions/app_sync_data.cc @@ -27,10 +27,12 @@ AppSyncData::AppSyncData(const Extension& extension, bool enabled, bool incognito_enabled, const syncer::StringOrdinal& app_launch_ordinal, - const syncer::StringOrdinal& page_ordinal) + const syncer::StringOrdinal& page_ordinal, + extensions::LaunchType launch_type) : extension_sync_data_(extension, enabled, incognito_enabled), app_launch_ordinal_(app_launch_ordinal), - page_ordinal_(page_ordinal) { + page_ordinal_(page_ordinal), + launch_type_(launch_type) { } AppSyncData::~AppSyncData() {} @@ -51,12 +53,22 @@ syncer::SyncChange AppSyncData::GetSyncChange( void AppSyncData::PopulateAppSpecifics(sync_pb::AppSpecifics* specifics) const { DCHECK(specifics); - // Only sync the ordinal values if they are valid. + // Only sync the ordinal values and launch type if they are valid. if (app_launch_ordinal_.IsValid()) specifics->set_app_launch_ordinal(app_launch_ordinal_.ToInternalValue()); if (page_ordinal_.IsValid()) specifics->set_page_ordinal(page_ordinal_.ToInternalValue()); + sync_pb::AppSpecifics::LaunchType sync_launch_type = + static_cast<sync_pb::AppSpecifics::LaunchType>(launch_type_); + + // The corresponding validation of this value during processing of an + // AppSyncData is in ExtensionSyncService::ProcessAppSyncData. + if (launch_type_ >= LAUNCH_TYPE_FIRST && launch_type_ < NUM_LAUNCH_TYPES && + sync_pb::AppSpecifics_LaunchType_IsValid(sync_launch_type)) { + specifics->set_launch_type(sync_launch_type); + } + extension_sync_data_.PopulateExtensionSpecifics( specifics->mutable_extension()); } @@ -67,6 +79,10 @@ void AppSyncData::PopulateFromAppSpecifics( app_launch_ordinal_ = syncer::StringOrdinal(specifics.app_launch_ordinal()); page_ordinal_ = syncer::StringOrdinal(specifics.page_ordinal()); + + launch_type_ = specifics.has_launch_type() + ? static_cast<extensions::LaunchType>(specifics.launch_type()) + : LAUNCH_TYPE_INVALID; } void AppSyncData::PopulateFromSyncData(const syncer::SyncData& sync_data) { diff --git a/chrome/browser/extensions/app_sync_data.h b/chrome/browser/extensions/app_sync_data.h index 0d2c67f..2e34460 100644 --- a/chrome/browser/extensions/app_sync_data.h +++ b/chrome/browser/extensions/app_sync_data.h @@ -6,6 +6,7 @@ #define CHROME_BROWSER_EXTENSIONS_APP_SYNC_DATA_H_ #include "chrome/browser/extensions/extension_sync_data.h" +#include "chrome/common/extensions/extension_constants.h" #include "sync/api/string_ordinal.h" #include "sync/api/sync_change.h" @@ -32,7 +33,8 @@ class AppSyncData { bool enabled, bool incognito_enabled, const syncer::StringOrdinal& app_launch_ordinal, - const syncer::StringOrdinal& page_ordinal); + const syncer::StringOrdinal& page_ordinal, + extensions::LaunchType launch_type); ~AppSyncData(); // Retrive sync data from this class. @@ -55,6 +57,10 @@ class AppSyncData { return extension_sync_data_; } + extensions::LaunchType launch_type() const { + return launch_type_; + } + private: // Convert an AppSyncData back out to a sync structure. void PopulateAppSpecifics(sync_pb::AppSpecifics* specifics) const; @@ -67,6 +73,7 @@ class AppSyncData { ExtensionSyncData extension_sync_data_; syncer::StringOrdinal app_launch_ordinal_; syncer::StringOrdinal page_ordinal_; + extensions::LaunchType launch_type_; }; } // namespace extensions diff --git a/chrome/browser/extensions/extension_sync_service.cc b/chrome/browser/extensions/extension_sync_service.cc index 9b875a1..1fb29b5 100644 --- a/chrome/browser/extensions/extension_sync_service.cc +++ b/chrome/browser/extensions/extension_sync_service.cc @@ -16,6 +16,7 @@ #include "chrome/browser/extensions/extension_sync_data.h" #include "chrome/browser/extensions/extension_sync_service_factory.h" #include "chrome/browser/extensions/extension_util.h" +#include "chrome/browser/extensions/launch_util.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sync/glue/sync_start_util.h" #include "chrome/browser/sync/sync_prefs.h" @@ -241,7 +242,9 @@ extensions::AppSyncData ExtensionSyncService::GetAppSyncData( extension_service_->IsExtensionEnabled(extension.id()), extension_util::IsIncognitoEnabled(extension.id(), extension_service_), extension_prefs_->app_sorting()->GetAppLaunchOrdinal(extension.id()), - extension_prefs_->app_sorting()->GetPageOrdinal(extension.id())); + extension_prefs_->app_sorting()->GetPageOrdinal(extension.id()), + extensions::GetLaunchTypePrefValue(extension_prefs_, extension.id())); + } std::vector<extensions::ExtensionSyncData> @@ -311,6 +314,14 @@ bool ExtensionSyncService::ProcessAppSyncData( app_sync_data.page_ordinal()); } + // The corresponding validation of this value during AppSyncData population + // is in AppSyncData::PopulateAppSpecifics. + if (app_sync_data.launch_type() >= extensions::LAUNCH_TYPE_FIRST && + app_sync_data.launch_type() < extensions::NUM_LAUNCH_TYPES) { + extensions::SetLaunchType(extension_service_, id, + app_sync_data.launch_type()); + } + if (!ProcessExtensionSyncDataHelper(app_sync_data.extension_sync_data(), syncer::APPS)) { app_sync_bundle_.AddPendingApp(id, app_sync_data); diff --git a/chrome/browser/extensions/launch_util.cc b/chrome/browser/extensions/launch_util.cc index 14423ae..e1f4d6f 100644 --- a/chrome/browser/extensions/launch_util.cc +++ b/chrome/browser/extensions/launch_util.cc @@ -7,6 +7,7 @@ #include "base/command_line.h" #include "base/values.h" #include "chrome/browser/extensions/extension_prefs.h" +#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/ui/host_desktop.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/extensions/extension_constants.h" @@ -32,7 +33,6 @@ const char kPrefLaunchType[] = "launchType"; LaunchType GetLaunchType(const ExtensionPrefs* prefs, const Extension* extension) { - int value = -1; LaunchType result = LAUNCH_TYPE_DEFAULT; // Launch hosted apps as windows by default for streamlined hosted apps. @@ -42,13 +42,10 @@ LaunchType GetLaunchType(const ExtensionPrefs* prefs, result = LAUNCH_TYPE_WINDOW; } - if (prefs->ReadPrefAsInteger(extension->id(), kPrefLaunchType, &value) && - (value == LAUNCH_TYPE_PINNED || - value == LAUNCH_TYPE_REGULAR || - value == LAUNCH_TYPE_FULLSCREEN || - value == LAUNCH_TYPE_WINDOW)) { + int value = GetLaunchTypePrefValue(prefs, extension->id()); + if (value >= LAUNCH_TYPE_FIRST && value < NUM_LAUNCH_TYPES) result = static_cast<LaunchType>(value); - } + #if defined(OS_MACOSX) // App windows are not yet supported on mac. Pref sync could make // the launch type LAUNCH_TYPE_WINDOW, even if there is no UI to set it @@ -66,11 +63,27 @@ LaunchType GetLaunchType(const ExtensionPrefs* prefs, return result; } -void SetLaunchType(ExtensionPrefs* prefs, +LaunchType GetLaunchTypePrefValue(const ExtensionPrefs* prefs, + const std::string& extension_id) { + int value = LAUNCH_TYPE_INVALID; + return prefs->ReadPrefAsInteger(extension_id, kPrefLaunchType, &value) + ? static_cast<LaunchType>(value) : LAUNCH_TYPE_INVALID; +} + +void SetLaunchType(ExtensionService* service, const std::string& extension_id, LaunchType launch_type) { - prefs->UpdateExtensionPref(extension_id, kPrefLaunchType, + DCHECK(launch_type >= LAUNCH_TYPE_FIRST && launch_type < NUM_LAUNCH_TYPES); + + service->extension_prefs()->UpdateExtensionPref(extension_id, kPrefLaunchType, new base::FundamentalValue(static_cast<int>(launch_type))); + + // Sync the launch type. + const Extension* extension = service->GetInstalledExtension(extension_id); + if (extension) { + ExtensionSyncService::Get(service->profile())-> + SyncExtensionChangeIfNeeded(*extension); + } } LaunchContainer GetLaunchContainer(const ExtensionPrefs* prefs, diff --git a/chrome/browser/extensions/launch_util.h b/chrome/browser/extensions/launch_util.h index 0cdfb72..cd5f693 100644 --- a/chrome/browser/extensions/launch_util.h +++ b/chrome/browser/extensions/launch_util.h @@ -9,6 +9,8 @@ #include "chrome/common/extensions/extension_constants.h" +class ExtensionService; + namespace extensions { class Extension; @@ -21,8 +23,13 @@ class ExtensionPrefs; LaunchType GetLaunchType(const ExtensionPrefs* prefs, const Extension* extension); -// Sets an extension's launch type preference. -void SetLaunchType(ExtensionPrefs* prefs, +// Returns the LaunchType that is set in the prefs. Returns LAUNCH_TYPE_INVALID +// if no value is set in prefs. +LaunchType GetLaunchTypePrefValue(const ExtensionPrefs* prefs, + const std::string& extension_id); + +// Sets an extension's launch type preference and syncs the value if necessary. +void SetLaunchType(ExtensionService* prefs, const std::string& extension_id, LaunchType launch_type); diff --git a/chrome/browser/sync/test/integration/sync_app_helper.cc b/chrome/browser/sync/test/integration/sync_app_helper.cc index 5e1f845..cde49ed 100644 --- a/chrome/browser/sync/test/integration/sync_app_helper.cc +++ b/chrome/browser/sync/test/integration/sync_app_helper.cc @@ -6,10 +6,12 @@ #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_system.h" +#include "chrome/browser/extensions/launch_util.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sync/test/integration/extensions_helper.h" #include "chrome/browser/sync/test/integration/sync_datatype_helper.h" #include "chrome/browser/sync/test/integration/sync_extension_helper.h" +#include "chrome/common/extensions/extension_constants.h" #include "chrome/common/extensions/sync_helper.h" #include "extensions/browser/app_sorting.h" #include "extensions/common/extension.h" @@ -26,11 +28,12 @@ struct AppState { syncer::StringOrdinal app_launch_ordinal; syncer::StringOrdinal page_ordinal; + extensions::LaunchType launch_type; }; typedef std::map<std::string, AppState> AppStateMap; -AppState::AppState() {} +AppState::AppState() : launch_type(extensions::LAUNCH_TYPE_INVALID) {} AppState::~AppState() {} @@ -40,7 +43,8 @@ bool AppState::IsValid() const { bool AppState::Equals(const AppState& other) const { return app_launch_ordinal.Equals(other.app_launch_ordinal) && - page_ordinal.Equals(other.page_ordinal); + page_ordinal.Equals(other.page_ordinal) && + launch_type == other.launch_type; } // Load all the app specific values for |id| into |app_state|. @@ -51,6 +55,9 @@ void LoadApp(ExtensionService* extension_service, app_sorting()->GetAppLaunchOrdinal(id); app_state->page_ordinal = extension_service->extension_prefs()-> app_sorting()->GetPageOrdinal(id); + app_state->launch_type = + extensions::GetLaunchTypePrefValue(extension_service->extension_prefs(), + id); } // Returns a map from |profile|'s installed extensions to their state. diff --git a/chrome/browser/sync/test/integration/two_client_apps_sync_test.cc b/chrome/browser/sync/test/integration/two_client_apps_sync_test.cc index 9d953db..8b9e9a9 100644 --- a/chrome/browser/sync/test/integration/two_client_apps_sync_test.cc +++ b/chrome/browser/sync/test/integration/two_client_apps_sync_test.cc @@ -4,6 +4,7 @@ #include "base/basictypes.h" #include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/launch_util.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sync/test/integration/apps_helper.h" #include "chrome/browser/sync/test/integration/profile_sync_service_harness.h" @@ -422,6 +423,75 @@ IN_PROC_BROWSER_TEST_F(TwoClientAppsSyncTest, UpdateCWSOrdinals) { ASSERT_TRUE(AllProfilesHaveSameAppsAsVerifier()); } +// Adjust the launch type on the first client and sync. Both clients should +// have the same launch type values for the CWS. +IN_PROC_BROWSER_TEST_F(TwoClientAppsSyncTest, UpdateLaunchType) { + ASSERT_TRUE(SetupSync()); + ASSERT_TRUE(AllProfilesHaveSameAppsAsVerifier()); + + // Change the launch type to window. + extensions::SetLaunchType(GetProfile(1)->GetExtensionService(), + extension_misc::kWebStoreAppId, + extensions::LAUNCH_TYPE_WINDOW); + extensions::SetLaunchType(verifier()->GetExtensionService(), + extension_misc::kWebStoreAppId, + extensions::LAUNCH_TYPE_WINDOW); + ASSERT_TRUE(AwaitQuiescence()); + ASSERT_TRUE(AllProfilesHaveSameAppsAsVerifier()); + + // Change the launch type to regular tab. + extensions::SetLaunchType(GetProfile(1)->GetExtensionService(), + extension_misc::kWebStoreAppId, + extensions::LAUNCH_TYPE_REGULAR); + ASSERT_FALSE(HasSameAppsAsVerifier(1)); + extensions::SetLaunchType(verifier()->GetExtensionService(), + extension_misc::kWebStoreAppId, + extensions::LAUNCH_TYPE_REGULAR); + ASSERT_TRUE(AwaitQuiescence()); + ASSERT_TRUE(AllProfilesHaveSameAppsAsVerifier()); +} + +IN_PROC_BROWSER_TEST_F(TwoClientAppsSyncTest, UnexpectedLaunchType) { + ASSERT_TRUE(SetupSync()); + ASSERT_TRUE(AllProfilesHaveSameAppsAsVerifier()); + + extensions::SetLaunchType(GetProfile(1)->GetExtensionService(), + extension_misc::kWebStoreAppId, + extensions::LAUNCH_TYPE_REGULAR); + extensions::SetLaunchType(verifier()->GetExtensionService(), + extension_misc::kWebStoreAppId, + extensions::LAUNCH_TYPE_REGULAR); + ASSERT_TRUE(AwaitQuiescence()); + ASSERT_TRUE(AllProfilesHaveSameAppsAsVerifier()); + + const extensions::Extension* extension = + GetProfile(1)->GetExtensionService()-> + GetInstalledExtension(extension_misc::kWebStoreAppId); + ASSERT_TRUE(extension); + + ExtensionSyncService* extension_sync_service = + ExtensionSyncService::Get(GetProfile(1)); + + extensions::AppSyncData original_data( + extension_sync_service->GetAppSyncData(*extension)); + + // Create an invalid launch type and ensure it doesn't get down-synced. This + // simulates the case of a future launch type being added which old versions + // don't yet understand. + extensions::AppSyncData invalid_launch_type_data( + *extension, + original_data.extension_sync_data().enabled(), + original_data.extension_sync_data().incognito_enabled(), + original_data.app_launch_ordinal(), + original_data.page_ordinal(), + extensions::NUM_LAUNCH_TYPES); + extension_sync_service->ProcessAppSyncData(invalid_launch_type_data); + + // The launch type should remain the same. + ASSERT_TRUE(AwaitQuiescence()); + ASSERT_TRUE(AllProfilesHaveSameAppsAsVerifier()); +} + // TODO(akalin): Add tests exercising: // - Offline installation/uninstallation behavior // - App-specific properties diff --git a/chrome/browser/ui/app_list/app_list_controller_delegate.cc b/chrome/browser/ui/app_list/app_list_controller_delegate.cc index 66cc94a..af06152 100644 --- a/chrome/browser/ui/app_list/app_list_controller_delegate.cc +++ b/chrome/browser/ui/app_list/app_list_controller_delegate.cc @@ -148,7 +148,7 @@ void AppListControllerDelegate::SetExtensionLaunchType( ExtensionService* service = extensions::ExtensionSystem::Get(profile)->extension_service(); extensions::SetLaunchType( - service->extension_prefs(), extension_id, launch_type); + service, extension_id, launch_type); } bool AppListControllerDelegate::IsExtensionInstalled( diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc index f2954bc4..1acf755 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc @@ -819,7 +819,7 @@ void ChromeLauncherController::SetLaunchType( if (!HasItemController(id)) return; - extensions::SetLaunchType(profile_->GetExtensionService()->extension_prefs(), + extensions::SetLaunchType(profile_->GetExtensionService(), id_to_item_controller_map_[id]->app_id(), launch_type); } diff --git a/chrome/browser/ui/ash/launcher/launcher_context_menu.cc b/chrome/browser/ui/ash/launcher/launcher_context_menu.cc index c85c0ad..36d7d6e 100644 --- a/chrome/browser/ui/ash/launcher/launcher_context_menu.cc +++ b/chrome/browser/ui/ash/launcher/launcher_context_menu.cc @@ -204,6 +204,9 @@ base::string16 LauncherContextMenu::GetLabelForCommandId(int command_id) const { case extensions::LAUNCH_TYPE_FULLSCREEN: case extensions::LAUNCH_TYPE_WINDOW: return l10n_util::GetStringUTF16(IDS_LAUNCHER_CONTEXT_MENU_NEW_WINDOW); + default: + NOTREACHED(); + return base::string16(); } } NOTREACHED(); diff --git a/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc b/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc index 88f2acd..13887c5 100644 --- a/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc +++ b/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc @@ -122,7 +122,7 @@ class StartupBrowserCreatorTest : public ExtensionBrowserTest { extensions::LaunchType launch_type) { ExtensionService* service = extensions::ExtensionSystem::Get( browser()->profile())->extension_service(); - extensions::SetLaunchType(service->extension_prefs(), app_id, launch_type); + extensions::SetLaunchType(service, app_id, launch_type); } Browser* FindOneOtherBrowserForProfile(Profile* profile, diff --git a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc index 3330da6..cada323 100644 --- a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc +++ b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc @@ -568,10 +568,9 @@ void AppLauncherHandler::HandleSetLaunchType(const base::ListValue* args) { base::AutoReset<bool> auto_reset(&ignore_changes_, true); extensions::SetLaunchType( - extension_service_->extension_prefs(), + extension_service_, extension_id, - static_cast<extensions::LaunchType>( - static_cast<int>(launch_type))); + static_cast<extensions::LaunchType>(static_cast<int>(launch_type))); } void AppLauncherHandler::HandleUninstallApp(const base::ListValue* args) { |