Changeset 58948 in webkit

Timestamp:

May 7, 2010 8:04:58 AM (14 years ago)

Author:

eric@webkit.org

Message:

2010-05-07 Ben Murdoch <​benm@google.com>

Reviewed by Darin Adler.

Potential crash in EventHandler::handleTouchEvent
​https://bugs.webkit.org/show_bug.cgi?id=38646

Fix a ref counting bug that can cause a crash if the m_originatingouchPointTargets
hashmap holds the last ref to an EventTarget when the user lifts their finger.

This is very hard to reproduce in a consistent way and clearly a
simple logic error in the code, therefore no new tests.

• page/EventHandler.cpp: (WebCore::EventHandler::handleTouchEvent): Don't let the RefPtr we get back from

the hasmap go out of scope so soon as it could delete the wrapped ptr if the
hashmap held the last ref (and we use the raw ptr that the RefPtr
wraps later in the WebCore::Touch constructor).

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/EventHandler.cpp (modified) (2 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-05-07  Ben Murdoch  <benm@google.com>
+
+        Reviewed by Darin Adler.
+
+        Potential crash in EventHandler::handleTouchEvent
+        https://bugs.webkit.org/show_bug.cgi?id=38646
+
+        Fix a ref counting bug that can cause a crash if the m_originatingouchPointTargets
+        hashmap holds the last ref to an EventTarget when the user lifts their finger.
+
+        This is very hard to reproduce in a consistent way and clearly a
+        simple logic error in the code, therefore no new tests.
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::handleTouchEvent): Don't let the RefPtr we get back from
+            the hasmap go out of scope so soon as it could delete the wrapped ptr if the
+            hashmap held the last ref (and we use the raw ptr that the RefPtr
+            wraps later in the WebCore::Touch constructor).
+
 2010-05-07  Gustavo Noronha Silva  <gustavo.noronha@collabora.co.uk>
 

trunk/WebCore/page/EventHandler.cpp

@@ -2783 +2783 @@
         // Increment the platform touch id by 1 to avoid storing a key of 0 in the hashmap.
         unsigned touchPointTargetKey = point.id() + 1;
-        EventTarget* touchTarget = 0;
+        RefPtr<EventTarget> touchTarget;
         if (point.state() == PlatformTouchPoint::TouchPressed) {
             m_originatingTouchPointTargets.set(touchPointTargetKey, target);
@@ -2790 +2790 @@
             // The target should be the original target for this touch, so get it from the hashmap. As it's a release or cancel
             // we also remove it from the map.
-            touchTarget = m_originatingTouchPointTargets.take(touchPointTargetKey).get();
+            touchTarget = m_originatingTouchPointTargets.take(touchPointTargetKey);
         } else
-            touchTarget = m_originatingTouchPointTargets.get(touchPointTargetKey).get();
-
-        if (!touchTarget)
+            touchTarget = m_originatingTouchPointTargets.get(touchPointTargetKey);
+
+        if (!touchTarget.get())
             continue;
 
-        RefPtr<Touch> touch = Touch::create(doc->frame(), touchTarget, point.id(),
+        RefPtr<Touch> touch = Touch::create(doc->frame(), touchTarget.get(), point.id(),
                                             point.screenPos().x(), point.screenPos().y(),
                                             adjustedPageX, adjustedPageY);