Commit 35e8309c authored by Nancy Wang's avatar Nancy Wang Committed by Chromium LUCI CQ

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

This reverts commit f112bfc2.

Reason for revert: <INSERT REASONING HERE>
Fix the browser tests issue.

Original change's description:
> 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/+/2626821
> Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#842947}

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


Bug: 1162514
Bug: 1142373
Change-Id: If55904732c4fd468db2a5f56cece04fc8a937a0d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2626943Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Reviewed-by: default avatarNancy Wang <nancylingwang@chromium.org>
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843283}
parent ed185930
...@@ -85,7 +85,7 @@ bool WaitForTabsPopupsApps(Browser* browser, ...@@ -85,7 +85,7 @@ bool WaitForTabsPopupsApps(Browser* browser,
browser->tab_strip_model()->count() == num_tabs) browser->tab_strip_model()->count() == num_tabs)
break; break;
content::RunAllPendingInMessageLoop(); content::RunAllTasksUntilIdle();
} }
EXPECT_EQ(num_browsers, chrome::GetBrowserCount(browser->profile())); EXPECT_EQ(num_browsers, chrome::GetBrowserCount(browser->profile()));
......
...@@ -32,20 +32,10 @@ std::string GetAppId(Profile* profile, const std::string& id) { ...@@ -32,20 +32,10 @@ std::string GetAppId(Profile* profile, const std::string& id) {
} // namespace } // namespace
AppServiceAppIconLoader::AppServiceAppIconLoader( // static
Profile* profile, bool AppServiceAppIconLoader::CanLoadImage(Profile* profile,
int resource_size_in_dip, const std::string& id) {
AppIconLoaderDelegate* delegate) const std::string app_id = GetAppId(profile, id);
: 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.
...@@ -53,13 +43,15 @@ bool AppServiceAppIconLoader::CanLoadImageForApp(const std::string& id) { ...@@ -53,13 +43,15 @@ bool AppServiceAppIconLoader::CanLoadImageForApp(const std::string& id) {
return false; return false;
} }
apps::AppServiceProxy* proxy = if (!apps::AppServiceProxyFactory::IsAppServiceAvailableForProfile(profile)) {
apps::AppServiceProxyFactory::GetForProfile(profile()); return false;
}
// 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 (proxy->AppRegistryCache().GetAppType(app_id) != if (apps::AppServiceProxyFactory::GetForProfile(profile)
apps::mojom::AppType::kUnknown || ->AppRegistryCache()
.GetAppType(app_id) != apps::mojom::AppType::kUnknown ||
crostini::IsUnmatchedCrostiniShelfAppId(app_id)) { crostini::IsUnmatchedCrostiniShelfAppId(app_id)) {
return true; return true;
} }
...@@ -67,6 +59,22 @@ bool AppServiceAppIconLoader::CanLoadImageForApp(const std::string& id) { ...@@ -67,6 +59,22 @@ bool AppServiceAppIconLoader::CanLoadImageForApp(const std::string& id) {
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,6 +22,8 @@ class Profile; ...@@ -22,6 +22,8 @@ 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,6 +39,7 @@ ...@@ -39,6 +39,7 @@
#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"
...@@ -928,6 +929,10 @@ void ChromeLauncherController::OnAppUninstalledPrepared( ...@@ -928,6 +929,10 @@ 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];
...@@ -936,7 +941,8 @@ void ChromeLauncherController::OnAppImageUpdated(const std::string& app_id, ...@@ -936,7 +941,8 @@ void ChromeLauncherController::OnAppImageUpdated(const std::string& app_id,
item.id.app_id != app_id) { item.id.app_id != app_id) {
continue; continue;
} }
item.image = image; item.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,6 +38,7 @@ ...@@ -38,6 +38,7 @@
#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"
...@@ -171,8 +172,11 @@ bool TaskManagerView::ExecuteWindowsCommand(int command_id) { ...@@ -171,8 +172,11 @@ bool TaskManagerView::ExecuteWindowsCommand(int command_id) {
gfx::ImageSkia TaskManagerView::GetWindowIcon() { gfx::ImageSkia TaskManagerView::GetWindowIcon() {
#if BUILDFLAG(IS_CHROMEOS_ASH) #if BUILDFLAG(IS_CHROMEOS_ASH)
return *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed( // TODO(crbug.com/1162514): Move app_list::CreateStandardIconImage to some
IDR_ASH_SHELF_ICON_TASK_MANAGER); // where lower in the stack.
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