Changeset 228888 in webkit

Timestamp:

Feb 21, 2018 11:42:35 AM (6 years ago)

Author:

Michael Catanzaro

Message:

[GTK] USE_UPOWER causes crashes inside a chroot or on systems with broken dbus/upower
​https://bugs.webkit.org/show_bug.cgi?id=181825

Reviewed by Carlos Garcia Campos.

.:

Get rid of the upower-glib dependency. We will use upower's D-Bus API instead.

• Source/cmake/FindUPowerGLib.cmake: Removed.
• Source/cmake/OptionsGTK.cmake:

Source/WebCore:

We could fix this crash, but that would not be good enough, because upower-glib is a
synchronous API that wraps D-Bus calls. That's not acceptable for use in the web process.
Rewrite LowPowerModeNotifierGLib to use upower's D-Bus API directly, instead.

Note that this also enables LowPowerModeNotifier for WPE, since the USE(UPOWER) build
flag is no longer needed.

• platform/LowPowerModeNotifier.cpp:
• platform/LowPowerModeNotifier.h:
• platform/glib/LowPowerModeNotifierGLib.cpp:

(WebCore::LowPowerModeNotifier::LowPowerModeNotifier):
(WebCore::LowPowerModeNotifier::updateWarningLevel):
(WebCore::LowPowerModeNotifier::warningLevelChanged):
(WebCore::LowPowerModeNotifier::gPropertiesChangedCallback):
(WebCore::LowPowerModeNotifier::~LowPowerModeNotifier):
(WebCore::LowPowerModeNotifier::updateState): Deleted.
(WebCore::LowPowerModeNotifier::warningLevelCallback): Deleted.

Location:

trunk

Files:

• ChangeLog (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/LowPowerModeNotifier.cpp (modified) (1 diff)
• Source/WebCore/platform/LowPowerModeNotifier.h (modified) (2 diffs)
• Source/WebCore/platform/glib/LowPowerModeNotifierGLib.cpp (modified) (3 diffs)
• Source/cmake/FindUPowerGLib.cmake (deleted)
• Source/cmake/OptionsGTK.cmake (modified) (2 diffs)

trunk/ChangeLog

@@ +1 @@
+2018-02-21  Michael Catanzaro  <mcatanzaro@igalia.com>
+
+        [GTK] USE_UPOWER causes crashes inside a chroot or on systems with broken dbus/upower
+        https://bugs.webkit.org/show_bug.cgi?id=181825
+
+        Reviewed by Carlos Garcia Campos.
+
+        Get rid of the upower-glib dependency. We will use upower's D-Bus API instead.
+
+        * Source/cmake/FindUPowerGLib.cmake: Removed.
+        * Source/cmake/OptionsGTK.cmake:
+
 2018-02-20  Adrian Perez de Castro  <aperez@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-02-21  Michael Catanzaro  <mcatanzaro@igalia.com>
+
+        [GTK] USE_UPOWER causes crashes inside a chroot or on systems with broken dbus/upower
+        https://bugs.webkit.org/show_bug.cgi?id=181825
+
+        Reviewed by Carlos Garcia Campos.
+
+        We could fix this crash, but that would not be good enough, because upower-glib is a
+        synchronous API that wraps D-Bus calls. That's not acceptable for use in the web process.
+        Rewrite LowPowerModeNotifierGLib to use upower's D-Bus API directly, instead.
+
+        Note that this also enables LowPowerModeNotifier for WPE, since the USE(UPOWER) build
+        flag is no longer needed.
+
+        * platform/LowPowerModeNotifier.cpp:
+        * platform/LowPowerModeNotifier.h:
+        * platform/glib/LowPowerModeNotifierGLib.cpp:
+        (WebCore::LowPowerModeNotifier::LowPowerModeNotifier):
+        (WebCore::LowPowerModeNotifier::updateWarningLevel):
+        (WebCore::LowPowerModeNotifier::warningLevelChanged):
+        (WebCore::LowPowerModeNotifier::gPropertiesChangedCallback):
+        (WebCore::LowPowerModeNotifier::~LowPowerModeNotifier):
+        (WebCore::LowPowerModeNotifier::updateState): Deleted.
+        (WebCore::LowPowerModeNotifier::warningLevelCallback): Deleted.
+
 2018-02-21  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/platform/LowPowerModeNotifier.cpp

@@ -29 +29 @@
 namespace WebCore {
 
-#if !PLATFORM(IOS) && !USE(UPOWER)
+#if !PLATFORM(IOS) && !USE(GLIB)
 
 LowPowerModeNotifier::LowPowerModeNotifier(LowPowerModeChangeCallback&&)

trunk/Source/WebCore/platform/LowPowerModeNotifier.h

@@ -29 +29 @@
 
 #if PLATFORM(IOS)
-
 #include <wtf/RetainPtr.h>
 OBJC_CLASS WebLowPowerModeObserver;
-
 #endif
 
-#if USE(UPOWER)
-#include <libupower-glib/upower.h>
+#if USE(GLIB)
 #include <wtf/glib/GRefPtr.h>
+typedef _GDBusProxy GDBusProxy;
 #endif
 
@@ -57 +55 @@
     RetainPtr<WebLowPowerModeObserver> m_observer;
     LowPowerModeChangeCallback m_callback;
-#elif USE(UPOWER)
-    static void warningLevelCallback(LowPowerModeNotifier*);
-    void updateState();
+#elif USE(GLIB)
+    void updateWarningLevel();
+    void warningLevelChanged();
+    static void gPropertiesChangedCallback(LowPowerModeNotifier*, GVariant* changedProperties);
 
-    GRefPtr<UpClient> m_upClient;
-    GRefPtr<UpDevice> m_device;
+    GRefPtr<GDBusProxy> m_displayDeviceProxy;
+    GRefPtr<GCancellable> m_cancellable;
     LowPowerModeChangeCallback m_callback;
     bool m_lowPowerModeEnabled { false };

trunk/Source/WebCore/platform/glib/LowPowerModeNotifierGLib.cpp

@@ -1 +1 @@
 /*
  *  Copyright (C) 2017 Collabora Ltd.
+ *  Copyright (C) 2018 Igalia S.L.
  *
  *  This library is free software; you can redistribute it and/or
@@ -20 +21 @@
 #include "LowPowerModeNotifier.h"
 
-#if USE(UPOWER)
+#include <cstring>
+#include <gio/gio.h>
+
 namespace WebCore {
 
+static const char kWarningLevel[] = "WarningLevel";
+
 LowPowerModeNotifier::LowPowerModeNotifier(LowPowerModeChangeCallback&& callback)
-    : m_upClient(adoptGRef(up_client_new()))
-    , m_device(adoptGRef(up_client_get_display_device(m_upClient.get())))
+    : m_cancellable(adoptGRef(g_cancellable_new()))
     , m_callback(WTFMove(callback))
 {
-    updateState();
+    g_dbus_proxy_new_for_bus(G_BUS_TYPE_SYSTEM, static_cast<GDBusProxyFlags>(G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS | G_DBUS_PROXY_FLAGS_GET_INVALIDATED_PROPERTIES),
+        nullptr, "org.freedesktop.UPower", "/org/freedesktop/UPower/devices/DisplayDevice", "org.freedesktop.UPower.Device", m_cancellable.get(),
+        [](GObject*, GAsyncResult* result, gpointer userData) {
+            GUniqueOutPtr<GError> error;
+            GRefPtr<GDBusProxy> proxy = adoptGRef(g_dbus_proxy_new_for_bus_finish(result, &error.outPtr()));
+            if (g_error_matches(error.get(), G_IO_ERROR, G_IO_ERROR_CANCELLED))
+                return;
 
-    g_signal_connect_swapped(m_device.get(), "notify::warning-level", G_CALLBACK(warningLevelCallback), this);
+            auto* self = static_cast<LowPowerModeNotifier*>(userData);
+            if (proxy) {
+                GUniquePtr<char> nameOwner(g_dbus_proxy_get_name_owner(proxy.get()));
+                if (nameOwner) {
+                    self->m_displayDeviceProxy = WTFMove(proxy);
+                    self->updateWarningLevel();
+                    g_signal_connect_swapped(self->m_displayDeviceProxy.get(), "g-properties-changed", G_CALLBACK(gPropertiesChangedCallback), self);
+                    return;
+                }
+            }
+
+            // Now, if there is no name owner, it would be good to try to
+            // connect to a Flatpak battery status portal instead.
+            // Unfortunately, no such portal currently exists.
+            self->m_cancellable = nullptr;
+    }, this);
 }
 
-void LowPowerModeNotifier::updateState()
+void LowPowerModeNotifier::updateWarningLevel()
 {
-    UpDeviceLevel warningLevel;
-    g_object_get(G_OBJECT(m_device.get()), "warning-level", &warningLevel, nullptr);
-    m_lowPowerModeEnabled = warningLevel > UP_DEVICE_LEVEL_NONE && warningLevel <= UP_DEVICE_LEVEL_ACTION;
+    GRefPtr<GVariant> variant = adoptGRef(g_dbus_proxy_get_cached_property(m_displayDeviceProxy.get(), kWarningLevel));
+    if (!variant) {
+        m_lowPowerModeEnabled = false;
+        return;
+    }
+
+    // 0: Unknown
+    // 1: None
+    // 2: Discharging (only for universal power supplies)
+    // 3: Low
+    // 4: Critical
+    // 5: Action
+    m_lowPowerModeEnabled = g_variant_get_uint32(variant.get()) > 1;
 }
 
-void LowPowerModeNotifier::warningLevelCallback(LowPowerModeNotifier* notifier)
+void LowPowerModeNotifier::warningLevelChanged()
 {
-    notifier->updateState();
-    notifier->m_callback(notifier->m_lowPowerModeEnabled);
+    updateWarningLevel();
+    m_callback(m_lowPowerModeEnabled);
+}
+
+void LowPowerModeNotifier::gPropertiesChangedCallback(LowPowerModeNotifier* self, GVariant* changedProperties)
+{
+    GUniqueOutPtr<GVariantIter> iter;
+    g_variant_get(changedProperties, "a{sv}", &iter.outPtr());
+
+    const char* propertyName;
+    while (g_variant_iter_next(iter.get(), "{&sv}", &propertyName, nullptr)) {
+        if (!strcmp(propertyName, kWarningLevel)) {
+            self->warningLevelChanged();
+            break;
+        }
+    }
 }
 
 LowPowerModeNotifier::~LowPowerModeNotifier()
 {
-    g_signal_handlers_disconnect_by_data(m_device.get(), this);
+    g_cancellable_cancel(m_cancellable.get());
 }
 
@@ -56 +105 @@
 }
 
-}
-#endif
+} // namespace WebCore

trunk/Source/cmake/OptionsGTK.cmake

@@ -82 +82 @@
 WEBKIT_OPTION_DEFINE(USE_LIBHYPHEN "Whether to enable the default automatic hyphenation implementation." PUBLIC ON)
 WEBKIT_OPTION_DEFINE(USE_LIBSECRET "Whether to enable the persistent credential storage using libsecret." PUBLIC ON)
-WEBKIT_OPTION_DEFINE(USE_UPOWER "Whether to enable the low power mode implementation." PUBLIC ON)
 WEBKIT_OPTION_DEFINE(USE_WOFF2 "Whether to enable support for WOFF2 Web Fonts." PUBLIC ON)
 
@@ -342 +341 @@
 endif ()
 
-if (USE_UPOWER)
-    find_package(UPowerGLib)
-    if (NOT UPOWERGLIB_FOUND)
-       message(FATAL_ERROR "upower-glib is needed for USE_UPOWER.")
-    endif ()
-endif ()
-
 if (USE_WOFF2)
     find_package(WOFF2Dec 1.0.2)