From 86af20ded062598fbc71244fd180e3e608954ba6 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 12 Mar 2018 09:33:06 +0900 Subject: [PATCH] Linux named notifications (#12192) * Set name & desktop-entry on Linux notifications * DBusMock now honors verbose mode flag * Disable DBus Notification tests on ia32 --- atom/browser/browser.cc | 3 +- atom/browser/browser.h | 2 - atom/common/linux/application_info.cc | 9 +- .../browser/linux/libnotify_notification.cc | 16 +++ brightray/common/application_info.cc | 4 +- brightray/common/application_info_win.cc | 5 +- script/lib/dbus_mock.py | 15 ++- script/test.py | 1 + spec/api-notification-dbus-spec.js | 122 ++++++++++++++++++ 9 files changed, 163 insertions(+), 14 deletions(-) create mode 100644 spec/api-notification-dbus-spec.js diff --git a/atom/browser/browser.cc b/atom/browser/browser.cc index f16ee5015850..3ba915f91eb3 100644 --- a/atom/browser/browser.cc +++ b/atom/browser/browser.cc @@ -108,14 +108,13 @@ void Browser::SetVersion(const std::string& version) { } std::string Browser::GetName() const { - std::string ret = name_override_; + std::string ret = brightray::GetOverriddenApplicationName(); if (ret.empty()) ret = GetExecutableFileProductName(); return ret; } void Browser::SetName(const std::string& name) { - name_override_ = name; brightray::OverrideApplicationName(name); } diff --git a/atom/browser/browser.h b/atom/browser/browser.h index 0970b26b776b..a8c1a8c5e7c4 100644 --- a/atom/browser/browser.h +++ b/atom/browser/browser.h @@ -273,8 +273,6 @@ class Browser : public WindowListObserver { // The browser is being shutdown. bool is_shutdown_; - std::string name_override_; - int badge_count_ = 0; #if defined(OS_MACOSX) diff --git a/atom/common/linux/application_info.cc b/atom/common/linux/application_info.cc index cff228ba4906..37661ed1750d 100644 --- a/atom/common/linux/application_info.cc +++ b/atom/common/linux/application_info.cc @@ -18,10 +18,15 @@ namespace { GDesktopAppInfo* get_desktop_app_info() { + GDesktopAppInfo * ret = nullptr; + std::unique_ptr env(base::Environment::Create()); const std::string desktop_id = libgtkui::GetDesktopName(env.get()); - return desktop_id.empty() ? nullptr - : g_desktop_app_info_new(desktop_id.c_str()); + const char * libcc_default_id = "chromium-browser.desktop"; + if (!desktop_id.empty() && (desktop_id != libcc_default_id)) + ret = g_desktop_app_info_new(desktop_id.c_str()); + + return ret; } } // namespace diff --git a/brightray/browser/linux/libnotify_notification.cc b/brightray/browser/linux/libnotify_notification.cc index 143a5fdea3eb..c1a72b3cc0ec 100644 --- a/brightray/browser/linux/libnotify_notification.cc +++ b/brightray/browser/linux/libnotify_notification.cc @@ -8,12 +8,14 @@ #include #include +#include "base/environment.h" #include "base/files/file_enumerator.h" #include "base/logging.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "brightray/browser/notification_delegate.h" #include "brightray/common/application_info.h" +#include "chrome/browser/ui/libgtkui/gtk_util.h" #include "chrome/browser/ui/libgtkui/skia_utils_gtk.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -126,6 +128,20 @@ void LibnotifyNotification::Show(const NotificationOptions& options) { notification_, "x-canonical-append", "true"); } + // Send the desktop name to identify the application + // The desktop-entry is the part before the .desktop + std::unique_ptr env(base::Environment::Create()); + std::string desktop_id = libgtkui::GetDesktopName(env.get()); + if (!desktop_id.empty()) { + const std::string suffix{".desktop"}; + if (base::EndsWith(desktop_id, suffix, + base::CompareCase::INSENSITIVE_ASCII)) { + desktop_id.resize(desktop_id.size() - suffix.size()); + } + libnotify_loader_.notify_notification_set_hint_string( + notification_, "desktop-entry", desktop_id.c_str()); + } + GError* error = nullptr; libnotify_loader_.notify_notification_show(notification_, &error); if (error) { diff --git a/brightray/common/application_info.cc b/brightray/common/application_info.cc index 1d3da35d723e..271dd10f81e3 100644 --- a/brightray/common/application_info.cc +++ b/brightray/common/application_info.cc @@ -7,8 +7,9 @@ namespace { std::string g_overridden_application_name; std::string g_overridden_application_version; -} +} // namespace +// name void OverrideApplicationName(const std::string& name) { g_overridden_application_name = name; } @@ -16,6 +17,7 @@ std::string GetOverriddenApplicationName() { return g_overridden_application_name; } +// version void OverrideApplicationVersion(const std::string& version) { g_overridden_application_version = version; } diff --git a/brightray/common/application_info_win.cc b/brightray/common/application_info_win.cc index 08b203a23291..89ea0aceb678 100644 --- a/brightray/common/application_info_win.cc +++ b/brightray/common/application_info_win.cc @@ -47,10 +47,7 @@ PCWSTR GetRawAppUserModelID() { if (SUCCEEDED(GetCurrentProcessExplicitAppUserModelID(¤t_app_id))) { g_app_user_model_id = current_app_id; } else { - std::string name = GetOverriddenApplicationName(); - if (name.empty()) { - name = GetApplicationName(); - } + std::string name = GetApplicationName(); base::string16 generated_app_id = base::ReplaceStringPlaceholders( kAppUserModelIDFormat, base::UTF8ToUTF16(name), nullptr); SetAppUserModelID(generated_app_id); diff --git a/script/lib/dbus_mock.py b/script/lib/dbus_mock.py index 76f39fc26973..00b1fd98d783 100644 --- a/script/lib/dbus_mock.py +++ b/script/lib/dbus_mock.py @@ -1,13 +1,22 @@ +from config import is_verbose_mode from dbusmock import DBusTestCase import atexit +import os +import sys + def cleanup(): DBusTestCase.stop_dbus(DBusTestCase.system_bus_pid) + DBusTestCase.stop_dbus(DBusTestCase.session_bus_pid) atexit.register(cleanup) + +dbusmock_log = sys.stdout if is_verbose_mode() else open(os.devnull, 'w') + DBusTestCase.start_system_bus() -# create a mock for "org.freedesktop.login1" using python-dbusmock -# preconfigured template -(logind_mock, logind) = DBusTestCase.spawn_server_template('logind') +DBusTestCase.spawn_server_template('logind', None, dbusmock_log) + +DBusTestCase.start_session_bus() +DBusTestCase.spawn_server_template('notification_daemon', None, dbusmock_log) diff --git a/script/test.py b/script/test.py index 1f90ed795fcc..cb17102557dc 100755 --- a/script/test.py +++ b/script/test.py @@ -38,6 +38,7 @@ def main(): if args.verbose: enable_verbose_mode() + os.environ['ELECTRON_ENABLE_LOGGING'] = '1' spec_modules = os.path.join(SOURCE_ROOT, 'spec', 'node_modules') if args.rebuild_native_modules or not os.path.isdir(spec_modules): diff --git a/spec/api-notification-dbus-spec.js b/spec/api-notification-dbus-spec.js new file mode 100644 index 000000000000..81c1b68f31d0 --- /dev/null +++ b/spec/api-notification-dbus-spec.js @@ -0,0 +1,122 @@ +// For these tests we use a fake DBus daemon to verify libnotify interaction +// with the session bus. This requires python-dbusmock to be installed and +// running at $DBUS_SESSION_BUS_ADDRESS. +// +// script/test.py spawns dbusmock, which sets DBUS_SESSION_BUS_ADDRESS. +// +// See https://pypi.python.org/pypi/python-dbusmock to read about dbusmock. + +const assert = require('assert') +const dbus = require('dbus-native') +const Promise = require('bluebird') + +const {remote} = require('electron') +const {app} = remote.require('electron') + +const skip = process.platform !== 'linux' || + process.arch === 'ia32' || + !process.env.DBUS_SESSION_BUS_ADDRESS; + +(skip ? describe.skip : describe)('Notification module (dbus)', () => { + let mock, Notification, getCalls, reset + const realAppName = app.getName() + const realAppVersion = app.getVersion() + const appName = 'api-notification-dbus-spec' + const serviceName = 'org.freedesktop.Notifications' + + before(async () => { + // init app + app.setName(appName) + app.setDesktopName(appName + '.desktop') + // init dbus + const path = '/org/freedesktop/Notifications' + const iface = 'org.freedesktop.DBus.Mock' + const bus = dbus.sessionBus() + console.log('session bus: ' + process.env.DBUS_SESSION_BUS_ADDRESS) + const service = bus.getService(serviceName) + const getInterface = Promise.promisify(service.getInterface, {context: service}) + mock = await getInterface(path, iface) + getCalls = Promise.promisify(mock.GetCalls, {context: mock}) + reset = Promise.promisify(mock.Reset, {context: mock}) + }) + + after(async () => { + // cleanup dbus + await reset() + // cleanup app + app.setName(realAppName) + app.setVersion(realAppVersion) + }) + + describe('Notification module using ' + serviceName, () => { + function onMethodCalled (done) { + function cb (name) { + console.log('onMethodCalled: ' + name) + if (name === 'Notify') { + mock.removeListener('MethodCalled', cb) + console.log('done') + done() + } + } + return cb + } + + function unmarshalDBusNotifyHints (dbusHints) { + let o = {} + for (let hint of dbusHints) { + let key = hint[0] + let value = hint[1][1][0] + o[key] = value + } + return o + } + + function unmarshalDBusNotifyArgs (dbusArgs) { + return { + app_name: dbusArgs[0][1][0], + replaces_id: dbusArgs[1][1][0], + app_icon: dbusArgs[2][1][0], + title: dbusArgs[3][1][0], + body: dbusArgs[4][1][0], + actions: dbusArgs[5][1][0], + hints: unmarshalDBusNotifyHints(dbusArgs[6][1][0]) + } + } + + before((done) => { + mock.on('MethodCalled', onMethodCalled(done)) + // lazy load Notification after we listen to MethodCalled mock signal + Notification = require('electron').remote.Notification + const n = new Notification({ + title: 'title', + subtitle: 'subtitle', + body: 'body', + replyPlaceholder: 'replyPlaceholder', + sound: 'sound', + closeButtonText: 'closeButtonText' + }) + n.show() + }) + + it('should call ' + serviceName + ' to show notifications', async () => { + const calls = await getCalls() + assert(calls.length >= 1) + let lastCall = calls[calls.length - 1] + let methodName = lastCall[1] + assert.equal(methodName, 'Notify') + let args = unmarshalDBusNotifyArgs(lastCall[2]) + assert.deepEqual(args, { + app_name: appName, + replaces_id: 0, + app_icon: '', + title: 'title', + body: 'body', + actions: [], + hints: { + 'append': 'true', + 'desktop-entry': appName + } + }) + }) + }) +})