Commit b169a83c authored by Alan Cutter's avatar Alan Cutter Committed by Commit Bot

Transfer user UI preferences as part of hosted app to web app migration

This CL extends WebAppUiManagerImpl::UninstallAndReplace() to also
migrate the following user preferences:
 - chrome://apps grid position.
 - standalone/browser tab launch preference.

Pre-migration screenshot:
https://bugs.chromium.org/p/chromium/issues/attachment?aid=462010&signed_aid=oHAn32fAhc9IyNofMQ7C9Q==&inline=1

Post-migration screenshot before:
https://bugs.chromium.org/p/chromium/issues/attachment?aid=462009&signed_aid=LhJc8aoValNlvgCzMUSiUg==&inline=1

Post-migration screenshot after:
https://bugs.chromium.org/p/chromium/issues/attachment?aid=462008&signed_aid=cvtBWSuplfvc9sMKp2XvCw==&inline=1

Bug: 1105338
Change-Id: I908d11870c940f27485ac1436570659313ae794f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2358938Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798906}
parent cbb67783
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/apps/app_service/app_service_proxy.h" #include "chrome/browser/apps/app_service/app_service_proxy.h"
#include "chrome/browser/apps/app_service/app_service_proxy_factory.h" #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
#include "chrome/browser/extensions/launch_util.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/browser_finder.h"
...@@ -18,8 +19,10 @@ ...@@ -18,8 +19,10 @@
#include "chrome/browser/ui/web_applications/web_app_dialog_manager.h" #include "chrome/browser/ui/web_applications/web_app_dialog_manager.h"
#include "chrome/browser/ui/web_applications/web_app_launch_utils.h" #include "chrome/browser/ui/web_applications/web_app_launch_utils.h"
#include "chrome/browser/ui/web_applications/web_app_metrics.h" #include "chrome/browser/ui/web_applications/web_app_metrics.h"
#include "chrome/browser/web_applications/components/app_registry_controller.h"
#include "chrome/browser/web_applications/system_web_app_manager.h" #include "chrome/browser/web_applications/system_web_app_manager.h"
#include "chrome/browser/web_applications/web_app_provider.h" #include "chrome/browser/web_applications/web_app_provider.h"
#include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
#include "extensions/browser/app_sorting.h" #include "extensions/browser/app_sorting.h"
#include "extensions/browser/extension_system.h" #include "extensions/browser/extension_system.h"
...@@ -59,6 +62,11 @@ WebAppUiManagerImpl::WebAppUiManagerImpl(Profile* profile) ...@@ -59,6 +62,11 @@ WebAppUiManagerImpl::WebAppUiManagerImpl(Profile* profile)
WebAppUiManagerImpl::~WebAppUiManagerImpl() = default; WebAppUiManagerImpl::~WebAppUiManagerImpl() = default;
void WebAppUiManagerImpl::SetSubsystems(
AppRegistryController* app_registry_controller) {
app_registry_controller_ = app_registry_controller;
}
void WebAppUiManagerImpl::Start() { void WebAppUiManagerImpl::Start() {
DCHECK(!started_); DCHECK(!started_);
started_ = true; started_ = true;
...@@ -122,6 +130,7 @@ void WebAppUiManagerImpl::UninstallAndReplace( ...@@ -122,6 +130,7 @@ void WebAppUiManagerImpl::UninstallAndReplace(
for (const AppId& from_app : from_apps) { for (const AppId& from_app : from_apps) {
if (!has_migrated) { if (!has_migrated) {
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
// Grid position in app list.
auto* app_list_syncable_service = auto* app_list_syncable_service =
app_list::AppListSyncableServiceFactory::GetForProfile(profile_); app_list::AppListSyncableServiceFactory::GetForProfile(profile_);
if (app_list_syncable_service->GetSyncItem(from_app)) { if (app_list_syncable_service->GetSyncItem(from_app)) {
...@@ -129,6 +138,39 @@ void WebAppUiManagerImpl::UninstallAndReplace( ...@@ -129,6 +138,39 @@ void WebAppUiManagerImpl::UninstallAndReplace(
has_migrated = true; has_migrated = true;
} }
#endif #endif
// If migration of user/UI data is required for other app types consider
// generalising this operation to be part of app service.
const extensions::Extension* from_extension =
extensions::ExtensionRegistry::Get(profile_)
->enabled_extensions()
.GetByID(from_app);
if (from_extension) {
// Grid position in chrome://apps.
extensions::AppSorting* app_sorting =
extensions::ExtensionSystem::Get(profile_)->app_sorting();
app_sorting->SetAppLaunchOrdinal(
to_app, app_sorting->GetAppLaunchOrdinal(from_app));
app_sorting->SetPageOrdinal(to_app,
app_sorting->GetPageOrdinal(from_app));
// User pref for window/tab launch.
switch (extensions::GetLaunchContainer(
extensions::ExtensionPrefs::Get(profile_), from_extension)) {
case extensions::LaunchContainer::kLaunchContainerWindow:
case extensions::LaunchContainer::kLaunchContainerPanelDeprecated:
app_registry_controller_->SetAppUserDisplayMode(
to_app, DisplayMode::kStandalone);
break;
case extensions::LaunchContainer::kLaunchContainerTab:
case extensions::LaunchContainer::kLaunchContainerNone:
app_registry_controller_->SetAppUserDisplayMode(
to_app, DisplayMode::kBrowser);
break;
}
has_migrated = true;
}
} }
apps::AppServiceProxy* proxy = apps::AppServiceProxy* proxy =
......
...@@ -34,6 +34,7 @@ class WebAppUiManagerImpl : public BrowserListObserver, public WebAppUiManager { ...@@ -34,6 +34,7 @@ class WebAppUiManagerImpl : public BrowserListObserver, public WebAppUiManager {
explicit WebAppUiManagerImpl(Profile* profile); explicit WebAppUiManagerImpl(Profile* profile);
~WebAppUiManagerImpl() override; ~WebAppUiManagerImpl() override;
void SetSubsystems(AppRegistryController* app_registry_controller) override;
void Start() override; void Start() override;
void Shutdown() override; void Shutdown() override;
...@@ -75,6 +76,8 @@ class WebAppUiManagerImpl : public BrowserListObserver, public WebAppUiManager { ...@@ -75,6 +76,8 @@ class WebAppUiManagerImpl : public BrowserListObserver, public WebAppUiManager {
Profile* const profile_; Profile* const profile_;
AppRegistryController* app_registry_controller_ = nullptr;
std::map<AppId, std::vector<base::OnceClosure>> windows_closed_requests_map_; std::map<AppId, std::vector<base::OnceClosure>> windows_closed_requests_map_;
std::map<AppId, size_t> num_windows_for_apps_map_; std::map<AppId, size_t> num_windows_for_apps_map_;
bool started_ = false; bool started_ = false;
......
...@@ -19,6 +19,7 @@ class WebContents; ...@@ -19,6 +19,7 @@ class WebContents;
namespace web_app { namespace web_app {
class AppRegistryController;
// WebAppUiManagerImpl can be used only in UI code. // WebAppUiManagerImpl can be used only in UI code.
class WebAppUiManagerImpl; class WebAppUiManagerImpl;
...@@ -32,6 +33,8 @@ class WebAppUiManager { ...@@ -32,6 +33,8 @@ class WebAppUiManager {
virtual ~WebAppUiManager() = default; virtual ~WebAppUiManager() = default;
virtual void SetSubsystems(
AppRegistryController* app_registry_controller) = 0;
virtual void Start() = 0; virtual void Start() = 0;
virtual void Shutdown() = 0; virtual void Shutdown() = 0;
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/external_provider_impl.h" #include "chrome/browser/extensions/external_provider_impl.h"
#include "chrome/browser/extensions/external_testing_loader.h" #include "chrome/browser/extensions/external_testing_loader.h"
#include "chrome/browser/extensions/launch_util.h"
#include "chrome/browser/extensions/updater/extension_updater.h" #include "chrome/browser/extensions/updater/extension_updater.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/web_applications/test/ssl_test_utils.h" #include "chrome/browser/ui/web_applications/test/ssl_test_utils.h"
...@@ -22,6 +23,7 @@ ...@@ -22,6 +23,7 @@
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "content/public/test/browser_test.h" #include "content/public/test/browser_test.h"
#include "content/public/test/url_loader_interceptor.h" #include "content/public/test/url_loader_interceptor.h"
#include "extensions/browser/app_sorting.h"
#include "extensions/browser/extension_registry.h" #include "extensions/browser/extension_registry.h"
#include "extensions/browser/extension_system.h" #include "extensions/browser/extension_system.h"
#include "extensions/browser/test_extension_registry_observer.h" #include "extensions/browser/test_extension_registry_observer.h"
...@@ -30,6 +32,14 @@ ...@@ -30,6 +32,14 @@
#include "net/test/embedded_test_server/http_request.h" #include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h" #include "net/test/embedded_test_server/http_response.h"
#if defined(OS_CHROMEOS)
#include "ash/public/cpp/app_list/app_list_types.h"
#include "chrome/browser/ui/app_list/app_list_model_updater.h"
#include "chrome/browser/ui/app_list/app_list_syncable_service.h"
#include "chrome/browser/ui/app_list/app_list_syncable_service_factory.h"
#include "chrome/browser/ui/app_list/chrome_app_list_item.h"
#endif
namespace { namespace {
constexpr char kMigrationFlag[] = "MigrationTest"; constexpr char kMigrationFlag[] = "MigrationTest";
...@@ -244,4 +254,98 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppMigrationBrowserTest, MigrateAndRevert) { ...@@ -244,4 +254,98 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppMigrationBrowserTest, MigrateAndRevert) {
} }
} }
IN_PROC_BROWSER_TEST_F(ExternalWebAppMigrationBrowserTest, MigratePreferences) {
#if defined(OS_CHROMEOS)
app_list::AppListSyncableService* app_list_syncable_service =
app_list::AppListSyncableServiceFactory::GetForProfile(profile());
AppListModelUpdater* app_list_model_updater =
app_list_syncable_service->GetModelUpdater();
app_list_model_updater->SetActive(true);
#endif
extensions::AppSorting* app_sorting =
extensions::ExtensionSystem::Get(profile())->app_sorting();
// Set up pre-migration state.
{
ASSERT_FALSE(IsExternalAppInstallFeatureEnabled(kMigrationFlag));
SyncExternalExtensions();
SyncExternalWebApps(/*expect_install=*/false, /*expect_uninstall=*/false);
EXPECT_FALSE(IsWebAppInstalled());
EXPECT_TRUE(IsExtensionAppInstalled());
#if defined(OS_CHROMEOS)
ChromeAppListItem* app_list_item =
app_list_model_updater->FindItem(kExtensionId);
app_list_item->SetPosition(syncer::StringOrdinal("testapplistposition"));
app_list_model_updater->OnItemUpdated(app_list_item->CloneMetadata());
app_list_syncable_service->SetPinPosition(
kExtensionId, syncer::StringOrdinal("testpinposition"));
#endif
// Set chrome://apps position.
app_sorting->SetAppLaunchOrdinal(kExtensionId,
syncer::StringOrdinal("testapplaunch"));
app_sorting->SetPageOrdinal(kExtensionId,
syncer::StringOrdinal("testpageordinal"));
// Set user preference to launch as browser tab.
extensions::SetLaunchType(profile(), kExtensionId,
extensions::LAUNCH_TYPE_REGULAR);
}
// Migrate extension app to web app.
{
base::AutoReset<bool> testing_scope =
SetExternalAppInstallFeatureAlwaysEnabledForTesting();
ASSERT_TRUE(IsExternalAppInstallFeatureEnabled(kMigrationFlag));
SyncExternalExtensions();
// Extension sticks around to be uninstalled by the replacement web app.
EXPECT_TRUE(IsExtensionAppInstalled());
{
extensions::TestExtensionRegistryObserver uninstall_observer(
extensions::ExtensionRegistry::Get(profile()));
SyncExternalWebApps(/*expect_install=*/true, /*expect_uninstall=*/false);
EXPECT_TRUE(IsWebAppInstalled());
scoped_refptr<const extensions::Extension> uninstalled_app =
uninstall_observer.WaitForExtensionUninstalled();
EXPECT_EQ(uninstalled_app->id(), kExtensionId);
EXPECT_FALSE(IsExtensionAppInstalled());
}
}
// Check UI preferences have migrated across.
{
const AppId web_app_id = GetWebAppId();
#if defined(OS_CHROMEOS)
// Chrome OS shelf/list position should migrate.
EXPECT_EQ(app_list_model_updater->FindItem(web_app_id)
->position()
.ToDebugString(),
"testapplistposition");
EXPECT_EQ(
app_list_syncable_service->GetPinPosition(web_app_id).ToDebugString(),
"testpinposition");
#endif
// chrome://apps position should migrate.
EXPECT_EQ(app_sorting->GetAppLaunchOrdinal(web_app_id).ToDebugString(),
"testapplaunch");
EXPECT_EQ(app_sorting->GetPageOrdinal(web_app_id).ToDebugString(),
"testpageordinal");
// User launch preference should migrate across and override
// "launch_container": "window" in the JSON config.
EXPECT_EQ(WebAppProvider::Get(profile())->registrar().GetAppUserDisplayMode(
web_app_id),
DisplayMode::kBrowser);
}
}
} // namespace web_app } // namespace web_app
...@@ -17,6 +17,9 @@ TestWebAppUiManager::TestWebAppUiManager() = default; ...@@ -17,6 +17,9 @@ TestWebAppUiManager::TestWebAppUiManager() = default;
TestWebAppUiManager::~TestWebAppUiManager() = default; TestWebAppUiManager::~TestWebAppUiManager() = default;
void TestWebAppUiManager::SetSubsystems(
AppRegistryController* app_registry_controller) {}
void TestWebAppUiManager::Start() {} void TestWebAppUiManager::Start() {}
void TestWebAppUiManager::Shutdown() {} void TestWebAppUiManager::Shutdown() {}
......
...@@ -17,6 +17,7 @@ class TestWebAppUiManager : public WebAppUiManager { ...@@ -17,6 +17,7 @@ class TestWebAppUiManager : public WebAppUiManager {
TestWebAppUiManager(); TestWebAppUiManager();
~TestWebAppUiManager() override; ~TestWebAppUiManager() override;
void SetSubsystems(AppRegistryController* app_registry_controller) override;
void Start() override; void Start() override;
void Shutdown() override; void Shutdown() override;
......
...@@ -272,6 +272,7 @@ void WebAppProvider::ConnectSubsystems() { ...@@ -272,6 +272,7 @@ void WebAppProvider::ConnectSubsystems() {
web_app_policy_manager_->SetSubsystems(pending_app_manager_.get()); web_app_policy_manager_->SetSubsystems(pending_app_manager_.get());
file_handler_manager_->SetSubsystems(registrar_.get()); file_handler_manager_->SetSubsystems(registrar_.get());
shortcut_manager_->SetSubsystems(icon_manager_.get(), registrar_.get()); shortcut_manager_->SetSubsystems(icon_manager_.get(), registrar_.get());
ui_manager_->SetSubsystems(registry_controller_.get());
os_integration_manager_->SetSubsystems( os_integration_manager_->SetSubsystems(
registrar_.get(), shortcut_manager_.get(), file_handler_manager_.get(), registrar_.get(), shortcut_manager_.get(), file_handler_manager_.get(),
ui_manager_.get()); ui_manager_.get());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment