Commit f112bfc2 authored by Yuki Shiino's avatar Yuki Shiino Committed by Chromium LUCI CQ

Revert "Create standard icons on shelf for non-app icons."

This reverts commit 201f15f1.

Reason for revert: Suspicious about causing a test breakage of WindowOpenApiTest.PopupBlockingExtension.
https://ci.chromium.org/ui/p/chromium/builders/ci/linux-chromeos-dbg/22177/overview

Original change's description:
> Create standard icons on shelf for non-app icons.
>
> AppService creates standard circle icons for all apps. However, there
> are a few cases that icons on shelf are not app icons, e.g.:
> 1. Task manager
> 2. A few extensions show icons when open a new window, e.g. google
> hangouts.
>
> This CL create standard circle icons for them when they are shown on
> shelf.
>
> Modify TaskManagerView to create standard icon when running in Chrome
> OS.
>
> Modify ChromeLauncherController::OnAppImageUpdated. If the icon is not
> loaded by AppServiceAppIconLoader (which loads the standard circle
> icons), create standard icons.
>
> BUG=1162514
> BUG=1142373
>
> Change-Id: Ibfe40c33944a1120ca9c991c4953d0da906ee2e2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2624273
> Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#842874}

TBR=xiyuan@chromium.org,sky@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com,nancylingwang@chromium.org

Change-Id: I6e0dbef6a173a179d79e39cebbd4a4f78f15bcfb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1162514
Bug: 1142373
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2626821Reviewed-by: default avatarYuki Shiino <yukishiino@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842947}
parent f549d5be
...@@ -32,10 +32,20 @@ std::string GetAppId(Profile* profile, const std::string& id) { ...@@ -32,10 +32,20 @@ std::string GetAppId(Profile* profile, const std::string& id) {
} // namespace } // namespace
// static AppServiceAppIconLoader::AppServiceAppIconLoader(
bool AppServiceAppIconLoader::CanLoadImage(Profile* profile, Profile* profile,
const std::string& id) { int resource_size_in_dip,
const std::string app_id = GetAppId(profile, id); AppIconLoaderDelegate* delegate)
: AppIconLoader(profile, resource_size_in_dip, delegate) {
apps::AppServiceProxy* proxy =
apps::AppServiceProxyFactory::GetForProfile(profile);
Observe(&proxy->AppRegistryCache());
}
AppServiceAppIconLoader::~AppServiceAppIconLoader() = default;
bool AppServiceAppIconLoader::CanLoadImageForApp(const std::string& id) {
const std::string app_id = GetAppId(profile(), id);
// Skip the ARC intent helper, the system Android app that proxies links to // Skip the ARC intent helper, the system Android app that proxies links to
// Chrome, which should be hidden. // Chrome, which should be hidden.
...@@ -43,15 +53,13 @@ bool AppServiceAppIconLoader::CanLoadImage(Profile* profile, ...@@ -43,15 +53,13 @@ bool AppServiceAppIconLoader::CanLoadImage(Profile* profile,
return false; return false;
} }
if (!apps::AppServiceProxyFactory::IsAppServiceAvailableForProfile(profile)) { apps::AppServiceProxy* proxy =
return false; apps::AppServiceProxyFactory::GetForProfile(profile());
}
// Support icon loading for apps registered in AppService or Crostini apps // Support icon loading for apps registered in AppService or Crostini apps
// with the prefix "crostini:". // with the prefix "crostini:".
if (apps::AppServiceProxyFactory::GetForProfile(profile) if (proxy->AppRegistryCache().GetAppType(app_id) !=
->AppRegistryCache() apps::mojom::AppType::kUnknown ||
.GetAppType(app_id) != apps::mojom::AppType::kUnknown ||
crostini::IsUnmatchedCrostiniShelfAppId(app_id)) { crostini::IsUnmatchedCrostiniShelfAppId(app_id)) {
return true; return true;
} }
...@@ -59,22 +67,6 @@ bool AppServiceAppIconLoader::CanLoadImage(Profile* profile, ...@@ -59,22 +67,6 @@ bool AppServiceAppIconLoader::CanLoadImage(Profile* profile,
return false; return false;
} }
AppServiceAppIconLoader::AppServiceAppIconLoader(
Profile* profile,
int resource_size_in_dip,
AppIconLoaderDelegate* delegate)
: AppIconLoader(profile, resource_size_in_dip, delegate) {
apps::AppServiceProxy* proxy =
apps::AppServiceProxyFactory::GetForProfile(profile);
Observe(&proxy->AppRegistryCache());
}
AppServiceAppIconLoader::~AppServiceAppIconLoader() = default;
bool AppServiceAppIconLoader::CanLoadImageForApp(const std::string& id) {
return AppServiceAppIconLoader::CanLoadImage(profile(), id);
}
void AppServiceAppIconLoader::FetchImage(const std::string& id) { void AppServiceAppIconLoader::FetchImage(const std::string& id) {
const std::string app_id = GetAppId(profile(), id); const std::string app_id = GetAppId(profile(), id);
......
...@@ -22,8 +22,6 @@ class Profile; ...@@ -22,8 +22,6 @@ class Profile;
class AppServiceAppIconLoader : public AppIconLoader, class AppServiceAppIconLoader : public AppIconLoader,
private apps::AppRegistryCache::Observer { private apps::AppRegistryCache::Observer {
public: public:
static bool CanLoadImage(Profile* profile, const std::string& id);
AppServiceAppIconLoader(Profile* profile, AppServiceAppIconLoader(Profile* profile,
int resource_size_in_dip, int resource_size_in_dip,
AppIconLoaderDelegate* delegate); AppIconLoaderDelegate* delegate);
......
...@@ -39,7 +39,6 @@ ...@@ -39,7 +39,6 @@
#include "chrome/browser/ui/app_list/app_list_syncable_service_factory.h" #include "chrome/browser/ui/app_list/app_list_syncable_service_factory.h"
#include "chrome/browser/ui/app_list/app_service/app_service_app_icon_loader.h" #include "chrome/browser/ui/app_list/app_service/app_service_app_icon_loader.h"
#include "chrome/browser/ui/app_list/arc/arc_app_utils.h" #include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
#include "chrome/browser/ui/app_list/icon_standardizer.h"
#include "chrome/browser/ui/app_list/md_icon_normalizer.h" #include "chrome/browser/ui/app_list/md_icon_normalizer.h"
#include "chrome/browser/ui/apps/app_info_dialog.h" #include "chrome/browser/ui/apps/app_info_dialog.h"
#include "chrome/browser/ui/ash/chrome_launcher_prefs.h" #include "chrome/browser/ui/ash/chrome_launcher_prefs.h"
...@@ -929,10 +928,6 @@ void ChromeLauncherController::OnAppUninstalledPrepared( ...@@ -929,10 +928,6 @@ void ChromeLauncherController::OnAppUninstalledPrepared(
void ChromeLauncherController::OnAppImageUpdated(const std::string& app_id, void ChromeLauncherController::OnAppImageUpdated(const std::string& app_id,
const gfx::ImageSkia& image) { const gfx::ImageSkia& image) {
bool is_standard_icon = true;
if (!AppServiceAppIconLoader::CanLoadImage(latest_active_profile_, app_id))
is_standard_icon = false;
// TODO: need to get this working for shortcuts. // TODO: need to get this working for shortcuts.
for (int index = 0; index < model_->item_count(); ++index) { for (int index = 0; index < model_->item_count(); ++index) {
ash::ShelfItem item = model_->items()[index]; ash::ShelfItem item = model_->items()[index];
...@@ -941,8 +936,7 @@ void ChromeLauncherController::OnAppImageUpdated(const std::string& app_id, ...@@ -941,8 +936,7 @@ void ChromeLauncherController::OnAppImageUpdated(const std::string& app_id,
item.id.app_id != app_id) { item.id.app_id != app_id) {
continue; continue;
} }
item.image = item.image = image;
is_standard_icon ? image : app_list::CreateStandardIconImage(image);
shelf_spinner_controller_->MaybeApplySpinningEffect(app_id, &item.image); shelf_spinner_controller_->MaybeApplySpinningEffect(app_id, &item.image);
model_->Set(index, item); model_->Set(index, item);
// It's possible we're waiting on more than one item, so don't break. // It's possible we're waiting on more than one item, so don't break.
......
...@@ -38,7 +38,6 @@ ...@@ -38,7 +38,6 @@
#if BUILDFLAG(IS_CHROMEOS_ASH) #if BUILDFLAG(IS_CHROMEOS_ASH)
#include "ash/public/cpp/shelf_item.h" #include "ash/public/cpp/shelf_item.h"
#include "ash/public/cpp/window_properties.h" #include "ash/public/cpp/window_properties.h"
#include "chrome/browser/ui/app_list/icon_standardizer.h"
#include "chrome/grit/theme_resources.h" #include "chrome/grit/theme_resources.h"
#include "ui/aura/window.h" #include "ui/aura/window.h"
#include "ui/base/resource/resource_bundle.h" #include "ui/base/resource/resource_bundle.h"
...@@ -172,11 +171,8 @@ bool TaskManagerView::ExecuteWindowsCommand(int command_id) { ...@@ -172,11 +171,8 @@ bool TaskManagerView::ExecuteWindowsCommand(int command_id) {
gfx::ImageSkia TaskManagerView::GetWindowIcon() { gfx::ImageSkia TaskManagerView::GetWindowIcon() {
#if BUILDFLAG(IS_CHROMEOS_ASH) #if BUILDFLAG(IS_CHROMEOS_ASH)
// TODO(crbug.com/1162514): Move app_list::CreateStandardIconImage to some return *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
// where lower in the stack. IDR_ASH_SHELF_ICON_TASK_MANAGER);
return app_list::CreateStandardIconImage(
*ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
IDR_ASH_SHELF_ICON_TASK_MANAGER));
#else #else
return views::DialogDelegateView::GetWindowIcon(); return views::DialogDelegateView::GetWindowIcon();
#endif #endif
......
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