Skip to content

GitLab

T
tangled
Commit b8bcd254 authored by Ben Pastene's avatar Ben Pastene Committed by Commit Bot
Browse files
Options

Revert "CrOS: Add Files app to default shelf pins in second position" 

This reverts commit 5d6ecbfb.

Reason for revert: expected to be causing ui.ShelfLaunchedApps flakes
"Unexpected apps in the shelf. Expected only Chrome"
from:
https://ci.chromium.org/p/chromium/builders/ci/chromeos-amd64-generic-rel/47952
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8863730536929393584/+/steps/chrome_all_tast_tests/0/logs/Deterministic_failure:_ui.ShelfLaunchedApps__status_FAILURE_/0


Original change's description:
> CrOS: Add Files app to default shelf pins in second position
>
> This adds the Files app to the spot after the Chrome icon, for new users
> only. There's also some variations of the default pin layout controlled
> by a Finch experiment, I've moved the Files app up in those layouts for
> consistency.
>
> This change was requested for M88 and will be merged back.
>
> Bug: 1148519
> Change-Id: Iae54561fec599b813ba97e82a5678fa834a02db2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537214
> Reviewed-by: Dominick Ng <dominickn@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
> Auto-Submit: Tim Sergeant <tsergeant@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#827272}

TBR=xiyuan@chromium.org,tsergeant@chromium.org,dominickn@chromium.org

Change-Id: Ic86ed4ca44b70a2357395be9a44f5ace6997bd14
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1148519
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2538145Reviewed-by: default avatarBen Pastene <bpastene@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827448}
parent d6dad665
Hide whitespace changes
Inline Side-by-side
Showing with 22 additions and 35 deletions
chrome/browser/ui/ash/chrome_launcher_prefs.cc
View file @ b8bcd254
......@@ -48,28 +48,27 @@ namespace {
// Chrome is pinned explicitly.
const char* kDefaultPinnedApps[] = {
extension_misc::kFilesManagerAppId, extension_misc::kGmailAppId,
extension_misc::kGoogleDocAppId, extension_misc::kYoutubeAppId,
arc::kPlayStoreAppId};
extension_misc::kGmailAppId, extension_misc::kGoogleDocAppId,
extension_misc::kYoutubeAppId, arc::kPlayStoreAppId};
const char* kDefaultPinnedApps7Apps[] = {
extension_misc::kFilesManagerAppId, extension_misc::kGmailAppId,
extension_misc::kGoogleDocAppId, extension_misc::kGooglePhotosAppId,
extension_misc::kGmailAppId, extension_misc::kGoogleDocAppId,
extension_misc::kGooglePhotosAppId, extension_misc::kFilesManagerAppId,
extension_misc::kYoutubeAppId, arc::kPlayStoreAppId};
const char* kDefaultPinnedApps10Apps[] = {extension_misc::kFilesManagerAppId,
extension_misc::kGmailAppId,
const char* kDefaultPinnedApps10Apps[] = {extension_misc::kGmailAppId,
extension_misc::kCalendarAppId,
extension_misc::kGoogleDocAppId,
extension_misc::kGoogleSheetsAppId,
extension_misc::kGoogleSlidesAppId,
extension_misc::kFilesManagerAppId,
extension_misc::kCameraAppId,
extension_misc::kGooglePhotosAppId,
arc::kPlayStoreAppId};
const char* kTabletFormFactorDefaultPinnedApps[] = {
extension_misc::kFilesManagerAppId, arc::kGmailAppId,
extension_misc::kGoogleDocAppId, arc::kYoutubeAppId, arc::kPlayStoreAppId};
arc::kGmailAppId, extension_misc::kGoogleDocAppId, arc::kYoutubeAppId,
arc::kPlayStoreAppId};
const char kDefaultPinnedAppsKey[] = "default";
const char kDefaultPinnedApps7AppsKey[] = "7apps";
......
chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
View file @ b8bcd254
......@@ -417,11 +417,6 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {
base::FilePath(), Manifest::UNPACKED, manifest, Extension::NO_FLAGS,
extension_misc::kYoutubeAppId, &error);
// Fake File Manager app.
extension_files_app_ = Extension::Create(
base::FilePath(), Manifest::UNPACKED, manifest, Extension::NO_FLAGS,
extension_misc::kFilesManagerAppId, &error);
MaybeStartWebAppProvider();
}
......@@ -793,8 +788,6 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {
result += "Doc";
} else if (app == extension_youtube_app_->id()) {
result += "Youtube";
} else if (app == extension_files_app_->id()) {
result += "Files";
} else if (app == extension_platform_app_->id()) {
result += "Platform_App";
} else if (app == arc_support_host_->id()) {
......@@ -969,7 +962,6 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {
scoped_refptr<Extension> extension_gmail_app_;
scoped_refptr<Extension> extension_doc_app_;
scoped_refptr<Extension> extension_youtube_app_;
scoped_refptr<Extension> extension_files_app_;
scoped_refptr<Extension> extension_platform_app_;
scoped_refptr<Extension> arc_support_host_;
......@@ -1072,7 +1064,6 @@ class ChromeLauncherControllerExtendedShelfTest
extension_service_->AddExtension(extension_gmail_app_.get());
extension_service_->AddExtension(extension_doc_app_.get());
extension_service_->AddExtension(extension_youtube_app_.get());
extension_service_->AddExtension(extension_files_app_.get());
extension_service_->AddExtension(arc_support_host_.get());
std::string error;
......@@ -1090,6 +1081,7 @@ class ChromeLauncherControllerExtendedShelfTest
{extension_misc::kCalendarAppId, "Calendar"},
{extension_misc::kGoogleSheetsAppId, "Sheets"},
{extension_misc::kGoogleSlidesAppId, "Slides"},
{extension_misc::kFilesManagerAppId, "Files"},
{extension_misc::kCameraAppId, "Camera"},
{extension_misc::kGooglePhotosAppId, "Photos"},
};
......@@ -1351,8 +1343,6 @@ TEST_F(ChromeLauncherControllerTest, DefaultApps) {
EXPECT_EQ("Chrome, Doc, Youtube, App1", GetPinnedAppStatus());
AddExtension(extension_gmail_app_.get());
EXPECT_EQ("Chrome, Gmail, Doc, Youtube, App1", GetPinnedAppStatus());
AddExtension(extension_files_app_.get());
EXPECT_EQ("Chrome, Files, Gmail, Doc, Youtube, App1", GetPinnedAppStatus());
}
TEST_F(ChromeLauncherControllerSplitSettingsSyncTest, DefaultApps) {
......@@ -1393,29 +1383,28 @@ TEST_F(ChromeLauncherControllerLacrosTest, LacrosPinnedByDefault) {
EXPECT_EQ("Chrome, Lacros", GetPinnedAppStatus());
}
TEST_F(ChromeLauncherControllerExtendedShelfTest, ExtendedShelfDefault) {
TEST_F(ChromeLauncherControllerExtendedShelfTest, ExtendedShefDefault) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeatureWithParameters(
kEnableExtendedShelfLayout,
{std::pair<std::string, std::string>("app_count", "0")});
InitLauncherController();
EXPECT_EQ("Chrome, Files, Gmail, Doc, Youtube, Play Store",
GetPinnedAppStatus());
EXPECT_EQ("Chrome, Gmail, Doc, Youtube, Play Store", GetPinnedAppStatus());
}
TEST_F(ChromeLauncherControllerExtendedShelfTest, ExtendedShelf7Apps) {
TEST_F(ChromeLauncherControllerExtendedShelfTest, ExtendedShef7Apps) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeatureWithParameters(
kEnableExtendedShelfLayout,
{std::pair<std::string, std::string>("app_count", "7")});
InitLauncherController();
EXPECT_EQ("Chrome, Files, Gmail, Doc, Photos, Youtube, Play Store",
EXPECT_EQ("Chrome, Gmail, Doc, Photos, Files, Youtube, Play Store",
GetPinnedAppStatus());
}
TEST_F(ChromeLauncherControllerExtendedShelfTest, ExtendedShelf10Apps) {
TEST_F(ChromeLauncherControllerExtendedShelfTest, ExtendedShef10Apps) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeatureWithParameters(
kEnableExtendedShelfLayout,
......@@ -1423,15 +1412,14 @@ TEST_F(ChromeLauncherControllerExtendedShelfTest, ExtendedShelf10Apps) {
InitLauncherController();
EXPECT_EQ(
"Chrome, Files, Gmail, Calendar, Doc, Sheets, Slides, Camera, Photos, "
"Chrome, Gmail, Calendar, Doc, Sheets, Slides, Files, Camera, Photos, "
"Play Store",
GetPinnedAppStatus());
}
TEST_F(ChromeLauncherControllerExtendedShelfTest, UpgradeFromDefault) {
InitLauncherController();
EXPECT_EQ("Chrome, Files, Gmail, Doc, Youtube, Play Store",
GetPinnedAppStatus());
EXPECT_EQ("Chrome, Gmail, Doc, Youtube, Play Store", GetPinnedAppStatus());
// Upgrade happens only in case default layout is active.
base::test::ScopedFeatureList scoped_feature_list;
......@@ -1443,15 +1431,15 @@ TEST_F(ChromeLauncherControllerExtendedShelfTest, UpgradeFromDefault) {
AddExtension(extension1_.get());
EXPECT_EQ(
"Chrome, Files, Gmail, Calendar, Doc, Sheets, Slides, Camera, Photos, "
"Chrome, Gmail, Calendar, Doc, Sheets, Slides, Files, Camera, Photos, "
"Play Store",
GetPinnedAppStatus());
}
TEST_F(ChromeLauncherControllerExtendedShelfTest, NoDefaultAfterExperimental) {
TEST_F(ChromeLauncherControllerExtendedShelfTest, NoDefaultAfterExperemental) {
const std::string expectations =
"Chrome, Files, Gmail, Calendar, Doc, Sheets, "
"Slides, Camera, Photos, Play Store";
"Chrome, Gmail, Calendar, Doc, Sheets, "
"Slides, Files, Camera, Photos, Play Store";
{
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeatureWithParameters(
......@@ -1478,7 +1466,7 @@ TEST_F(ChromeLauncherControllerExtendedShelfTest, NoDefaultAfterExperimental) {
TEST_F(ChromeLauncherControllerExtendedShelfTest, NoUpgradeFromNonDefault) {
InitLauncherController();
launcher_controller_->UnpinAppWithID(extension_misc::kYoutubeAppId);
EXPECT_EQ("Chrome, Files, Gmail, Doc, Play Store", GetPinnedAppStatus());
EXPECT_EQ("Chrome, Gmail, Doc, Play Store", GetPinnedAppStatus());
// Upgrade does not happen due to default pin layout change.
base::test::ScopedFeatureList scoped_feature_list;
......@@ -1489,7 +1477,7 @@ TEST_F(ChromeLauncherControllerExtendedShelfTest, NoUpgradeFromNonDefault) {
// Trigger layout update, app_id does not matter.
AddExtension(extension1_.get());
EXPECT_EQ("Chrome, Files, Gmail, Doc, Play Store", GetPinnedAppStatus());
EXPECT_EQ("Chrome, Gmail, Doc, Play Store", GetPinnedAppStatus());
}
TEST_F(ChromeLauncherControllerWithArcTest, ArcAppPinCrossPlatformWorkflow) {
......
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