Commit db29fe04 authored by Sheng-Hao Tsao's avatar Sheng-Hao Tsao Committed by Commit Bot

Reland: Don't show camera icon in guest mode

This CL relands "Don't show camera icon in guest mode", which was
reverted in
https://chromium-review.googlesource.com/c/chromium/src/+/1165962

Original description:

This CL hides camera icon of internal camera app in guest mode. Since
extensions are not allowed in guest mode, users can't perform any
actions after clicking on the camera icon.

TBR=stevenjb@chromium.org,melandory@chromium.org

Bug: 866412
Test: Tested on eve that there's no camera icon in guest mode.
Change-Id: Ia033c0899c5d23005c1c4779d20aff72f9a91501
Reviewed-on: https://chromium-review.googlesource.com/1167147
Commit-Queue: Sheng-hao Tsao <shenghao@google.com>
Reviewed-by: default avatarRicky Liang <jcliang@chromium.org>
Reviewed-by: default avatarTatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582096}
parent 4d034df1
...@@ -127,7 +127,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientAppListSyncTest, AppListSomeApps) { ...@@ -127,7 +127,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientAppListSyncTest, AppListSomeApps) {
// Default apps: chrome + web store + internal apps. // Default apps: chrome + web store + internal apps.
const size_t kNumDefaultApps = const size_t kNumDefaultApps =
2u + app_list::GetNumberOfInternalAppsShowInLauncherForTest( 2u + app_list::GetNumberOfInternalAppsShowInLauncherForTest(
/*apps_name=*/nullptr); /*apps_name=*/nullptr, GetProfile(0)->IsGuestSession());
ASSERT_EQ(kNumApps + kNumDefaultApps, service->GetNumSyncItemsForTest()); ASSERT_EQ(kNumApps + kNumDefaultApps, service->GetNumSyncItemsForTest());
ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait()); ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
......
...@@ -708,7 +708,8 @@ TEST_F(AppContextMenuTest, CommandIdsMatchEnumsForHistograms) { ...@@ -708,7 +708,8 @@ TEST_F(AppContextMenuTest, CommandIdsMatchEnumsForHistograms) {
// Tests that internal app's context menu is correct. // Tests that internal app's context menu is correct.
TEST_P(AppContextMenuTest, InternalAppMenu) { TEST_P(AppContextMenuTest, InternalAppMenu) {
for (const auto& internal_app : app_list::GetInternalAppList()) { for (const auto& internal_app :
app_list::GetInternalAppList(profile()->IsGuestSession())) {
if (!internal_app.show_in_launcher) if (!internal_app.show_in_launcher)
continue; continue;
......
...@@ -49,43 +49,51 @@ constexpr char kAndroidCameraAppId[] = "goamfaniemdfcajgcmmflhchgkmbngka"; ...@@ -49,43 +49,51 @@ constexpr char kAndroidCameraAppId[] = "goamfaniemdfcajgcmmflhchgkmbngka";
constexpr char kAndroidLegacyCameraAppId[] = "obfofkigjfamlldmipdegnjlcpincibc"; constexpr char kAndroidLegacyCameraAppId[] = "obfofkigjfamlldmipdegnjlcpincibc";
} // namespace } // namespace
const std::vector<InternalApp>& GetInternalAppList() { const std::vector<InternalApp>& GetInternalAppList(bool is_guest_mode) {
static const base::NoDestructor<std::vector<InternalApp>> internal_app_list( static const base::NoDestructor<std::vector<InternalApp>>
{{kInternalAppIdKeyboardShortcutViewer, internal_app_list_static(
IDS_INTERNAL_APP_KEYBOARD_SHORTCUT_VIEWER, IDR_SHORTCUT_VIEWER_LOGO_192, {{kInternalAppIdKeyboardShortcutViewer,
/*recommendable=*/false, IDS_INTERNAL_APP_KEYBOARD_SHORTCUT_VIEWER,
/*searchable=*/true, IDR_SHORTCUT_VIEWER_LOGO_192,
/*show_in_launcher=*/false, /*recommendable=*/false,
InternalAppName::kKeyboardShortcutViewer, /*searchable=*/true,
IDS_LAUNCHER_SEARCHABLE_KEYBOARD_SHORTCUT_VIEWER}, /*show_in_launcher=*/false,
InternalAppName::kKeyboardShortcutViewer,
{kInternalAppIdSettings, IDS_INTERNAL_APP_SETTINGS, IDS_LAUNCHER_SEARCHABLE_KEYBOARD_SHORTCUT_VIEWER},
IDR_SETTINGS_LOGO_192,
/*recommendable=*/true, {kInternalAppIdSettings, IDS_INTERNAL_APP_SETTINGS,
/*searchable=*/true, IDR_SETTINGS_LOGO_192,
/*show_in_launcher=*/true, /*recommendable=*/true,
InternalAppName::kSettings, /*searchable=*/true,
/*searchable_string_resource_id=*/0}, /*show_in_launcher=*/true, InternalAppName::kSettings,
/*searchable_string_resource_id=*/0},
{kInternalAppIdContinueReading, IDS_INTERNAL_APP_CONTINUOUS_READING,
IDR_PRODUCT_LOGO_256, {kInternalAppIdContinueReading, IDS_INTERNAL_APP_CONTINUOUS_READING,
/*recommendable=*/true, IDR_PRODUCT_LOGO_256,
/*searchable=*/false, /*recommendable=*/true,
/*show_in_launcher=*/false, /*searchable=*/false,
InternalAppName::kContinueReading, /*show_in_launcher=*/false, InternalAppName::kContinueReading,
/*searchable_string_resource_id=*/0}, /*searchable_string_resource_id=*/0}});
{kInternalAppIdCamera, IDS_INTERNAL_APP_CAMERA, IDR_CAMERA_LOGO_192, static base::NoDestructor<std::vector<InternalApp>> internal_app_list;
/*recommendable=*/true, internal_app_list->clear();
/*searchable=*/true, internal_app_list->insert(internal_app_list->begin(),
/*show_in_launcher=*/true, internal_app_list_static->begin(),
InternalAppName::kCamera, internal_app_list_static->end());
/*searchable_string_resource_id=*/0}});
if (!is_guest_mode) {
internal_app_list->push_back(
{kInternalAppIdCamera, IDS_INTERNAL_APP_CAMERA, IDR_CAMERA_LOGO_192,
/*recommendable=*/true,
/*searchable=*/true,
/*show_in_launcher=*/true, InternalAppName::kCamera,
/*searchable_string_resource_id=*/0});
}
return *internal_app_list; return *internal_app_list;
} }
const InternalApp* FindInternalApp(const std::string& app_id) { const InternalApp* FindInternalApp(const std::string& app_id) {
for (const auto& app : GetInternalAppList()) { for (const auto& app : GetInternalAppList(false)) {
if (app_id == app.app_id) if (app_id == app.app_id)
return &app; return &app;
} }
...@@ -235,10 +243,11 @@ InternalAppName GetInternalAppNameByAppId( ...@@ -235,10 +243,11 @@ InternalAppName GetInternalAppNameByAppId(
return app->internal_app_name; return app->internal_app_name;
} }
size_t GetNumberOfInternalAppsShowInLauncherForTest(std::string* apps_name) { size_t GetNumberOfInternalAppsShowInLauncherForTest(std::string* apps_name,
bool is_guest_mode) {
size_t num_of_internal_apps_show_in_launcher = 0u; size_t num_of_internal_apps_show_in_launcher = 0u;
std::vector<std::string> internal_apps_name; std::vector<std::string> internal_apps_name;
for (const auto& app : GetInternalAppList()) { for (const auto& app : GetInternalAppList(is_guest_mode)) {
if (app.show_in_launcher) { if (app.show_in_launcher) {
++num_of_internal_apps_show_in_launcher; ++num_of_internal_apps_show_in_launcher;
if (apps_name) { if (apps_name) {
......
...@@ -52,7 +52,7 @@ struct InternalApp { ...@@ -52,7 +52,7 @@ struct InternalApp {
}; };
// Returns a list of Chrome OS internal apps, which are searchable in launcher. // Returns a list of Chrome OS internal apps, which are searchable in launcher.
const std::vector<InternalApp>& GetInternalAppList(); const std::vector<InternalApp>& GetInternalAppList(bool is_guest_mode);
// Returns InternalApp by |app_id|. // Returns InternalApp by |app_id|.
// Returns nullptr if |app_id| does not correspond to an internal app. // Returns nullptr if |app_id| does not correspond to an internal app.
...@@ -95,7 +95,8 @@ InternalAppName GetInternalAppNameByAppId( ...@@ -95,7 +95,8 @@ InternalAppName GetInternalAppNameByAppId(
// Returns the number of internal apps which can show in launcher. // Returns the number of internal apps which can show in launcher.
// If |apps_name| is not nullptr, it will be the concatenated string of these // If |apps_name| is not nullptr, it will be the concatenated string of these
// internal apps' name. // internal apps' name.
size_t GetNumberOfInternalAppsShowInLauncherForTest(std::string* apps_name); size_t GetNumberOfInternalAppsShowInLauncherForTest(std::string* apps_name,
bool is_guest_mode);
} // namespace app_list } // namespace app_list
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "chrome/browser/ui/app_list/internal_app/internal_app_model_builder.h" #include "chrome/browser/ui/app_list/internal_app/internal_app_model_builder.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/app_list/app_list_controller_delegate.h" #include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
#include "chrome/browser/ui/app_list/internal_app/internal_app_item.h" #include "chrome/browser/ui/app_list/internal_app/internal_app_item.h"
#include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h" #include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
...@@ -13,7 +14,8 @@ InternalAppModelBuilder::InternalAppModelBuilder( ...@@ -13,7 +14,8 @@ InternalAppModelBuilder::InternalAppModelBuilder(
: AppListModelBuilder(controller, InternalAppItem::kItemType) {} : AppListModelBuilder(controller, InternalAppItem::kItemType) {}
void InternalAppModelBuilder::BuildModel() { void InternalAppModelBuilder::BuildModel() {
for (const auto& internal_app : app_list::GetInternalAppList()) { for (const auto& internal_app :
app_list::GetInternalAppList(profile()->IsGuestSession())) {
if (!internal_app.show_in_launcher) if (!internal_app.show_in_launcher)
continue; continue;
......
...@@ -6,11 +6,13 @@ ...@@ -6,11 +6,13 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/app_list/app_list_test_util.h" #include "chrome/browser/ui/app_list/app_list_test_util.h"
#include "chrome/browser/ui/app_list/chrome_app_list_item.h" #include "chrome/browser/ui/app_list/chrome_app_list_item.h"
#include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h" #include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
#include "chrome/browser/ui/app_list/test/fake_app_list_model_updater.h" #include "chrome/browser/ui/app_list/test/fake_app_list_model_updater.h"
#include "chrome/browser/ui/app_list/test/test_app_list_controller_delegate.h" #include "chrome/browser/ui/app_list/test/test_app_list_controller_delegate.h"
#include "chrome/test/base/testing_profile.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace { namespace {
...@@ -33,7 +35,6 @@ class InternalAppModelBuilderTest : public AppListTestBase { ...@@ -33,7 +35,6 @@ class InternalAppModelBuilderTest : public AppListTestBase {
// AppListTestBase: // AppListTestBase:
void SetUp() override { void SetUp() override {
AppListTestBase::SetUp(); AppListTestBase::SetUp();
CreateBuilder();
} }
void TearDown() override { void TearDown() override {
...@@ -42,19 +43,20 @@ class InternalAppModelBuilderTest : public AppListTestBase { ...@@ -42,19 +43,20 @@ class InternalAppModelBuilderTest : public AppListTestBase {
} }
protected: protected:
std::unique_ptr<FakeAppListModelUpdater> model_updater_;
private:
// Creates a new builder, destroying any existing one. // Creates a new builder, destroying any existing one.
void CreateBuilder() { void CreateBuilder(bool guest_mode) {
ResetBuilder(); // Destroy any existing builder in the correct order. ResetBuilder(); // Destroy any existing builder in the correct order.
testing_profile()->SetGuestSession(guest_mode);
model_updater_ = std::make_unique<FakeAppListModelUpdater>(); model_updater_ = std::make_unique<FakeAppListModelUpdater>();
controller_ = std::make_unique<test::TestAppListControllerDelegate>(); controller_ = std::make_unique<test::TestAppListControllerDelegate>();
builder_ = std::make_unique<InternalAppModelBuilder>(controller_.get()); builder_ = std::make_unique<InternalAppModelBuilder>(controller_.get());
builder_->Initialize(nullptr, profile(), model_updater_.get()); builder_->Initialize(nullptr, profile(), model_updater_.get());
} }
std::unique_ptr<FakeAppListModelUpdater> model_updater_;
private:
void ResetBuilder() { void ResetBuilder() {
builder_.reset(); builder_.reset();
controller_.reset(); controller_.reset();
...@@ -71,8 +73,20 @@ TEST_F(InternalAppModelBuilderTest, Build) { ...@@ -71,8 +73,20 @@ TEST_F(InternalAppModelBuilderTest, Build) {
// The internal apps list is provided by GetInternalAppList() in // The internal apps list is provided by GetInternalAppList() in
// internal_app_metadata.cc. Only count the apps can display in launcher. // internal_app_metadata.cc. Only count the apps can display in launcher.
std::string internal_apps_name; std::string internal_apps_name;
CreateBuilder(false);
EXPECT_EQ(app_list::GetNumberOfInternalAppsShowInLauncherForTest(
&internal_apps_name, profile()->IsGuestSession()),
model_updater_->ItemCount());
EXPECT_EQ(internal_apps_name, GetModelContent(model_updater_.get()));
}
TEST_F(InternalAppModelBuilderTest, BuildGuestMode) {
// The internal apps list is provided by GetInternalAppList() in
// internal_app_metadata.cc. Only count the apps can display in launcher.
std::string internal_apps_name;
CreateBuilder(true);
EXPECT_EQ(app_list::GetNumberOfInternalAppsShowInLauncherForTest( EXPECT_EQ(app_list::GetNumberOfInternalAppsShowInLauncherForTest(
&internal_apps_name), &internal_apps_name, profile()->IsGuestSession()),
model_updater_->ItemCount()); model_updater_->ItemCount());
EXPECT_EQ(internal_apps_name, GetModelContent(model_updater_.get())); EXPECT_EQ(internal_apps_name, GetModelContent(model_updater_.get()));
} }
...@@ -371,7 +371,8 @@ class InternalDataSource : public AppSearchProvider::DataSource, ...@@ -371,7 +371,8 @@ class InternalDataSource : public AppSearchProvider::DataSource,
// AppSearchProvider::DataSource overrides: // AppSearchProvider::DataSource overrides:
void AddApps(AppSearchProvider::Apps* apps) override { void AddApps(AppSearchProvider::Apps* apps) override {
const base::Time time; const base::Time time;
for (const auto& internal_app : GetInternalAppList()) { for (const auto& internal_app :
GetInternalAppList(profile()->IsGuestSession())) {
if (!std::strcmp(internal_app.app_id, kInternalAppIdContinueReading) && if (!std::strcmp(internal_app.app_id, kInternalAppIdContinueReading) &&
(!features::IsContinueReadingEnabled() || (!features::IsContinueReadingEnabled() ||
!profile()->GetPrefs()->GetBoolean( !profile()->GetPrefs()->GetBoolean(
......
...@@ -4306,7 +4306,8 @@ TEST_F(ChromeLauncherControllerTest, InternalAppWindowRecreation) { ...@@ -4306,7 +4306,8 @@ TEST_F(ChromeLauncherControllerTest, InternalAppWindowRecreation) {
InitLauncherController(); InitLauncherController();
// Only test the first internal app. The others should be the same. // Only test the first internal app. The others should be the same.
const auto& internal_app = app_list::GetInternalAppList().front(); const auto& internal_app =
app_list::GetInternalAppList(profile()->IsGuestSession()).front();
const std::string app_id = internal_app.app_id; const std::string app_id = internal_app.app_id;
const ash::ShelfID shelf_id(app_id); const ash::ShelfID shelf_id(app_id);
EXPECT_FALSE(launcher_controller_->GetItem(shelf_id)); EXPECT_FALSE(launcher_controller_->GetItem(shelf_id));
...@@ -4337,7 +4338,8 @@ TEST_F(ChromeLauncherControllerTest, InternalAppWindowPropertyChanged) { ...@@ -4337,7 +4338,8 @@ TEST_F(ChromeLauncherControllerTest, InternalAppWindowPropertyChanged) {
InitLauncherController(); InitLauncherController();
// Only test the first internal app. The others should be the same. // Only test the first internal app. The others should be the same.
const auto& internal_app = app_list::GetInternalAppList().front(); const auto& internal_app =
app_list::GetInternalAppList(profile()->IsGuestSession()).front();
std::string app_id; std::string app_id;
ash::ShelfID shelf_id; ash::ShelfID shelf_id;
EXPECT_FALSE(launcher_controller_->GetItem(shelf_id)); EXPECT_FALSE(launcher_controller_->GetItem(shelf_id));
......
...@@ -462,7 +462,8 @@ TEST_F(LauncherContextMenuTest, ArcContextMenuOptions) { ...@@ -462,7 +462,8 @@ TEST_F(LauncherContextMenuTest, ArcContextMenuOptions) {
// Tests that the context menu of internal app is correct. // Tests that the context menu of internal app is correct.
TEST_F(LauncherContextMenuTest, InternalAppShelfContextMenu) { TEST_F(LauncherContextMenuTest, InternalAppShelfContextMenu) {
for (const auto& internal_app : app_list::GetInternalAppList()) { for (const auto& internal_app :
app_list::GetInternalAppList(profile()->IsGuestSession())) {
if (!internal_app.show_in_launcher) if (!internal_app.show_in_launcher)
continue; continue;
...@@ -492,7 +493,8 @@ TEST_F(LauncherContextMenuTest, InternalAppShelfContextMenu) { ...@@ -492,7 +493,8 @@ TEST_F(LauncherContextMenuTest, InternalAppShelfContextMenu) {
// Tests that the number of context menu options of internal app is correct. // Tests that the number of context menu options of internal app is correct.
TEST_F(LauncherContextMenuTest, InternalAppShelfContextMenuOptionsNumber) { TEST_F(LauncherContextMenuTest, InternalAppShelfContextMenuOptionsNumber) {
for (const auto& internal_app : app_list::GetInternalAppList()) { for (const auto& internal_app :
app_list::GetInternalAppList(profile()->IsGuestSession())) {
const std::string app_id = internal_app.app_id; const std::string app_id = internal_app.app_id;
const ash::ShelfID shelf_id(app_id); const ash::ShelfID shelf_id(app_id);
// Pin internal app. // Pin internal app.
......
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