Changeset 206985 in webkit

Timestamp:

Oct 10, 2016 12:12:35 AM (8 years ago)

Author:

commit-queue@webkit.org

Message:

[GTK] UIProcess crashes when using Japanese IM
​https://bugs.webkit.org/show_bug.cgi?id=163011

We have to reference the current GdkEventKey before we try process it
as later when the lambda body is reached the event could be already
freed.

Patch by Tomas Popela <​tpopela@redhat.com> on 2016-10-10
Reviewed by Carlos Garcia Campos.

• UIProcess/API/gtk/WebKitWebViewBase.cpp:

(webkitWebViewBaseKeyPressEvent):
(webkitWebViewBaseKeyReleaseEvent):

• UIProcess/gtk/InputMethodFilter.h:

Use non-copyable Function so we can use WTFMove to pass the event to
lambda.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/gtk/WebKitWebViewBase.cpp (modified) (5 diffs)
• UIProcess/gtk/InputMethodFilter.h (modified) (2 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2016-10-10  Tomas Popela  <tpopela@redhat.com>
+
+        [GTK] UIProcess crashes when using Japanese IM
+        https://bugs.webkit.org/show_bug.cgi?id=163011
+
+        We have to reference the current GdkEventKey before we try process it
+        as later when the lambda body is reached the event could be already
+        freed.
+
+        Reviewed by Carlos Garcia Campos.
+
+        * UIProcess/API/gtk/WebKitWebViewBase.cpp:
+        (webkitWebViewBaseKeyPressEvent):
+        (webkitWebViewBaseKeyReleaseEvent):
+        * UIProcess/gtk/InputMethodFilter.h:
+        Use non-copyable Function so we can use WTFMove to pass the event to
+        lambda.
+
 2016-10-09  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp

@@ -675 +675 @@
 }
 
-static gboolean webkitWebViewBaseKeyPressEvent(GtkWidget* widget, GdkEventKey* event)
+static gboolean webkitWebViewBaseKeyPressEvent(GtkWidget* widget, GdkEventKey* keyEvent)
 {
     WebKitWebViewBase* webViewBase = WEBKIT_WEB_VIEW_BASE(widget);
@@ -681 +681 @@
 
     if (priv->authenticationDialog)
-        return GTK_WIDGET_CLASS(webkit_web_view_base_parent_class)->key_press_event(widget, event);
+        return GTK_WIDGET_CLASS(webkit_web_view_base_parent_class)->key_press_event(widget, keyEvent);
 
 #if ENABLE(FULLSCREEN_API)
     if (priv->fullScreenModeActive) {
-        switch (event->keyval) {
+        switch (keyEvent->keyval) {
         case GDK_KEY_Escape:
         case GDK_KEY_f:
@@ -703 +703 @@
     if (priv->shouldForwardNextKeyEvent) {
         priv->shouldForwardNextKeyEvent = FALSE;
-        return GTK_WIDGET_CLASS(webkit_web_view_base_parent_class)->key_press_event(widget, event);
-    }
-
-    priv->inputMethodFilter.filterKeyEvent(event, [priv, event](const WebCore::CompositionResults& compositionResults, InputMethodFilter::EventFakedForComposition faked) {
-        priv->pageProxy->handleKeyboardEvent(NativeWebKeyboardEvent(reinterpret_cast<GdkEvent*>(event), compositionResults, faked,
-            !compositionResults.compositionUpdated() ? priv->keyBindingTranslator.commandsForKeyEvent(event) : Vector<String>()));
+        return GTK_WIDGET_CLASS(webkit_web_view_base_parent_class)->key_press_event(widget, keyEvent);
+    }
+
+    // We need to copy the event as otherwise it could be destroyed before we reach the lambda body.
+    GUniquePtr<GdkEvent> event(gdk_event_copy(reinterpret_cast<GdkEvent*>(keyEvent)));
+    priv->inputMethodFilter.filterKeyEvent(keyEvent, [priv, event = WTFMove(event)](const WebCore::CompositionResults& compositionResults, InputMethodFilter::EventFakedForComposition faked) {
+        priv->pageProxy->handleKeyboardEvent(NativeWebKeyboardEvent(event.get(), compositionResults, faked,
+            !compositionResults.compositionUpdated() ? priv->keyBindingTranslator.commandsForKeyEvent(&event->key) : Vector<String>()));
     });
 
@@ -714 +716 @@
 }
 
-static gboolean webkitWebViewBaseKeyReleaseEvent(GtkWidget* widget, GdkEventKey* event)
+static gboolean webkitWebViewBaseKeyReleaseEvent(GtkWidget* widget, GdkEventKey* keyEvent)
 {
     WebKitWebViewBase* webViewBase = WEBKIT_WEB_VIEW_BASE(widget);
@@ -721 +723 @@
     if (priv->shouldForwardNextKeyEvent) {
         priv->shouldForwardNextKeyEvent = FALSE;
-        return GTK_WIDGET_CLASS(webkit_web_view_base_parent_class)->key_release_event(widget, event);
-    }
-
-    priv->inputMethodFilter.filterKeyEvent(event, [priv, event](const WebCore::CompositionResults& compositionResults, InputMethodFilter::EventFakedForComposition faked) {
-        priv->pageProxy->handleKeyboardEvent(NativeWebKeyboardEvent(reinterpret_cast<GdkEvent*>(event), compositionResults, faked, { }));
+        return GTK_WIDGET_CLASS(webkit_web_view_base_parent_class)->key_release_event(widget, keyEvent);
+    }
+
+    // We need to copy the event as otherwise it could be destroyed before we reach the lambda body.
+    GUniquePtr<GdkEvent> event(gdk_event_copy(reinterpret_cast<GdkEvent*>(keyEvent)));
+    priv->inputMethodFilter.filterKeyEvent(keyEvent, [priv, event = WTFMove(event)](const WebCore::CompositionResults& compositionResults, InputMethodFilter::EventFakedForComposition faked) {
+        priv->pageProxy->handleKeyboardEvent(NativeWebKeyboardEvent(event.get(), compositionResults, faked, { }));
     });
 

trunk/Source/WebKit2/UIProcess/gtk/InputMethodFilter.h

@@ -22 +22 @@
 
 #include <WebCore/IntPoint.h>
-#include <functional>
+#include <wtf/Function.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/glib/GRefPtr.h>
@@ -57 +57 @@
     void setCursorRect(const WebCore::IntRect&);
 
-    using FilterKeyEventCompletionHandler = std::function<void (const WebCore::CompositionResults&, InputMethodFilter::EventFakedForComposition)>;
+    using FilterKeyEventCompletionHandler = Function<void(const WebCore::CompositionResults&, InputMethodFilter::EventFakedForComposition)>;
     void filterKeyEvent(GdkEventKey*, FilterKeyEventCompletionHandler&& = nullptr);
     void notifyFocusedIn();