Commit 2ed086fc authored by Toni Barzic's avatar Toni Barzic Committed by Chromium LUCI CQ

Unblock default pinned apps roll if sync is disabled

When rolling default set of apps pinned to shelf, chrome_launcher_prefs
would check whether it's safe to do so using
IsSafeToApplyDefaultPinLayout(). One of conditions was verifying that
the app list sync has already started (to avoid getting default pinned
apps getting later overridden by an update from app list sync). This
does not work if sync is disabled for the profile, as in that case sync
would never start, so default apps roll would be delayed indefinitely.
For example, this may happen when logging in using a fake user (by
bypassing full login flow in test environments, or on linux-chromeos).

This updates the logic in IsSafeToApplyDefaultPinLayout() to allow
rolling default pinned apps if sync is disabled. This matches the logic
that's already in place for OS settings with split settings feature
enabled.

TEST=Run linux-chromeos with a fresh user data dir, and with no
--login-manager flag. Verify that both Chrome and Files app are pinned
to the shelf by default (both with SplitSettingsSync feature
enabled and disabled).

BUG=1166063,1154486

Change-Id: I8293b7a26447e5b5299b447984ce63417ead7653
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2640308Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845482}
parent 5e0a3298
...@@ -237,7 +237,8 @@ bool IsSafeToApplyDefaultPinLayout(Profile* profile) { ...@@ -237,7 +237,8 @@ bool IsSafeToApplyDefaultPinLayout(Profile* profile) {
return false; return false;
} }
} else { } else {
if (settings->GetSelectedTypes().Has(UserSelectableType::kApps) && if (sync_service->IsSyncFeatureEnabled() &&
settings->GetSelectedTypes().Has(UserSelectableType::kApps) &&
!app_list::AppListSyncableServiceFactory::GetForProfile(profile) !app_list::AppListSyncableServiceFactory::GetForProfile(profile)
->IsSyncing()) { ->IsSyncing()) {
return false; return false;
...@@ -254,7 +255,8 @@ bool IsSafeToApplyDefaultPinLayout(Profile* profile) { ...@@ -254,7 +255,8 @@ bool IsSafeToApplyDefaultPinLayout(Profile* profile) {
return false; return false;
} }
} else { } else {
if (settings->GetSelectedTypes().Has(UserSelectableType::kPreferences) && if (sync_service->IsSyncFeatureEnabled() &&
settings->GetSelectedTypes().Has(UserSelectableType::kPreferences) &&
!PrefServiceSyncableFromProfile(profile)->IsSyncing()) { !PrefServiceSyncableFromProfile(profile)->IsSyncing()) {
return false; return false;
} }
......
...@@ -115,6 +115,7 @@ ...@@ -115,6 +115,7 @@
#include "components/sync/base/model_type.h" #include "components/sync/base/model_type.h"
#include "components/sync/driver/sync_service.h" #include "components/sync/driver/sync_service.h"
#include "components/sync/driver/sync_user_settings.h" #include "components/sync/driver/sync_user_settings.h"
#include "components/sync/driver/test_sync_service.h"
#include "components/sync/protocol/sync.pb.h" #include "components/sync/protocol/sync.pb.h"
#include "components/sync/test/model/fake_sync_change_processor.h" #include "components/sync/test/model/fake_sync_change_processor.h"
#include "components/sync/test/model/sync_error_factory_mock.h" #include "components/sync/test/model/sync_error_factory_mock.h"
...@@ -169,6 +170,11 @@ constexpr char kCrxAppPrefix[] = "_crx_"; ...@@ -169,6 +170,11 @@ constexpr char kCrxAppPrefix[] = "_crx_";
// pin model with default apps that can affect some tests. // pin model with default apps that can affect some tests.
constexpr char kDummyAppId[] = "dummyappid_dummyappid_dummyappid"; constexpr char kDummyAppId[] = "dummyappid_dummyappid_dummyappid";
std::unique_ptr<KeyedService> BuildTestSyncService(
content::BrowserContext* context) {
return std::make_unique<syncer::TestSyncService>();
}
std::vector<arc::mojom::AppInfoPtr> GetArcSettingsAppInfo() { std::vector<arc::mojom::AppInfoPtr> GetArcSettingsAppInfo() {
std::vector<arc::mojom::AppInfoPtr> apps; std::vector<arc::mojom::AppInfoPtr> apps;
arc::mojom::AppInfoPtr app(arc::mojom::AppInfo::New()); arc::mojom::AppInfoPtr app(arc::mojom::AppInfo::New());
...@@ -367,6 +373,9 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest { ...@@ -367,6 +373,9 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {
extensions::manifest_keys::kPlatformAppBackgroundScripts, extensions::manifest_keys::kPlatformAppBackgroundScripts,
std::move(scripts)); std::move(scripts));
ProfileSyncServiceFactory::GetInstance()->SetTestingFactory(
profile(), base::BindRepeating(&BuildTestSyncService));
extensions::TestExtensionSystem* extension_system( extensions::TestExtensionSystem* extension_system(
static_cast<extensions::TestExtensionSystem*>( static_cast<extensions::TestExtensionSystem*>(
extensions::ExtensionSystem::Get(profile()))); extensions::ExtensionSystem::Get(profile())));
......
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