[Sharesheet] Add app icons.

This CL adds app icons to the Sharesheet. It also removes the default
margin in ShareActionView.

Bug: 1097623
Change-Id: Ia9f58543d89863b97472fbbb11c6d499cad2ee25
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2331759Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Melissa Zhang <melzhang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794333}

chrome/browser/sharesheet/share_action.h

@@ -8,7 +8,7 @@
 #include "base/strings/string16.h"
 #include "chrome/browser/sharesheet/sharesheet_controller.h"
 #include "components/services/app_service/public/mojom/types.mojom.h"
-#include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_skia.h"
 #include "ui/views/view.h"
 
 namespace sharesheet {
@@ -20,7 +20,7 @@ class ShareAction {
 
   virtual const base::string16 GetActionName() = 0;
 
-  virtual const gfx::Image GetActionIcon() = 0;
+  virtual const gfx::ImageSkia GetActionIcon() = 0;
 
   // LaunchAction should synchronously create all UI needed and fill
   // the |root_view|. Methods on |controller| can be used to inform

chrome/browser/sharesheet/sharesheet_service.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/bind.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
@@ -14,10 +15,16 @@
 #include "chrome/browser/sharesheet/share_action.h"
 #include "chrome/browser/sharesheet/sharesheet_service_delegate.h"
 #include "chrome/browser/sharesheet/sharesheet_types.h"
+#include "chrome/common/chrome_features.h"
 #include "components/services/app_service/public/cpp/intent_util.h"
 #include "ui/display/types/display_constants.h"
 #include "ui/views/view.h"
 
+namespace {
+// In px.
+constexpr int kIconSize = 40;
+}  // namespace
+
 namespace sharesheet {
 
 SharesheetService::SharesheetService(Profile* profile)
@@ -48,17 +55,11 @@ void SharesheetService::ShowBubble(views::View* bubble_anchor_view,
 
   std::vector<apps::AppIdAndActivityName> app_id_and_activities =
       app_service_proxy_->GetAppsForIntent(intent);
-  for (const auto& app_id_and_activity : app_id_and_activities) {
-    // TODO(1097623) : Load Real Icons.
-    targets.emplace(targets.begin(), TargetType::kApp, gfx::Image(),
-                    base::UTF8ToUTF16(app_id_and_activity.app_id),
-                    base::UTF8ToUTF16(app_id_and_activity.activity_name));
-  }
-
-  sharesheet_service_delegate->ShowBubble(std::move(targets),
-                                          std::move(intent));
-
-  active_delegates_.push_back(std::move(sharesheet_service_delegate));
+  LoadAppIcons(std::move(app_id_and_activities), std::move(targets), 0,
+               base::BindOnce(&SharesheetService::OnAppIconsLoaded,
+                              weak_factory_.GetWeakPtr(),
+                              std::move(sharesheet_service_delegate),
+                              std::move(intent)));
 }
 
 // Cleanup delegate when bubble closes.
@@ -128,4 +129,54 @@ bool SharesheetService::HasShareTargets(apps::mojom::IntentPtr intent) {
   return !actions.empty() || !app_id_and_activities.empty();
 }
 
+void SharesheetService::LoadAppIcons(
+    std::vector<apps::AppIdAndActivityName> app_id_and_activities,
+    std::vector<TargetInfo> targets,
+    size_t index,
+    base::OnceCallback<void(std::vector<TargetInfo> targets)> callback) {
+  if (index >= app_id_and_activities.size()) {
+    std::move(callback).Run(std::move(targets));
+    return;
+  }
+
+  // Making a copy because we move |app_id_and_activities| out below.
+  auto app_id = app_id_and_activities[index].app_id;
+  auto app_type = app_service_proxy_->AppRegistryCache().GetAppType(app_id);
+  auto icon_type =
+      (base::FeatureList::IsEnabled(features::kAppServiceAdaptiveIcon))
+          ? apps::mojom::IconType::kStandard
+          : apps::mojom::IconType::kUncompressed;
+  constexpr bool allow_placeholder_icon = false;
+  app_service_proxy_->LoadIcon(
+      app_type, app_id, icon_type, kIconSize, allow_placeholder_icon,
+      base::BindOnce(&SharesheetService::OnIconLoaded,
+                     weak_factory_.GetWeakPtr(),
+                     std::move(app_id_and_activities), std::move(targets),
+                     index, std::move(callback)));
+}
+
+void SharesheetService::OnIconLoaded(
+    std::vector<apps::AppIdAndActivityName> app_id_and_activities,
+    std::vector<TargetInfo> targets,
+    size_t index,
+    base::OnceCallback<void(std::vector<TargetInfo> targets)> callback,
+    apps::mojom::IconValuePtr icon_value) {
+  const auto& app_id_and_activity = app_id_and_activities[index];
+  targets.emplace(targets.begin(), TargetType::kApp, icon_value->uncompressed,
+                  base::UTF8ToUTF16(app_id_and_activity.app_id),
+                  base::UTF8ToUTF16(app_id_and_activity.activity_name));
+
+  LoadAppIcons(std::move(app_id_and_activities), std::move(targets), index + 1,
+               std::move(callback));
+}
+
+void SharesheetService::OnAppIconsLoaded(
+    std::unique_ptr<SharesheetServiceDelegate> delegate,
+    apps::mojom::IntentPtr intent,
+    std::vector<TargetInfo> targets) {
+  delegate->ShowBubble(std::move(targets), std::move(intent));
+
+  active_delegates_.push_back(std::move(delegate));
+}
+
 }  // namespace sharesheet

chrome/browser/sharesheet/sharesheet_service.h

@@ -8,6 +8,8 @@
 #include <memory>
 #include <vector>
 
+#include "base/callback.h"
+#include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/sharesheet/sharesheet_action_cache.h"
 #include "chrome/browser/sharesheet/sharesheet_types.h"
@@ -17,6 +19,7 @@
 class Profile;
 
 namespace apps {
+struct AppIdAndActivityName;
 class AppServiceProxy;
 }
 
@@ -50,6 +53,26 @@ class SharesheetService : public KeyedService {
   bool HasShareTargets(apps::mojom::IntentPtr intent);
 
  private:
+  using SharesheetServiceIconLoaderCallback =
+      base::OnceCallback<void(std::vector<TargetInfo> targets)>;
+
+  void LoadAppIcons(
+      std::vector<apps::AppIdAndActivityName> app_id_and_activities,
+      std::vector<TargetInfo> targets,
+      size_t index,
+      SharesheetServiceIconLoaderCallback callback);
+
+  void OnIconLoaded(
+      std::vector<apps::AppIdAndActivityName> app_id_and_activities,
+      std::vector<TargetInfo> targets,
+      size_t index,
+      SharesheetServiceIconLoaderCallback callback,
+      apps::mojom::IconValuePtr icon_value);
+
+  void OnAppIconsLoaded(std::unique_ptr<SharesheetServiceDelegate> delegate,
+                        apps::mojom::IntentPtr intent,
+                        std::vector<TargetInfo> targets);
+
   uint32_t delegate_counter_ = 0;
   std::unique_ptr<SharesheetActionCache> sharesheet_action_cache_;
   apps::AppServiceProxy* app_service_proxy_;
@@ -57,6 +80,8 @@ class SharesheetService : public KeyedService {
   // Record of all active SharesheetServiceDelegates. These can be retrieved
   // by ShareActions and used as SharesheetControllers to make bubble changes.
   std::vector<std::unique_ptr<SharesheetServiceDelegate>> active_delegates_;
+
+  base::WeakPtrFactory<SharesheetService> weak_factory_{this};
 };
 
 }  // namespace sharesheet

chrome/browser/sharesheet/sharesheet_types.cc

@@ -7,7 +7,7 @@
 namespace sharesheet {
 
 TargetInfo::TargetInfo(TargetType type,
-                       const gfx::Image& icon,
+                       const gfx::ImageSkia& icon,
                        const base::string16& launch_name,
                        const base::string16& display_name)
     : type(type),

chrome/browser/sharesheet/sharesheet_types.h

@@ -6,7 +6,7 @@
 #define CHROME_BROWSER_SHARESHEET_SHARESHEET_TYPES_H_
 
 #include "base/strings/string16.h"
-#include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_skia.h"
 
 namespace sharesheet {
 
@@ -19,7 +19,7 @@ enum class TargetType {
 
 struct TargetInfo {
   TargetInfo(TargetType type,
-             const gfx::Image& icon,
+             const gfx::ImageSkia& icon,
              const base::string16& launch_name,
              const base::string16& display_name);
   // Allow move.
@@ -34,7 +34,7 @@ struct TargetInfo {
   TargetType type;
 
   // The icon to be displayed for this target in the sharesheet bubble.
-  gfx::Image icon;
+  gfx::ImageSkia icon;
 
   // The string used to launch this target. Represents an Android package name
   // when the app type is kArc.

chrome/browser/ui/views/sharesheet_bubble_view.cc

@@ -44,7 +44,7 @@ class ShareSheetTargetButton : public views::Button {
  public:
   ShareSheetTargetButton(views::ButtonListener* listener,
                          const base::string16& display_name,
-                         const gfx::Image* icon)
+                         const gfx::ImageSkia* icon)
       : Button(listener) {
     SetLayoutManager(std::make_unique<views::BoxLayout>(
         views::BoxLayout::Orientation::kVertical, gfx::Insets(),
@@ -54,8 +54,8 @@ class ShareSheetTargetButton : public views::Button {
     auto* image = AddChildView(std::make_unique<views::ImageView>());
     image->set_can_process_events_within_subtree(false);
 
-    if (!icon->IsEmpty()) {
-      image->SetImage(*icon->ToImageSkia());
+    if (!icon->isNull()) {
+      image->SetImage(icon);
     }
 
     auto* label = AddChildView(std::make_unique<views::Label>(display_name));
@@ -101,9 +101,7 @@ SharesheetBubbleView::SharesheetBubbleView(
 
   auto share_action_view = std::make_unique<views::View>();
   share_action_view->SetLayoutManager(std::make_unique<views::BoxLayout>(
-      views::BoxLayout::Orientation::kVertical, gfx::Insets(),
-      ChromeLayoutProvider::Get()->GetDistanceMetric(
-          views::DISTANCE_RELATED_CONTROL_VERTICAL)));
+      views::BoxLayout::Orientation::kVertical, gfx::Insets(), 0, true));
   share_action_view_ = root_view_->AddChildView(std::move(share_action_view));
   share_action_view_->SetVisible(false);
 }