Skip to content

Commit

Permalink
Revert "Use libappindicator to show icons in all cases"
Browse files Browse the repository at this point in the history
This reverts commit 05a0b26.

Reason for revert: http://crbug.com/815046 - same swarming bots failing because they don't have libappindicator3.

Original change's description:
> Use libappindicator to show icons in all cases
>
> libappindicator will transparently use GtkStatusIcon if an AppIndicator
> listener is not available. We will lose the left click action however,
> and instead the right-click menu will be shown in all instances.
>
> Using libappindicator in all cases not only reduces code complexity,
> but also abstracts away the complexity of determining when to use
> application indicators (vs legacy icons), which is a little more
> complicated with newer versions of GNOME removing the legacy icon tray,
> but older versions of GNOME not showing the application indicator by
> default.
>
> I also fixed an issue with detecting GNOME. The environment variable
> value is often prefixed with the distro.
>
> Bug: 799144, 797332, 419673
> Change-Id: I156698f1b37ba216b4df11fa2f17dd7012c593fa
> Reviewed-on: https://chromium-review.googlesource.com/911820
> Reviewed-by: Elliot Glaysher <erg@chromium.org>
> Reviewed-by: Scott Graham <scottmg@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Commit-Queue: Tim Brown <timbrown@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#538635}

TBR=thomasanderson@chromium.org

Change-Id: I61db904019740f878c4f6d6303e600b717d7888d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 799144, 797332, 419673
Reviewed-on: https://chromium-review.googlesource.com/934701
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Commit-Queue: Ian Clelland <iclelland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538800}
  • Loading branch information
Tim Brown authored and Commit Bot committed Feb 23, 2018
1 parent 8fb946b commit fda9690
Show file tree
Hide file tree
Showing 8 changed files with 311 additions and 33 deletions.
3 changes: 1 addition & 2 deletions base/nix/xdg_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ DesktopEnvironment GetDesktopEnvironment(Environment* env) {
}
return DESKTOP_ENVIRONMENT_UNITY;
}
if (base::EndsWith(xdg_current_desktop, "GNOME",
base::CompareCase::SENSITIVE))
if (xdg_current_desktop == "GNOME")
return DESKTOP_ENVIRONMENT_GNOME;
if (xdg_current_desktop == "X-Cinnamon")
return DESKTOP_ENVIRONMENT_CINNAMON;
Expand Down
10 changes: 0 additions & 10 deletions base/nix/xdg_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const char* const kXdgDesktopCinnamon = "X-Cinnamon";
const char* const kXdgDesktopGNOME = "GNOME";
const char* const kXdgDesktopKDE = "KDE";
const char* const kXdgDesktopPantheon = "Pantheon";
const char* const kXdgDesktopUbuntuGNOME = "ubuntu:GNOME";
const char* const kXdgDesktopUnity = "Unity";
const char* const kXdgDesktopUnity7 = "Unity:Unity7";
const char* const kXdgDesktopUnity8 = "Unity:Unity8";
Expand Down Expand Up @@ -111,15 +110,6 @@ TEST(XDGUtilTest, GetXdgDesktopGnome) {
EXPECT_EQ(DESKTOP_ENVIRONMENT_GNOME, GetDesktopEnvironment(&getter));
}

TEST(XDGUtilTest, GetXdgDesktopUbuntuGnome) {
MockEnvironment getter;
EXPECT_CALL(getter, GetVar(_, _)).WillRepeatedly(Return(false));
EXPECT_CALL(getter, GetVar(Eq(kXdgDesktop), _))
.WillOnce(DoAll(SetArgPointee<1>(kXdgDesktopUbuntuGNOME), Return(true)));

EXPECT_EQ(DESKTOP_ENVIRONMENT_GNOME, GetDesktopEnvironment(&getter));
}

TEST(XDGUtilTest, GetXdgDesktopGnomeFallback) {
MockEnvironment getter;
EXPECT_CALL(getter, GetVar(_, _)).WillRepeatedly(Return(false));
Expand Down
14 changes: 0 additions & 14 deletions build/config/linux/appindicator/BUILD.gn

This file was deleted.

3 changes: 2 additions & 1 deletion chrome/browser/ui/libgtkui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ template("libgtkui") {
"gtk_key_bindings_handler.cc",
"gtk_key_bindings_handler.h",
"gtk_signal.h",
"gtk_status_icon.cc",
"gtk_status_icon.h",
"gtk_ui.cc",
"gtk_ui.h",
"gtk_util.cc",
Expand All @@ -64,7 +66,6 @@ template("libgtkui") {
]

configs += [
"//build/config/linux/appindicator",
"//build/config/linux/pangocairo",
"//build/config/linux:x11",
]
Expand Down
154 changes: 152 additions & 2 deletions chrome/browser/ui/libgtkui/app_indicator_icon.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

#include <dlfcn.h>
#include <gtk/gtk.h>
#include <libappindicator/app-indicator.h>

#include "base/bind.h"
#include "base/environment.h"
Expand All @@ -27,6 +26,145 @@

namespace {

typedef enum {
APP_INDICATOR_CATEGORY_APPLICATION_STATUS,
APP_INDICATOR_CATEGORY_COMMUNICATIONS,
APP_INDICATOR_CATEGORY_SYSTEM_SERVICES,
APP_INDICATOR_CATEGORY_HARDWARE,
APP_INDICATOR_CATEGORY_OTHER
} AppIndicatorCategory;

typedef enum {
APP_INDICATOR_STATUS_PASSIVE,
APP_INDICATOR_STATUS_ACTIVE,
APP_INDICATOR_STATUS_ATTENTION
} AppIndicatorStatus;

typedef AppIndicator* (*app_indicator_new_func)(const gchar* id,
const gchar* icon_name,
AppIndicatorCategory category);

typedef AppIndicator* (*app_indicator_new_with_path_func)(
const gchar* id,
const gchar* icon_name,
AppIndicatorCategory category,
const gchar* icon_theme_path);

typedef void (*app_indicator_set_status_func)(AppIndicator* self,
AppIndicatorStatus status);

typedef void (*app_indicator_set_attention_icon_full_func)(
AppIndicator* self,
const gchar* icon_name,
const gchar* icon_desc);

typedef void (*app_indicator_set_menu_func)(AppIndicator* self, GtkMenu* menu);

typedef void (*app_indicator_set_icon_full_func)(AppIndicator* self,
const gchar* icon_name,
const gchar* icon_desc);

typedef void (*app_indicator_set_icon_theme_path_func)(
AppIndicator* self,
const gchar* icon_theme_path);

bool g_attempted_load = false;
bool g_opened = false;

// Retrieved functions from libappindicator.
app_indicator_new_func app_indicator_new = nullptr;
app_indicator_new_with_path_func app_indicator_new_with_path = nullptr;
app_indicator_set_status_func app_indicator_set_status = nullptr;
app_indicator_set_attention_icon_full_func
app_indicator_set_attention_icon_full = nullptr;
app_indicator_set_menu_func app_indicator_set_menu = nullptr;
app_indicator_set_icon_full_func app_indicator_set_icon_full = nullptr;
app_indicator_set_icon_theme_path_func app_indicator_set_icon_theme_path =
nullptr;

bool ShouldUseLibAppIndicator() {
// Only use libappindicator where it is needed to support dbus based status
// icons. In particular, libappindicator does not support a click action.
std::unique_ptr<base::Environment> env(base::Environment::Create());
switch (base::nix::GetDesktopEnvironment(env.get())) {
case base::nix::DESKTOP_ENVIRONMENT_KDE4:
case base::nix::DESKTOP_ENVIRONMENT_KDE5:
case base::nix::DESKTOP_ENVIRONMENT_PANTHEON:
case base::nix::DESKTOP_ENVIRONMENT_UNITY:
return true;
case base::nix::DESKTOP_ENVIRONMENT_CINNAMON:
case base::nix::DESKTOP_ENVIRONMENT_GNOME:
case base::nix::DESKTOP_ENVIRONMENT_KDE3:
case base::nix::DESKTOP_ENVIRONMENT_OTHER:
case base::nix::DESKTOP_ENVIRONMENT_XFCE:
return false;
}
}

void EnsureMethodsLoaded() {
if (g_attempted_load)
return;

g_attempted_load = true;

if (!ShouldUseLibAppIndicator())
return;

void* indicator_lib = nullptr;

// These include guards might be unnecessary, but let's keep them as a
// precaution since using gtk2 and gtk3 symbols in the same process is
// explicitly unsupported.
#if GTK_MAJOR_VERSION == 2
if (!indicator_lib)
indicator_lib = dlopen("libappindicator.so", RTLD_LAZY);

if (!indicator_lib)
indicator_lib = dlopen("libappindicator.so.1", RTLD_LAZY);

if (!indicator_lib)
indicator_lib = dlopen("libappindicator.so.0", RTLD_LAZY);
#endif

#if GTK_MAJOR_VERSION == 3
if (!indicator_lib)
indicator_lib = dlopen("libappindicator3.so", RTLD_LAZY);

if (!indicator_lib)
indicator_lib = dlopen("libappindicator3.so.1", RTLD_LAZY);
#endif

if (!indicator_lib)
return;

g_opened = true;

app_indicator_new = reinterpret_cast<app_indicator_new_func>(
dlsym(indicator_lib, "app_indicator_new"));

app_indicator_new_with_path =
reinterpret_cast<app_indicator_new_with_path_func>(
dlsym(indicator_lib, "app_indicator_new_with_path"));

app_indicator_set_status = reinterpret_cast<app_indicator_set_status_func>(
dlsym(indicator_lib, "app_indicator_set_status"));

app_indicator_set_attention_icon_full =
reinterpret_cast<app_indicator_set_attention_icon_full_func>(
dlsym(indicator_lib, "app_indicator_set_attention_icon_full"));

app_indicator_set_menu = reinterpret_cast<app_indicator_set_menu_func>(
dlsym(indicator_lib, "app_indicator_set_menu"));

app_indicator_set_icon_full =
reinterpret_cast<app_indicator_set_icon_full_func>(
dlsym(indicator_lib, "app_indicator_set_icon_full"));

app_indicator_set_icon_theme_path =
reinterpret_cast<app_indicator_set_icon_theme_path_func>(
dlsym(indicator_lib, "app_indicator_set_icon_theme_path"));
}

// Writes |bitmap| to a file at |path|. Returns true if successful.
bool WriteFile(const base::FilePath& path, const SkBitmap& bitmap) {
std::vector<unsigned char> png_data;
Expand Down Expand Up @@ -58,10 +196,10 @@ AppIndicatorIcon::AppIndicatorIcon(std::string id,
std::unique_ptr<base::Environment> env(base::Environment::Create());
desktop_env_ = base::nix::GetDesktopEnvironment(env.get());

EnsureMethodsLoaded();
tool_tip_ = base::UTF16ToUTF8(tool_tip);
SetImage(image);
}

AppIndicatorIcon::~AppIndicatorIcon() {
if (icon_) {
app_indicator_set_status(icon_, APP_INDICATOR_STATUS_PASSIVE);
Expand All @@ -72,7 +210,16 @@ AppIndicatorIcon::~AppIndicatorIcon() {
}
}

// static
bool AppIndicatorIcon::CouldOpen() {
EnsureMethodsLoaded();
return g_opened;
}

void AppIndicatorIcon::SetImage(const gfx::ImageSkia& image) {
if (!g_opened)
return;

++icon_change_count_;

// Copy the bitmap because it may be freed by the time it's accessed in
Expand Down Expand Up @@ -108,6 +255,9 @@ void AppIndicatorIcon::SetToolTip(const base::string16& tool_tip) {
}

void AppIndicatorIcon::UpdatePlatformContextMenu(ui::MenuModel* model) {
if (!g_opened)
return;

menu_model_ = model;

// The icon is created asynchronously so it might not exist when the menu is
Expand Down
84 changes: 84 additions & 0 deletions chrome/browser/ui/libgtkui/gtk_status_icon.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/ui/libgtkui/gtk_status_icon.h"

#include <gtk/gtk.h>

#include "base/debug/leak_annotations.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/ui/libgtkui/app_indicator_icon_menu.h"
#include "chrome/browser/ui/libgtkui/skia_utils_gtk.h"
#include "ui/base/models/menu_model.h"
#include "ui/gfx/image/image_skia.h"

G_GNUC_BEGIN_IGNORE_DEPRECATIONS

namespace libgtkui {

Gtk2StatusIcon::Gtk2StatusIcon(const gfx::ImageSkia& image,
const base::string16& tool_tip) {
GdkPixbuf* pixbuf = GdkPixbufFromSkBitmap(*image.bitmap());
{
#if GTK_MAJOR_VERSION == 3
// Gtk3 has a bug that leaks 384 bytes when creating a
// GtkStatusIcon. It will not be fixed since the status icon was
// deprectaed in version 3.14. Luckily, Chromium doesn't need to
// create a status icon very often, if at all.
ANNOTATE_SCOPED_MEMORY_LEAK;
#endif
gtk_status_icon_ = gtk_status_icon_new_from_pixbuf(pixbuf);
}
g_object_unref(pixbuf);

g_signal_connect(gtk_status_icon_, "activate", G_CALLBACK(OnClickThunk),
this);
g_signal_connect(gtk_status_icon_, "popup_menu",
G_CALLBACK(OnContextMenuRequestedThunk), this);
SetToolTip(tool_tip);
}

Gtk2StatusIcon::~Gtk2StatusIcon() {
gtk_status_icon_set_visible(gtk_status_icon_, FALSE);
g_object_unref(gtk_status_icon_);
}

void Gtk2StatusIcon::SetImage(const gfx::ImageSkia& image) {
GdkPixbuf* pixbuf = GdkPixbufFromSkBitmap(*image.bitmap());
gtk_status_icon_set_from_pixbuf(gtk_status_icon_, pixbuf);
g_object_unref(pixbuf);
}

void Gtk2StatusIcon::SetToolTip(const base::string16& tool_tip) {
gtk_status_icon_set_tooltip_text(gtk_status_icon_,
base::UTF16ToUTF8(tool_tip).c_str());
}

void Gtk2StatusIcon::UpdatePlatformContextMenu(ui::MenuModel* model) {
menu_.reset();
if (model)
menu_.reset(new AppIndicatorIconMenu(model));
}

void Gtk2StatusIcon::RefreshPlatformContextMenu() {
if (menu_.get())
menu_->Refresh();
}

void Gtk2StatusIcon::OnClick(GtkStatusIcon* status_icon) {
if (delegate())
delegate()->OnClick();
}

void Gtk2StatusIcon::OnContextMenuRequested(GtkStatusIcon* status_icon,
guint button,
guint32 activate_time) {
if (menu_.get()) {
gtk_menu_popup(menu_->GetGtkMenu(), nullptr, nullptr,
gtk_status_icon_position_menu, gtk_status_icon_, button,
activate_time);
}
}

} // namespace libgtkui
Loading

0 comments on commit fda9690

Please sign in to comment.