Changeset 211145 in webkit

Timestamp:

Jan 25, 2017, 8:54:28 AM (10 years ago)

Author:

Carlos Garcia Campos

Message:

[GTK] UIProcess from WebKitGtk+ 2.15.x SIGSEGVs because of X Error BadDamage in WebKit::AcceleratedBackingStoreX11::update(WebKit::LayerTreeContext const&) () at Source/WebKit2/UIProcess/gtk/AcceleratedBackingStoreX11.cpp:145
​https://bugs.webkit.org/show_bug.cgi?id=165656

Reviewed by Michael Catanzaro.

Source/WebCore:

Also return the base error code from PlatformDisplayX11::supportsXDamage().

• platform/graphics/x11/PlatformDisplayX11.cpp:

(WebCore::PlatformDisplayX11::supportsXDamage):

• platform/graphics/x11/PlatformDisplayX11.h:

Source/WebKit2:

We are incorrectly handling BadDamage errors because the BadDamage value we pass to the XErrorTrapper is not
the actual error code used by X11. Since XDamage is an extension, it has its own errors and a base error
code. We need to use the base error code we get when calling XDamageQueryExtension to pass the right error code
to the XErrorTrapper.

• UIProcess/gtk/AcceleratedBackingStoreX11.cpp:

(WebKit::AcceleratedBackingStoreX11::create): Get also the damage base error.
(WebKit::xDamageErrorCode): Helper to get the actual error code.
(WebKit::AcceleratedBackingStoreX11::~AcceleratedBackingStoreX11): Use xDamageErrorCode().
(WebKit::AcceleratedBackingStoreX11::update): Ditto.

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/platform/graphics/x11/PlatformDisplayX11.cpp (modified) (3 diffs)
• WebCore/platform/graphics/x11/PlatformDisplayX11.h (modified) (2 diffs)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/UIProcess/gtk/AcceleratedBackingStoreX11.cpp (modified) (6 diffs)
• WebKit2/UIProcess/gtk/WebPreferencesGtk.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-01-25  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] UIProcess from WebKitGtk+ 2.15.x SIGSEGVs because of X Error BadDamage in WebKit::AcceleratedBackingStoreX11::update(WebKit::LayerTreeContext const&) () at Source/WebKit2/UIProcess/gtk/AcceleratedBackingStoreX11.cpp:145
+        https://bugs.webkit.org/show_bug.cgi?id=165656
+
+        Reviewed by Michael Catanzaro.
+
+        Also return the base error code from PlatformDisplayX11::supportsXDamage().
+
+        * platform/graphics/x11/PlatformDisplayX11.cpp:
+        (WebCore::PlatformDisplayX11::supportsXDamage):
+        * platform/graphics/x11/PlatformDisplayX11.h:
+
 2017-01-25  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.cpp

@@ -99 +99 @@
 }
 
-bool PlatformDisplayX11::supportsXDamage(std::optional<int>& damageEventBase) const
+bool PlatformDisplayX11::supportsXDamage(std::optional<int>& damageEventBase, std::optional<int>& damageErrorBase) const
 {
     if (!m_supportsXDamage) {
@@ -107 +107 @@
             int eventBase, errorBase;
             m_supportsXDamage = XDamageQueryExtension(m_display, &eventBase, &errorBase);
-            if (m_supportsXDamage.value())
+            if (m_supportsXDamage.value()) {
                 m_damageEventBase = eventBase;
+                m_damageErrorBase = errorBase;
+            }
         }
 #endif
@@ -114 +116 @@
 
     damageEventBase = m_damageEventBase;
+    damageErrorBase = m_damageErrorBase;
     return m_supportsXDamage.value();
 }

trunk/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.h

@@ -44 +44 @@
     Display* native() const { return m_display; }
     bool supportsXComposite() const;
-    bool supportsXDamage(std::optional<int>& damageEventBase) const;
+    bool supportsXDamage(std::optional<int>& damageEventBase, std::optional<int>& damageErrorBase) const;
 
 private:
@@ -57 +57 @@
     mutable std::optional<bool> m_supportsXDamage;
     mutable std::optional<int> m_damageEventBase;
+    mutable std::optional<int> m_damageErrorBase;
 };
 

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2017-01-25  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] UIProcess from WebKitGtk+ 2.15.x SIGSEGVs because of X Error BadDamage in WebKit::AcceleratedBackingStoreX11::update(WebKit::LayerTreeContext const&) () at Source/WebKit2/UIProcess/gtk/AcceleratedBackingStoreX11.cpp:145
+        https://bugs.webkit.org/show_bug.cgi?id=165656
+
+        Reviewed by Michael Catanzaro.
+
+        We are incorrectly handling BadDamage errors because the BadDamage value we pass to the XErrorTrapper is not
+        the actual error code used by X11. Since XDamage is an extension, it has its own errors and a base error
+        code. We need to use the base error code we get when calling XDamageQueryExtension to pass the right error code
+        to the XErrorTrapper.
+
+        * UIProcess/gtk/AcceleratedBackingStoreX11.cpp:
+        (WebKit::AcceleratedBackingStoreX11::create): Get also the damage base error.
+        (WebKit::xDamageErrorCode): Helper to get the actual error code.
+        (WebKit::AcceleratedBackingStoreX11::~AcceleratedBackingStoreX11): Use xDamageErrorCode().
+        (WebKit::AcceleratedBackingStoreX11::update): Ditto.
+
 2017-01-25  Miguel Gomez  <magomez@igalia.com>
 

trunk/Source/WebKit2/UIProcess/gtk/AcceleratedBackingStoreX11.cpp

@@ -48 +48 @@
 
 static std::optional<int> s_damageEventBase;
+static std::optional<int> s_damageErrorBase;
 
 class XDamageNotifier {
@@ -106 +107 @@
 {
     auto& display = downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay());
-    if (!display.supportsXComposite() || !display.supportsXDamage(s_damageEventBase))
+    if (!display.supportsXComposite() || !display.supportsXDamage(s_damageEventBase, s_damageErrorBase))
         return nullptr;
     return std::unique_ptr<AcceleratedBackingStoreX11>(new AcceleratedBackingStoreX11(webPage));
@@ -116 +117 @@
 }
 
+static inline unsigned char xDamageErrorCode(unsigned char errorCode)
+{
+    ASSERT(s_damageErrorBase);
+    return static_cast<unsigned>(s_damageErrorBase.value()) + errorCode;
+}
+
 AcceleratedBackingStoreX11::~AcceleratedBackingStoreX11()
 {
@@ -122 +129 @@
 
     Display* display = downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native();
-    XErrorTrapper trapper(display, XErrorTrapper::Policy::Crash, { BadDrawable, BadDamage });
+    XErrorTrapper trapper(display, XErrorTrapper::Policy::Crash, { BadDrawable, xDamageErrorCode(BadDamage) });
     if (m_damage) {
         XDamageNotifier::singleton().remove(m_damage.get());
@@ -139 +146 @@
 
     if (m_surface) {
-        XErrorTrapper trapper(display, XErrorTrapper::Policy::Crash, { BadDrawable, BadDamage });
+        XErrorTrapper trapper(display, XErrorTrapper::Policy::Crash, { BadDrawable, xDamageErrorCode(BadDamage) });
         if (m_damage) {
             XDamageNotifier::singleton().remove(m_damage.get());
@@ -159 +166 @@
     size.scale(deviceScaleFactor);
 
-    XErrorTrapper trapper(display, XErrorTrapper::Policy::Crash, { BadDrawable, BadDamage });
+    XErrorTrapper trapper(display, XErrorTrapper::Policy::Crash, { BadDrawable, xDamageErrorCode(BadDamage) });
     ASSERT(downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay()).native() == GDK_DISPLAY_XDISPLAY(gdk_display_get_default()));
     GdkVisual* visual = gdk_screen_get_rgba_visual(gdk_screen_get_default());

trunk/Source/WebKit2/UIProcess/gtk/WebPreferencesGtk.cpp

@@ -62 +62 @@
     if (PlatformDisplay::sharedDisplay().type() == PlatformDisplay::Type::X11) {
         auto& display = downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay());
-        std::optional<int> damageBase;
-        if (!display.supportsXComposite() || !display.supportsXDamage(damageBase))
+        std::optional<int> damageBase, errorBase;
+        if (!display.supportsXComposite() || !display.supportsXDamage(damageBase, errorBase))
             setAcceleratedCompositingEnabled(false);
     }