Switch to using appindicator3 by default on Linux |
||||||
Issue descriptionWe should get rid of the X11 indicator backend and switch to using appindicator by default. List of required tasks: 1. List libappindicator3-1 instead of libappindicator1 as a dependency on debian https://cs.chromium.org/chromium/src/chrome/installer/linux/debian/additional_deps?rcl=f2e30e88e4cb38d025bc59aa39b67493da8387c0&l=13 2. List libappindicator3.so.1 as an rpm dependency https://cs.chromium.org/chromium/src/chrome/installer/linux/rpm/additional_deps?sq=package:chromium 3. Add appindicator (both versions) to sysroots 4. Add libappindicator3-1 to instrumented libraries (libappindicator1 is already there) 5. Switch to linking against the appropriate appindicator version at compile time instead of dynamically loading the library https://cs.chromium.org/chromium/src/chrome/browser/ui/libgtkui/app_indicator_icon.cc?rcl=f2e30e88e4cb38d025bc59aa39b67493da8387c0&l=118 6. Remove the old X11 code and always use the appindicator codepath I would prefer to remove status icons entirely, but https://bugs.chromium.org/p/chromium/issues/detail?id=771839#c3 suggests that they still need to be used to display a chrome icon when chrome is running in background mode. There's also https://bugs.chromium.org/p/chromium/issues/detail?id=419673#c46 which suggests switching between the X11 and appindicator impls based on the presence of a dbus service instead of XDG_CURRENT_DESKTOP. But I don't think there are any DE's left at this point that support the legacy X11 icons and not appindicator icons so this would probably be no different than always using appindicator. +erg does this all sound reasonable? +rdevlin.cronin and estade from bug 771839
,
Jan 4 2018
timbrown@ are there any DE's that only have the X11 status icon? If so, https://bugs.chromium.org/p/chromium/issues/detail?id=419673#c46 is probably our best bet. As an aside, how is the status icon supposed to work on wayland? It would be nice to have a display-server-agnostic way to handle it.
,
Jan 5 2018
I am still experimenting to confirm the behavior, but AFAIU libappindicator will fall back to using XEmbed if there is no appindicator service (i.e. if there is no StatusNotifierWatcher dbus service). In other words, using libappindicator should use appindicator/StatusNotifierItem where available (e.g. Unity/KDE respectively), and fallback to plain X11 status icons where they are not available. The only thing we lose (or _should_ lose) is the default action on left click on the icon. Instead the menu will popup, the same as a right click. As for Wayland, I haven't found anything saying there is direct support for status icons. GNOME has already defaulted to not showing them in 3.26, and Gtk 4 has apparently already removed support for GtkStatusIcon (which used legacy X11 status icons). The only way that I've read to get status icons in Wayland is to use appindicator.
,
Jan 8 2018
> I am still experimenting to confirm the behavior, but AFAIU libappindicator will fall back to using XEmbed if there is no appindicator service (i.e. if there is no StatusNotifierWatcher dbus service). Ok, this seems like pretty strong evidence to switch to appindicator, so long as the XEmbed fallback works. > The only thing we lose (or _should_ lose) is the default action on left click on the icon. Instead the menu will popup, the same as a right click. I think this is OK. Worst case, we could add a menu entry that would to what left-click would have done on XEmbed (if we don't already have that).
,
Jan 8 2018
> Worst case, we could add a menu entry that would to what left-click would have done on XEmbed (if we don't already have that). IIRC, the current appindicator integration code does this. As long as the old X11 status icon path continues to work after moving to solely libappindicator, I'm ok with this.
,
Jan 11 2018
As an update, solely using libappindicator works in all environments. Where an appindicator panel is available (e.g. Unity, GNOME with appindicator extension, KDE) that is used, and otherwise (Cinnamon, LXDE, i3status, GNOME <3.26 without appindicator extension) a legacy XEmbed icon is used. I have, however, noticed a bug in GNOME with appindicator extension. If in Chrome the user toggles the "run in background" setting, the Chrome icon is not removed, but a new icon is created every time the "run in background" setting is enabled. I'm going to try to work out what's happening there.
,
Feb 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/23c1347a9536590a847f913db08d67d8b3d44dc1 commit 23c1347a9536590a847f913db08d67d8b3d44dc1 Author: Tim Brown <timbrown@chromium.org> Date: Sat Feb 03 01:10:07 2018 Add libappindicator3-1 as a dependency This is to simplify our status icon code. Ubuntu Zesty has been EOL'd, so that has been removed too. Bug: 799144 Change-Id: I325144461a330b6321a087e4357e9033d7fdd880 Reviewed-on: https://chromium-review.googlesource.com/899867 Reviewed-by: Thomas Anderson <thomasanderson@chromium.org> Commit-Queue: Tim Brown <timbrown@chromium.org> Cr-Commit-Position: refs/heads/master@{#534224} [modify] https://crrev.com/23c1347a9536590a847f913db08d67d8b3d44dc1/chrome/installer/linux/debian/additional_deps [modify] https://crrev.com/23c1347a9536590a847f913db08d67d8b3d44dc1/chrome/installer/linux/debian/dist_package_versions.json [modify] https://crrev.com/23c1347a9536590a847f913db08d67d8b3d44dc1/chrome/installer/linux/debian/merge_package_versions.py [modify] https://crrev.com/23c1347a9536590a847f913db08d67d8b3d44dc1/chrome/installer/linux/debian/update_dist_package_versions.py [modify] https://crrev.com/23c1347a9536590a847f913db08d67d8b3d44dc1/chrome/installer/linux/rpm/additional_deps [modify] https://crrev.com/23c1347a9536590a847f913db08d67d8b3d44dc1/chrome/installer/linux/rpm/dist_package_provides.json [modify] https://crrev.com/23c1347a9536590a847f913db08d67d8b3d44dc1/chrome/installer/linux/rpm/merge_package_deps.py [modify] https://crrev.com/23c1347a9536590a847f913db08d67d8b3d44dc1/chrome/installer/linux/rpm/update_package_provides.py
,
Feb 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/06ee43ab03a2a91ace88a195865e29cdcd0468c3 commit 06ee43ab03a2a91ace88a195865e29cdcd0468c3 Author: Tim Brown <timbrown@chromium.org> Date: Thu Feb 08 18:42:12 2018 Add libappindicator and dependencies to the sysroot This adds in both the Gtk+2 and Gtk+3 packages. TBR=dpranke@chromium.org Bug: 799144 Change-Id: I692e16bd62ab2120c4855cd75ce6f3260e5d6ece Reviewed-on: https://chromium-review.googlesource.com/904856 Commit-Queue: Thomas Anderson <thomasanderson@chromium.org> Reviewed-by: Thomas Anderson <thomasanderson@chromium.org> Cr-Commit-Position: refs/heads/master@{#535457} [modify] https://crrev.com/06ee43ab03a2a91ace88a195865e29cdcd0468c3/build/install-build-deps.sh [modify] https://crrev.com/06ee43ab03a2a91ace88a195865e29cdcd0468c3/build/linux/sysroot_scripts/packagelist.stretch.amd64 [modify] https://crrev.com/06ee43ab03a2a91ace88a195865e29cdcd0468c3/build/linux/sysroot_scripts/packagelist.stretch.arm [modify] https://crrev.com/06ee43ab03a2a91ace88a195865e29cdcd0468c3/build/linux/sysroot_scripts/packagelist.stretch.arm64 [modify] https://crrev.com/06ee43ab03a2a91ace88a195865e29cdcd0468c3/build/linux/sysroot_scripts/packagelist.stretch.i386 [modify] https://crrev.com/06ee43ab03a2a91ace88a195865e29cdcd0468c3/build/linux/sysroot_scripts/packagelist.stretch.mips64el [modify] https://crrev.com/06ee43ab03a2a91ace88a195865e29cdcd0468c3/build/linux/sysroot_scripts/packagelist.stretch.mipsel [modify] https://crrev.com/06ee43ab03a2a91ace88a195865e29cdcd0468c3/build/linux/sysroot_scripts/sysroot-creator-stretch.sh [modify] https://crrev.com/06ee43ab03a2a91ace88a195865e29cdcd0468c3/build/linux/sysroot_scripts/sysroots.json
,
Feb 8 2018
The following revision refers to this bug: https://chrome-internal.googlesource.com/infra/puppet/+/b00c415b7e7629000349da40ce54443696e1ffd3 commit b00c415b7e7629000349da40ce54443696e1ffd3 Author: Tim Brown <timbrown@google.com> Date: Thu Feb 08 22:51:49 2018
,
Feb 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8d7d53f2d9d71b76145f244e0b7fb2c26948e730 commit 8d7d53f2d9d71b76145f244e0b7fb2c26948e730 Author: Tim Brown <timbrown@chromium.org> Date: Fri Feb 09 18:29:09 2018 Add libappindicator3-1 to instrumented libraries We actually had libappindicator1 already in there which both wasn't stricly needed, and actually had no chance of being used in our builds, since we use Gtk3. libappindicator3-1 will need to be in there once I make the code change to strongly depend on the library. Bug: 799144 Change-Id: Id9c13259acf72647b2543b37cb0b2e160f8df927 Reviewed-on: https://chromium-review.googlesource.com/906838 Reviewed-by: Evgeniy Stepanov <eugenis@chromium.org> Commit-Queue: Tim Brown <timbrown@chromium.org> Cr-Commit-Position: refs/heads/master@{#535758} [modify] https://crrev.com/8d7d53f2d9d71b76145f244e0b7fb2c26948e730/third_party/instrumented_libraries/BUILD.gn [modify] https://crrev.com/8d7d53f2d9d71b76145f244e0b7fb2c26948e730/third_party/instrumented_libraries/binaries/msan-chained-origins-trusty.tgz.sha1 [modify] https://crrev.com/8d7d53f2d9d71b76145f244e0b7fb2c26948e730/third_party/instrumented_libraries/binaries/msan-no-origins-trusty.tgz.sha1 [modify] https://crrev.com/8d7d53f2d9d71b76145f244e0b7fb2c26948e730/third_party/instrumented_libraries/scripts/install-build-deps.sh
,
Feb 9 2018
,
Feb 12 2018
Hi, this broke RHEL 7 installations - libappindicator3.so.1 is not available there. We need to investingate (in Red Hat) how to proceed..
,
Feb 12 2018
It's at least available in EPEL..
,
Feb 12 2018
Breaks updates on Fedora installations https://bugzilla.redhat.com/show_bug.cgi?id=1544362 There is an issue with the difference between 32 and 64 bit installations.
,
Feb 12 2018
Thanks for letting us know. I've created a CL to fix the 32-bit/64-bit dependency: http://crrev.com/c/913714
,
Feb 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e99e7768f9addeff4c61dbaab30cd25b2a996d66 commit e99e7768f9addeff4c61dbaab30cd25b2a996d66 Author: Tim Brown <timbrown@chromium.org> Date: Mon Feb 12 18:23:51 2018 Depend of 64-bit version of libappindicator3 We were incorrectly depending on the 32-bit version, which was causing installation problems downstream: https://bugzilla.redhat.com/show_bug.cgi?id=1544362 Bug: 799144 Change-Id: I51b4c55bd3de65ac9ef11d482a527d4718c30065 Reviewed-on: https://chromium-review.googlesource.com/913714 Reviewed-by: Thomas Anderson <thomasanderson@chromium.org> Commit-Queue: Thomas Anderson <thomasanderson@chromium.org> Cr-Commit-Position: refs/heads/master@{#536134} [modify] https://crrev.com/e99e7768f9addeff4c61dbaab30cd25b2a996d66/chrome/installer/linux/rpm/additional_deps
,
Feb 21 2018
The following revision refers to this bug: https://chrome-internal.googlesource.com/infra/puppet/+/8a5f06e923a8ef02af4627662dc6589a241f9435 commit 8a5f06e923a8ef02af4627662dc6589a241f9435 Author: Tim Brown <timbrown@google.com> Date: Wed Feb 21 20:45:39 2018
,
Feb 23 2018
The following revision refers to this bug: https://chrome-internal.googlesource.com/infradata/config/+/b7f1cf7f2a1de086f58506d429fa6c0d6025b91e commit b7f1cf7f2a1de086f58506d429fa6c0d6025b91e Author: smut <smut@google.com> Date: Fri Feb 23 00:14:17 2018
,
Feb 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/05a0b26e256a7fc45836ce69e43e439850d5dcce commit 05a0b26e256a7fc45836ce69e43e439850d5dcce Author: Tim Brown <timbrown@chromium.org> Date: Fri Feb 23 00:18:13 2018 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} [modify] https://crrev.com/05a0b26e256a7fc45836ce69e43e439850d5dcce/base/nix/xdg_util.cc [modify] https://crrev.com/05a0b26e256a7fc45836ce69e43e439850d5dcce/base/nix/xdg_util_unittest.cc [add] https://crrev.com/05a0b26e256a7fc45836ce69e43e439850d5dcce/build/config/linux/appindicator/BUILD.gn [modify] https://crrev.com/05a0b26e256a7fc45836ce69e43e439850d5dcce/chrome/browser/ui/libgtkui/BUILD.gn [modify] https://crrev.com/05a0b26e256a7fc45836ce69e43e439850d5dcce/chrome/browser/ui/libgtkui/app_indicator_icon.cc [delete] https://crrev.com/6f9f9577f12a005b3eb9edb2fd2a093d6930e0bc/chrome/browser/ui/libgtkui/gtk_status_icon.cc [delete] https://crrev.com/6f9f9577f12a005b3eb9edb2fd2a093d6930e0bc/chrome/browser/ui/libgtkui/gtk_status_icon.h [modify] https://crrev.com/05a0b26e256a7fc45836ce69e43e439850d5dcce/chrome/browser/ui/libgtkui/gtk_ui.cc
,
Feb 23 2018
,
Feb 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fda969051971b257fb637815b15f03e2353ef2e5 commit fda969051971b257fb637815b15f03e2353ef2e5 Author: Tim Brown <timbrown@chromium.org> Date: Fri Feb 23 16:35:04 2018 Revert "Use libappindicator to show icons in all cases" This reverts commit 05a0b26e256a7fc45836ce69e43e439850d5dcce. 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} [modify] https://crrev.com/fda969051971b257fb637815b15f03e2353ef2e5/base/nix/xdg_util.cc [modify] https://crrev.com/fda969051971b257fb637815b15f03e2353ef2e5/base/nix/xdg_util_unittest.cc [delete] https://crrev.com/8fb946b56bd14e8bf65c562addfedf7a5b18db7f/build/config/linux/appindicator/BUILD.gn [modify] https://crrev.com/fda969051971b257fb637815b15f03e2353ef2e5/chrome/browser/ui/libgtkui/BUILD.gn [modify] https://crrev.com/fda969051971b257fb637815b15f03e2353ef2e5/chrome/browser/ui/libgtkui/app_indicator_icon.cc [add] https://crrev.com/fda969051971b257fb637815b15f03e2353ef2e5/chrome/browser/ui/libgtkui/gtk_status_icon.cc [add] https://crrev.com/fda969051971b257fb637815b15f03e2353ef2e5/chrome/browser/ui/libgtkui/gtk_status_icon.h [modify] https://crrev.com/fda969051971b257fb637815b15f03e2353ef2e5/chrome/browser/ui/libgtkui/gtk_ui.cc
,
Feb 27 2018
Reopening due to rollback
,
Feb 27 2018
The following revision refers to this bug: https://chrome-internal.googlesource.com/infra/puppet/+/3594825ab5635d43f6ddd6908839440c5885f54a commit 3594825ab5635d43f6ddd6908839440c5885f54a Author: Tim Brown <timbrown@google.com> Date: Tue Feb 27 22:20:56 2018
,
Feb 28 2018
The following revision refers to this bug: https://chrome-internal.googlesource.com/infra/puppet/+/9d9470c62c8ca41d2a8ef03fec8db468bc28e1e3 commit 9d9470c62c8ca41d2a8ef03fec8db468bc28e1e3 Author: Elliott Friedman <friedman@google.com> Date: Wed Feb 28 00:42:42 2018
,
Feb 28 2018
The following revision refers to this bug: https://chrome-internal.googlesource.com/infra/puppet/+/29f0ea88594615d916e8c56f6eac3d43db757dd8 commit 29f0ea88594615d916e8c56f6eac3d43db757dd8 Author: Elliott Friedman <friedman@google.com> Date: Wed Feb 28 01:02:28 2018
,
Feb 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6e3fb99a7265b2481b9570344de4669f4c26a14c commit 6e3fb99a7265b2481b9570344de4669f4c26a14c Author: Tim Brown <timbrown@chromium.org> Date: Wed Feb 28 16:45:50 2018 Add 32-bit version of libappindicator3-1 When doing a 32-bit build on a 64-bit system, the 32-bit version of the library is not available. This CL should fix that. Bug: 799144 Change-Id: I40c6d16d708db102eb4da90ded7bc336cb9f9b09 Reviewed-on: https://chromium-review.googlesource.com/940196 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Tim Brown <timbrown@chromium.org> Cr-Commit-Position: refs/heads/master@{#539851} [modify] https://crrev.com/6e3fb99a7265b2481b9570344de4669f4c26a14c/build/install-build-deps.sh
,
Feb 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6e40caade1e6ae35c34127ca6b4fda7a9eaf3e38 commit 6e40caade1e6ae35c34127ca6b4fda7a9eaf3e38 Author: Tim Brown <timbrown@chromium.org> Date: Wed Feb 28 16:46:08 2018 Always use libappindicator if available This is in effect a temporary fix to always use libappindicator (when available, but we have deb/rpm dependency on the lib, so it should always be available), whilst I fix the dependency issue we have on the build bots which would allow me to add a hard dependency and clean up the code. Bug: 799144, 797332 , 419673 Change-Id: Id64a452029b00ec8d86f0d35d319bebcfd8bf99e Reviewed-on: https://chromium-review.googlesource.com/939732 Reviewed-by: Thomas Anderson <thomasanderson@chromium.org> Commit-Queue: Tim Brown <timbrown@chromium.org> Cr-Commit-Position: refs/heads/master@{#539852} [modify] https://crrev.com/6e40caade1e6ae35c34127ca6b4fda7a9eaf3e38/chrome/browser/ui/libgtkui/app_indicator_icon.cc
,
Feb 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bcd0c529a6b5c3512b48cc9aad98dd393199066d commit bcd0c529a6b5c3512b48cc9aad98dd393199066d Author: Dmytro Golovchenko <dgolovchenko@google.com> Date: Wed Feb 28 23:52:32 2018 Revert "Add 32-bit version of libappindicator3-1" This reverts commit 6e3fb99a7265b2481b9570344de4669f4c26a14c. Reason for revert: install-build-deps.sh brocken: The following packages have unmet dependencies: libappindicator3-1 : Conflicts: libappindicator3-1:i386 but 12.10.1+13.10.20130920-0ubuntu4.1 is to be installed libappindicator3-1:i386 : Depends: libindicator3-7:i386 (>= 0.4.90) but it is not going to be installed Conflicts: libappindicator3-1 but 12.10.1+13.10.20130920-0ubuntu4.1 is to be installed E: Unable to correct problems, you have held broken packages. Original change's description: > Add 32-bit version of libappindicator3-1 > > When doing a 32-bit build on a 64-bit system, the 32-bit version of the > library is not available. This CL should fix that. > > Bug: 799144 > Change-Id: I40c6d16d708db102eb4da90ded7bc336cb9f9b09 > Reviewed-on: https://chromium-review.googlesource.com/940196 > Reviewed-by: Dirk Pranke <dpranke@chromium.org> > Commit-Queue: Tim Brown <timbrown@chromium.org> > Cr-Commit-Position: refs/heads/master@{#539851} TBR=dpranke@chromium.org,timbrown@chromium.org Change-Id: Ic6ee1c67eb9c8a5cd438b7f0bc86fbea18125b46 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 799144 Reviewed-on: https://chromium-review.googlesource.com/941614 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Dmytro Golovchenko <dgolovchenko@google.com> Cr-Commit-Position: refs/heads/master@{#539949} [modify] https://crrev.com/bcd0c529a6b5c3512b48cc9aad98dd393199066d/build/install-build-deps.sh
,
Mar 9 2018
Un-cc-ing me from all bugs on my final day.
,
Mar 19 2018
The current state: libappindicator should be being used in all cases. However, we are still using dlopen() to open and use libappindicator. This unfortunately can't change due to an Ubuntu bug (https://bugs.launchpad.net/ubuntu/+source/libappindicator/+bug/1757009) preventing the amd64 and i386 versions of the library being installed at the same time. On user machines, this should be OK, but our CI and build bots need both libraries installed at the same time.
,
Jun 29 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by e...@chromium.org
, Jan 4 2018