[Chromium] Clear m_currentInputEvent after handled by pointerLockMouseEvent().
authorscheib@chromium.org <scheib@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Jul 2012 16:21:43 +0000 (16:21 +0000)
committerscheib@chromium.org <scheib@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Jul 2012 16:21:43 +0000 (16:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=90391

Source/WebKit/chromium:

WebViewImpl::handleInputEvent was keeping a pointer to an input event that would
later be accessed. When in pointer lock, that pointer was not being cleared.
Code modified to use TemporaryChange to automatically clear the pointer at all
method exit points.

Reviewed by Abhishek Arya.

* src/WebViewImpl.cpp:
(WebKit::WebViewImpl::handleInputEvent):

LayoutTests:

Test that reproduces bug 90391:
Enable pointer lock, receive mouse move, call window.open, don't crash.

Reviewed by Abhishek Arya.

* pointer-lock/bug90391-move-then-window-open-crash-expected.txt: Added.
* pointer-lock/bug90391-move-then-window-open-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@121909 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/pointer-lock/bug90391-move-then-window-open-crash-expected.txt [new file with mode: 0644]
LayoutTests/pointer-lock/bug90391-move-then-window-open-crash.html [new file with mode: 0644]
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/WebViewImpl.cpp

index 2dedb6c..a507a69 100644 (file)
@@ -1,3 +1,16 @@
+2012-07-05  Vincent Scheib  <scheib@chromium.org>
+
+        [Chromium] Clear m_currentInputEvent after handled by pointerLockMouseEvent().
+        https://bugs.webkit.org/show_bug.cgi?id=90391
+
+        Test that reproduces bug 90391:
+        Enable pointer lock, receive mouse move, call window.open, don't crash.
+
+        Reviewed by Abhishek Arya.
+
+        * pointer-lock/bug90391-move-then-window-open-crash-expected.txt: Added.
+        * pointer-lock/bug90391-move-then-window-open-crash.html: Added.
+
 2012-07-05  John Mellor  <johnme@chromium.org>
 
         Text Autosizing: Add test framework and simple test.
diff --git a/LayoutTests/pointer-lock/bug90391-move-then-window-open-crash-expected.txt b/LayoutTests/pointer-lock/bug90391-move-then-window-open-crash-expected.txt
new file mode 100644 (file)
index 0000000..0d1035c
--- /dev/null
@@ -0,0 +1,15 @@
+bug 90391: pointer lock mouse move events then window.open should not crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+     Locking targetdiv1.
+PASS document.onwebkitpointerlockchange event received.
+     Sending mouse move events.
+     Calling window.open.
+PASS Didn't crash
+PASS successfullyParsed is true
+
+TEST COMPLETE
+doNextStep for manual testing
+
diff --git a/LayoutTests/pointer-lock/bug90391-move-then-window-open-crash.html b/LayoutTests/pointer-lock/bug90391-move-then-window-open-crash.html
new file mode 100644 (file)
index 0000000..ccf4131
--- /dev/null
@@ -0,0 +1,64 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<script src="../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<div>
+  <button onclick="doNextStep('manual');">doNextStep for manual testing</button>
+  <div id="target1"></div>
+</div>
+<script>
+    description("bug 90391: pointer lock mouse move events then window.open should not crash.")
+    window.jsTestIsAsync = true;
+
+    targetdiv1 = document.getElementById("target1");
+
+    currentStep = 0;
+    function doNextStep(manual)
+    {
+        if (!window.layoutTestController && !manual)
+            return;
+        if (currentStep < todo.length)
+            setTimeout(function () { todo[currentStep++](); }, 0);
+        else if (currentStep++ == todo.length)
+            setTimeout(function () { finishJSTest(); }, 0);
+    }
+    todo = [
+        function () {
+            debug("     Locking targetdiv1.")
+            targetdiv1.webkitRequestPointerLock();
+            document.onwebkitpointerlockchange = function () {
+                document.onwebkitpointerlockchange = null;
+                testPassed("document.onwebkitpointerlockchange event received.");
+                doNextStep('manual');
+            };
+        },
+        function () {
+            debug("     Sending mouse move events.")
+            var mouseMoveEvents = 0;
+            targetdiv1.onmousemove = function () {
+                if (++mouseMoveEvents == 2) {
+                    targetdiv1.onmousemove = null;
+                    doNextStep('manual');
+                }
+            }
+            if (window.eventSender) {
+              eventSender.mouseMoveTo(100, 100);
+              eventSender.mouseMoveTo(200, 200);
+            }
+        },
+        function () {
+            debug("     Calling window.open.")
+            gc();
+            window.open();
+            testPassed("Didn't crash");
+            document.webkitExitPointerLock();
+            doNextStep('manual');
+        },
+    ];
+    doNextStep();
+</script>
+<script src="../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index 8c251f9..bccd0e5 100644 (file)
@@ -1,3 +1,18 @@
+2012-07-05  Vincent Scheib  <scheib@chromium.org>
+
+        [Chromium] Clear m_currentInputEvent after handled by pointerLockMouseEvent().
+        https://bugs.webkit.org/show_bug.cgi?id=90391
+
+        WebViewImpl::handleInputEvent was keeping a pointer to an input event that would
+        later be accessed. When in pointer lock, that pointer was not being cleared.
+        Code modified to use TemporaryChange to automatically clear the pointer at all
+        method exit points.
+
+        Reviewed by Abhishek Arya.
+
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::handleInputEvent):
+
 2012-07-05  John Mellor  <johnme@chromium.org>
 
         Text Autosizing: Add test framework and simple test.
index b3fddef..8368d1c 100644 (file)
 #include <wtf/CurrentTime.h>
 #include <wtf/MainThread.h>
 #include <wtf/RefPtr.h>
+#include <wtf/TemporaryChange.h>
 #include <wtf/Uint8ClampedArray.h>
 
 #if ENABLE(GESTURE_EVENTS)
@@ -1760,7 +1761,7 @@ bool WebViewImpl::handleInputEvent(const WebInputEvent& inputEvent)
     if (m_ignoreInputEvents)
         return false;
 
-    m_currentInputEvent = &inputEvent;
+    TemporaryChange<const WebInputEvent*>(m_currentInputEvent, &inputEvent);
 
 #if ENABLE(POINTER_LOCK)
     if (isPointerLocked() && WebInputEvent::isMouseEventType(inputEvent.type)) {
@@ -1798,12 +1799,10 @@ bool WebViewImpl::handleInputEvent(const WebInputEvent& inputEvent)
         node->dispatchMouseEvent(
               PlatformMouseEventBuilder(mainFrameImpl()->frameView(), *static_cast<const WebMouseEvent*>(&inputEvent)),
               eventType, static_cast<const WebMouseEvent*>(&inputEvent)->clickCount);
-        m_currentInputEvent = 0;
         return true;
     }
 
     bool handled = PageWidgetDelegate::handleInputEvent(m_page.get(), *this, inputEvent);
-    m_currentInputEvent = 0;
     return handled;
 }