[Chromium] WebPluginContainerImpl adding imbalanced touch handler refs
authorleviw@chromium.org <leviw@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 31 Jan 2013 02:15:03 +0000 (02:15 +0000)
committerleviw@chromium.org <leviw@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 31 Jan 2013 02:15:03 +0000 (02:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=108381

Reviewed by James Robinson.

Source/WebKit/chromium:

WebPluginContainerImpl would call Document::didAddTouchEventHandler every time the plugin requested
touch events. Some plugins make this request more than once, leading to an imbalance in Document's
touch event handler map, and a stale node pointer when the plugin is destroyed. This change
has WebPluginContainerImpl only add one ref for the plugin at a time.

* src/WebPluginContainerImpl.cpp:
(WebKit::WebPluginContainerImpl::requestTouchEventType):

Tools:

* DumpRenderTree/chromium/TestRunner/src/WebTestPlugin.cpp: Adding an attribute that
tickles the case in WebPluginContainerImpl that imbalanced touch handler refs in Document.

LayoutTests:

* platform/chromium/plugins/re-request-touch-events-crash-expected.txt: Added.
* platform/chromium/plugins/re-request-touch-events-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/chromium/plugins/re-request-touch-events-crash-expected.txt [new file with mode: 0644]
LayoutTests/platform/chromium/plugins/re-request-touch-events-crash.html [new file with mode: 0644]
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/WebPluginContainerImpl.cpp
Tools/ChangeLog
Tools/DumpRenderTree/chromium/TestRunner/src/WebTestPlugin.cpp

index 1707fc0..c91a10f 100644 (file)
@@ -1,3 +1,13 @@
+2013-01-30  Levi Weintraub  <leviw@chromium.org>
+
+        [Chromium] WebPluginContainerImpl adding imbalanced touch handler refs
+        https://bugs.webkit.org/show_bug.cgi?id=108381
+
+        Reviewed by James Robinson.
+
+        * platform/chromium/plugins/re-request-touch-events-crash-expected.txt: Added.
+        * platform/chromium/plugins/re-request-touch-events-crash.html: Added.
+
 2013-01-30  Rouslan Solomakhin  <rouslan@chromium.org>
 
         Tests for spellcheck behavior
diff --git a/LayoutTests/platform/chromium/plugins/re-request-touch-events-crash-expected.txt b/LayoutTests/platform/chromium/plugins/re-request-touch-events-crash-expected.txt
new file mode 100644 (file)
index 0000000..7562698
--- /dev/null
@@ -0,0 +1,5 @@
+PASS window.internals.touchEventHandlerCount(document) is 1
+PASS window.internals.touchEventHandlerCount(document) is 0
+This test ensures that we don't imbalance the touch event handler count if a plugin requests touch events more than once. The test passes if there are no touch event listeners after the plugin is removed, and there is no crash. Must be run in DRT.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
diff --git a/LayoutTests/platform/chromium/plugins/re-request-touch-events-crash.html b/LayoutTests/platform/chromium/plugins/re-request-touch-events-crash.html
new file mode 100644 (file)
index 0000000..0a677e4
--- /dev/null
@@ -0,0 +1,25 @@
+<html>
+<head>
+<script src="../../../fast/js/resources/js-test-pre.js"></script>
+</head>
+
+<body>
+<embed id="touch_plugin" type="application/x-webkit-test-webplugin" accepts-touch="raw" re-request-touch="true"></embed>
+<p id="description"></p>
+<script>
+description("This test ensures that we don't imbalance the touch event handler count if a plugin requests touch events more than once. The test passes if there are no touch event listeners after the plugin is removed, and there is no crash. Must be run in DRT.");
+
+if (window.testRunner) {
+    testRunner.dumpAsText();
+
+    // Force the plugin to initialize
+    touch_plugin.offsetTop;
+
+    shouldBe("window.internals.touchEventHandlerCount(document)", "1")
+    touch_plugin.parentNode.removeChild(touch_plugin);
+    shouldBe("window.internals.touchEventHandlerCount(document)", "0")
+}
+
+</script>
+</body>
+<script src="../../fast/js/resources/js-test-post.js"></script>
index eca0ddf..81a6c7c 100644 (file)
@@ -1,3 +1,18 @@
+2013-01-30  Levi Weintraub  <leviw@chromium.org>
+
+        [Chromium] WebPluginContainerImpl adding imbalanced touch handler refs
+        https://bugs.webkit.org/show_bug.cgi?id=108381
+
+        Reviewed by James Robinson.
+
+        WebPluginContainerImpl would call Document::didAddTouchEventHandler every time the plugin requested
+        touch events. Some plugins make this request more than once, leading to an imbalance in Document's
+        touch event handler map, and a stale node pointer when the plugin is destroyed. This change
+        has WebPluginContainerImpl only add one ref for the plugin at a time.
+
+        * src/WebPluginContainerImpl.cpp:
+        (WebKit::WebPluginContainerImpl::requestTouchEventType):
+
 2013-01-30  Yusuf Ozuysal  <yusufo@google.com>
 
         Start sending scrollType as NonBubblingGesture for flings
index a768c20..b89bcd1 100644 (file)
@@ -473,11 +473,12 @@ void WebPluginContainerImpl::requestTouchEventType(TouchEventRequestType request
 {
     if (m_touchEventRequestType == requestType)
         return;
-    m_touchEventRequestType = requestType;
-    if (m_touchEventRequestType != TouchEventRequestTypeNone)
+    
+    if (requestType != TouchEventRequestTypeNone && m_touchEventRequestType == TouchEventRequestTypeNone)
         m_element->document()->didAddTouchEventHandler(m_element);
-    else
+    else if (requestType == TouchEventRequestTypeNone && m_touchEventRequestType != TouchEventRequestTypeNone)
         m_element->document()->didRemoveTouchEventHandler(m_element);
+    m_touchEventRequestType = requestType;
 }
 
 void WebPluginContainerImpl::setWantsWheelEvents(bool wantsWheelEvents)
index f0e73cc..ba4afc6 100644 (file)
@@ -1,3 +1,13 @@
+2013-01-30  Levi Weintraub  <leviw@chromium.org>
+
+        [Chromium] WebPluginContainerImpl adding imbalanced touch handler refs
+        https://bugs.webkit.org/show_bug.cgi?id=108381
+
+        Reviewed by James Robinson.
+
+        * DumpRenderTree/chromium/TestRunner/src/WebTestPlugin.cpp: Adding an attribute that
+        tickles the case in WebPluginContainerImpl that imbalanced touch handler refs in Document.
+
 2013-01-30  Julie Parent  <jparent@chromium.org>
 
         Add a concept of dashboard parameters that invalidate others
index d1f750f..020328b 100644 (file)
@@ -235,6 +235,8 @@ private:
     OwnPtr<WebExternalTextureLayer> m_layer;
 
     WebPluginContainer::TouchEventRequestType m_touchEventRequest;
+    // Requests touch events from the WebPluginContainerImpl multiple times to tickle webkit.org/b/108381
+    bool m_reRequestTouchEvents;
     bool m_printEventDetails;
     bool m_printUserGestureStatus;
     bool m_canProcessDrag;
@@ -246,6 +248,7 @@ WebTestPluginImpl::WebTestPluginImpl(WebFrame* frame, const WebPluginParams& par
     , m_container(0)
     , m_context(0)
     , m_touchEventRequest(WebPluginContainer::TouchEventRequestTypeNone)
+    , m_reRequestTouchEvents(false)
     , m_printEventDetails(false)
     , m_printUserGestureStatus(false)
     , m_canProcessDrag(false)
@@ -255,6 +258,7 @@ WebTestPluginImpl::WebTestPluginImpl(WebFrame* frame, const WebPluginParams& par
     static const WebString kAttributePrimitiveColor = WebString::fromUTF8("primitive-color");
     static const WebString kAttributeOpacity = WebString::fromUTF8("opacity");
     static const WebString kAttributeAcceptsTouch = WebString::fromUTF8("accepts-touch");
+    static const WebString kAttributeReRequestTouchEvents = WebString::fromUTF8("re-request-touch");
     static const WebString kAttributePrintEventDetails = WebString::fromUTF8("print-event-details");
     static const WebString kAttributeCanProcessDrag = WebString::fromUTF8("can-process-drag");
     static const WebString kAttributePrintUserGestureStatus = WebString::fromUTF8("print-user-gesture-status");
@@ -275,6 +279,8 @@ WebTestPluginImpl::WebTestPluginImpl(WebFrame* frame, const WebPluginParams& par
             m_scene.opacity = parseOpacity(attributeValue);
         else if (attributeName == kAttributeAcceptsTouch)
             m_touchEventRequest = parseTouchEventRequestType(attributeValue);
+        else if (attributeName == kAttributeReRequestTouchEvents)
+            m_reRequestTouchEvents = parseBoolean(attributeValue);
         else if (attributeName == kAttributePrintEventDetails)
             m_printEventDetails = parseBoolean(attributeValue);
         else if (attributeName == kAttributeCanProcessDrag)
@@ -304,6 +310,10 @@ bool WebTestPluginImpl::initialize(WebPluginContainer* container)
     m_layer = adoptPtr(Platform::current()->compositorSupport()->createExternalTextureLayer(this));
     m_container = container;
     m_container->setWebLayer(m_layer->layer());
+    if (m_reRequestTouchEvents) {
+        m_container->requestTouchEventType(WebPluginContainer::TouchEventRequestTypeSynthesizedMouse);
+        m_container->requestTouchEventType(WebPluginContainer::TouchEventRequestTypeRaw);
+    }
     m_container->requestTouchEventType(m_touchEventRequest);
     m_container->setWantsWheelEvents(true);
     return true;