Browse Source

refactor: don't use AppIndicatorIcon directly (#15536)

Jeremy Apthorp 6 years ago
parent
commit
934a7fc118

+ 3 - 19
atom/browser/ui/tray_icon_gtk.cc

@@ -8,19 +8,11 @@
 #include "atom/common/application_info.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/ui/libgtkui/app_indicator_icon.h"
-#include "chrome/browser/ui/libgtkui/gtk_status_icon.h"
 #include "ui/gfx/image/image.h"
+#include "ui/views/linux_ui/linux_ui.h"
 
 namespace atom {
 
-namespace {
-
-// Number of app indicators used (used as part of app-indicator id).
-int indicators_count;
-
-}  // namespace
-
 TrayIconGtk::TrayIconGtk() {}
 
 TrayIconGtk::~TrayIconGtk() {}
@@ -32,16 +24,8 @@ void TrayIconGtk::SetImage(const gfx::Image& image) {
   }
 
   const auto toolTip = base::UTF8ToUTF16(GetApplicationName());
-
-  if (libgtkui::AppIndicatorIcon::CouldOpen()) {
-    ++indicators_count;
-    icon_.reset(new libgtkui::AppIndicatorIcon(
-        base::StringPrintf("%s%d", Browser::Get()->GetName().c_str(),
-                           indicators_count),
-        image.AsImageSkia(), toolTip));
-  } else {
-    icon_.reset(new libgtkui::Gtk2StatusIcon(image.AsImageSkia(), toolTip));
-  }
+  icon_ = views::LinuxUI::instance()->CreateLinuxStatusIcon(
+      image.AsImageSkia(), toolTip, Browser::Get()->GetName().c_str());
   icon_->set_delegate(this);
 }
 

+ 1 - 0
patches/common/chromium/.patches

@@ -75,3 +75,4 @@ color_chooser.patch
 printing.patch
 verbose_generate_breakpad_symbols.patch
 chrome_process_finder.patch
+customizable_app_indicator_id_prefix.patch

+ 107 - 0
patches/common/chromium/customizable_app_indicator_id_prefix.patch

@@ -0,0 +1,107 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jeremy Apthorp <[email protected]>
+Date: Mon, 5 Nov 2018 20:13:27 +0000
+Subject: backport from M72: Make gtk app indicator id prefix customizable
+
+This makes the app indicator API in libgtkui usable by non-Chrome apps (in
+particular, Electron).
+
+Change-Id: I39e547fc04595900d99806208955c632e4199be4
+Reviewed-on: https://chromium-review.googlesource.com/c/1315840
+Commit-Queue: Jeremy Apthorp <[email protected]>
+Reviewed-by: Thomas Anderson <[email protected]>
+Reviewed-by: Scott Violet <[email protected]>
+Cr-Commit-Position: refs/heads/master@{#605435}
+
+diff --git a/chrome/browser/ui/libgtkui/gtk_ui.cc b/chrome/browser/ui/libgtkui/gtk_ui.cc
+index 23542f2cd7a58bd4b4f51847a47740fc40560dad..77e42d5ed707670cae384b44fd1630c768c21775 100644
+--- a/chrome/browser/ui/libgtkui/gtk_ui.cc
++++ b/chrome/browser/ui/libgtkui/gtk_ui.cc
+@@ -274,9 +274,6 @@ typedef std::unique_ptr<GIcon, GObjectDeleter> ScopedGIcon;
+ typedef std::unique_ptr<GtkIconInfo, GtkIconInfoDeleter> ScopedGtkIconInfo;
+ typedef std::unique_ptr<GdkPixbuf, GObjectDeleter> ScopedGdkPixbuf;
+ 
+-// Prefix for app indicator ids
+-const char kAppIndicatorIdPrefix[] = "chrome_app_indicator_";
+-
+ // Number of app indicators used (used as part of app-indicator id).
+ int indicators_count;
+ 
+@@ -645,12 +642,13 @@ bool GtkUi::IsStatusIconSupported() const {
+ 
+ std::unique_ptr<views::StatusIconLinux> GtkUi::CreateLinuxStatusIcon(
+     const gfx::ImageSkia& image,
+-    const base::string16& tool_tip) const {
++    const base::string16& tool_tip,
++    const char* id_prefix) const {
+   if (AppIndicatorIcon::CouldOpen()) {
+     ++indicators_count;
+     return std::unique_ptr<views::StatusIconLinux>(new AppIndicatorIcon(
+-        base::StringPrintf("%s%d", kAppIndicatorIdPrefix, indicators_count),
+-        image, tool_tip));
++        base::StringPrintf("%s%d", id_prefix, indicators_count), image,
++        tool_tip));
+   } else {
+     return std::unique_ptr<views::StatusIconLinux>(
+         new Gtk2StatusIcon(image, tool_tip));
+diff --git a/chrome/browser/ui/libgtkui/gtk_ui.h b/chrome/browser/ui/libgtkui/gtk_ui.h
+index 8f3539e3aa339fc69fa1db68a32f9f04d4bedc3a..99760afb4a1ed2e6519a1be4dbcbde45f0e458b7 100644
+--- a/chrome/browser/ui/libgtkui/gtk_ui.h
++++ b/chrome/browser/ui/libgtkui/gtk_ui.h
+@@ -92,7 +92,8 @@ class GtkUi : public views::LinuxUI {
+   bool IsStatusIconSupported() const override;
+   std::unique_ptr<views::StatusIconLinux> CreateLinuxStatusIcon(
+       const gfx::ImageSkia& image,
+-      const base::string16& tool_tip) const override;
++      const base::string16& tool_tip,
++      const char* id_prefix) const override;
+   gfx::Image GetIconForContentType(const std::string& content_type,
+                                    int size) const override;
+   std::unique_ptr<views::Border> CreateNativeBorder(
+diff --git a/chrome/browser/ui/views/status_icons/status_icon_linux_wrapper.cc b/chrome/browser/ui/views/status_icons/status_icon_linux_wrapper.cc
+index eed6bb2eaf756189be016c382673e23eb7ca18e0..4694a9a920b1f9150399e183038f04ac700b4f52 100644
+--- a/chrome/browser/ui/views/status_icons/status_icon_linux_wrapper.cc
++++ b/chrome/browser/ui/views/status_icons/status_icon_linux_wrapper.cc
+@@ -8,6 +8,13 @@
+ #include "ui/message_center/public/cpp/notifier_id.h"
+ #include "ui/views/linux_ui/linux_ui.h"
+ 
++namespace {
++
++// Prefix for app indicator ids
++const char kAppIndicatorIdPrefix[] = "chrome_app_indicator_";
++
++}  // namespace
++
+ StatusIconLinuxWrapper::StatusIconLinuxWrapper(
+     std::unique_ptr<views::StatusIconLinux> status_icon)
+     : status_icon_(std::move(status_icon)), menu_model_(nullptr) {
+@@ -53,7 +60,8 @@ StatusIconLinuxWrapper::CreateWrappedStatusIcon(
+     const base::string16& tool_tip) {
+   const views::LinuxUI* linux_ui = views::LinuxUI::instance();
+   if (linux_ui) {
+-    auto status_icon = linux_ui->CreateLinuxStatusIcon(image, tool_tip);
++    auto status_icon =
++        linux_ui->CreateLinuxStatusIcon(image, tool_tip, kAppIndicatorIdPrefix);
+     if (status_icon) {
+       return base::WrapUnique(
+           new StatusIconLinuxWrapper(std::move(status_icon)));
+diff --git a/ui/views/linux_ui/linux_ui.h b/ui/views/linux_ui/linux_ui.h
+index c1f14d5c65aa5c1c58633c59431c1e7c5c720a0b..7c27b61e93f83b7f277461df94fb3c939e7702d3 100644
+--- a/ui/views/linux_ui/linux_ui.h
++++ b/ui/views/linux_ui/linux_ui.h
+@@ -137,10 +137,12 @@ class VIEWS_EXPORT LinuxUI : public ui::LinuxInputMethodContextFactory,
+   // Checks for platform support for status icons.
+   virtual bool IsStatusIconSupported() const = 0;
+ 
+-  // Create a native status icon.
++  // Create a native status icon. The id_prefix is used to distinguish Chrome's
++  // status icons from other apps' status icons, and should be unique.
+   virtual std::unique_ptr<StatusIconLinux> CreateLinuxStatusIcon(
+       const gfx::ImageSkia& image,
+-      const base::string16& tool_tip) const = 0;
++      const base::string16& tool_tip,
++      const char* id_prefix) const = 0;
+ 
+   // Returns the icon for a given content type from the icon theme.
+   // TODO(davidben): Add an observer for the theme changing, so we can drop the

+ 0 - 42
patches/common/chromium/libgtkui_export.patch

@@ -5,48 +5,6 @@ Subject: libgtkui_export.patch
 
 Export libgtkui symbols for the GN component build.
 
-diff --git a/chrome/browser/ui/libgtkui/app_indicator_icon.h b/chrome/browser/ui/libgtkui/app_indicator_icon.h
-index 7815fbb2cea8d3c3434287fa32c57e095199c217..f17a5c59e64b22ef0b864c3b8dd2dcb8b17d0c84 100644
---- a/chrome/browser/ui/libgtkui/app_indicator_icon.h
-+++ b/chrome/browser/ui/libgtkui/app_indicator_icon.h
-@@ -12,6 +12,7 @@
- #include "base/memory/weak_ptr.h"
- #include "base/nix/xdg_util.h"
- #include "chrome/browser/ui/libgtkui/gtk_signal.h"
-+#include "chrome/browser/ui/libgtkui/libgtkui_export.h"
- #include "ui/views/linux_ui/status_icon_linux.h"
- 
- typedef struct _AppIndicator AppIndicator;
-@@ -31,7 +32,7 @@ namespace libgtkui {
- class AppIndicatorIconMenu;
- 
- // Status icon implementation which uses libappindicator.
--class AppIndicatorIcon : public views::StatusIconLinux {
-+class LIBGTKUI_EXPORT AppIndicatorIcon : public views::StatusIconLinux {
-  public:
-   // The id uniquely identifies the new status icon from other chrome status
-   // icons.
-diff --git a/chrome/browser/ui/libgtkui/gtk_status_icon.h b/chrome/browser/ui/libgtkui/gtk_status_icon.h
-index e4e0da40981ca58964dfdcbd3fc0f730c9c09ec1..af028715ada9b8023f0ff7cf60e0f7e7acd6c042 100644
---- a/chrome/browser/ui/libgtkui/gtk_status_icon.h
-+++ b/chrome/browser/ui/libgtkui/gtk_status_icon.h
-@@ -10,6 +10,7 @@
- #include "base/macros.h"
- #include "base/strings/string16.h"
- #include "chrome/browser/ui/libgtkui/gtk_signal.h"
-+#include "chrome/browser/ui/libgtkui/libgtkui_export.h"
- #include "ui/base/glib/glib_integers.h"
- #include "ui/base/glib/glib_signal.h"
- #include "ui/views/linux_ui/status_icon_linux.h"
-@@ -29,7 +30,7 @@ class AppIndicatorIconMenu;
- 
- // Status icon implementation which uses the system tray X11 spec (via
- // GtkStatusIcon).
--class Gtk2StatusIcon : public views::StatusIconLinux {
-+class LIBGTKUI_EXPORT Gtk2StatusIcon : public views::StatusIconLinux {
-  public:
-   Gtk2StatusIcon(const gfx::ImageSkia& image, const base::string16& tool_tip);
-   ~Gtk2StatusIcon() override;
 diff --git a/chrome/browser/ui/libgtkui/gtk_util.h b/chrome/browser/ui/libgtkui/gtk_util.h
 index d9f245070249f5f153bd8fe11e0a5e718c7d3629..a0f033c3e3f7f3996897f5f969b2a209d7d5cf3f 100644
 --- a/chrome/browser/ui/libgtkui/gtk_util.h