Title: [141348] trunk/Source/WebCore
Revision
141348
Author
oli...@apple.com
Date
2013-01-30 17:08:25 -0800 (Wed, 30 Jan 2013)

Log Message

Make JSEventListener more robust in the event of the compiled handler being released.
https://bugs.webkit.org/show_bug.cgi?id=108390

Reviewed by Michael Saboff.

It shouldn't be possible for a handler to be collected for any object that
might still trigger an event.  Nonetheless it does still happen, and the
only point of failure that currently exists is compiling the handler.

This patch is mostly just assertions, but also removes a bunch of field
clearing that was previously being performed.  Given we do seem to periodically
end up needing to recompile a handler this appears to be necessary.  That
said there does not appear to be a memory hit from this as the event handlers
that make us use JSLazyEventListener are not frequently compiled, and the bulk
of the functions that are compiled end up causing us to keep the strings around
anyway.

* bindings/js/JSLazyEventListener.cpp:
(WebCore::JSLazyEventListener::JSLazyEventListener):
(WebCore::JSLazyEventListener::initializeJSFunction):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (141347 => 141348)


--- trunk/Source/WebCore/ChangeLog	2013-01-31 01:05:34 UTC (rev 141347)
+++ trunk/Source/WebCore/ChangeLog	2013-01-31 01:08:25 UTC (rev 141348)
@@ -1,3 +1,26 @@
+2013-01-30  Oliver Hunt  <oli...@apple.com>
+
+        Make JSEventListener more robust in the event of the compiled handler being released.
+        https://bugs.webkit.org/show_bug.cgi?id=108390
+
+        Reviewed by Michael Saboff.
+
+        It shouldn't be possible for a handler to be collected for any object that
+        might still trigger an event.  Nonetheless it does still happen, and the
+        only point of failure that currently exists is compiling the handler.
+
+        This patch is mostly just assertions, but also removes a bunch of field
+        clearing that was previously being performed.  Given we do seem to periodically
+        end up needing to recompile a handler this appears to be necessary.  That
+        said there does not appear to be a memory hit from this as the event handlers
+        that make us use JSLazyEventListener are not frequently compiled, and the bulk
+        of the functions that are compiled end up causing us to keep the strings around
+        anyway.
+
+        * bindings/js/JSLazyEventListener.cpp:
+        (WebCore::JSLazyEventListener::JSLazyEventListener):
+        (WebCore::JSLazyEventListener::initializeJSFunction):
+
 2013-01-30  Kentaro Hara  <hara...@chromium.org>
 
         Implement KeyboardEvent constructor

Modified: trunk/Source/WebCore/bindings/js/JSLazyEventListener.cpp (141347 => 141348)


--- trunk/Source/WebCore/bindings/js/JSLazyEventListener.cpp	2013-01-31 01:05:34 UTC (rev 141347)
+++ trunk/Source/WebCore/bindings/js/JSLazyEventListener.cpp	2013-01-31 01:08:25 UTC (rev 141348)
@@ -56,6 +56,8 @@
     if (m_position == TextPosition::belowRangePosition())
         m_position = TextPosition::minimumPosition();
 
+    ASSERT(m_eventParameterName == "evt" || m_eventParameterName == "event");
+
 #ifndef NDEBUG
     eventListenerCounter.increment();
 #endif
@@ -75,6 +77,11 @@
     if (!executionContext)
         return 0;
 
+    ASSERT(!m_code.isNull());
+    ASSERT(!m_eventParameterName.isNull());
+    if (m_code.isNull() || m_eventParameterName.isNull())
+        return 0;
+
     Document* document = static_cast<Document*>(executionContext);
 
     if (!document->frame())
@@ -117,12 +124,6 @@
         // (and the document, and the form - see JSHTMLElement::eventHandlerScope)
         listenerAsFunction->setScope(exec->globalData(), jsCast<JSNode*>(wrapper())->pushEventHandlerScope(exec, listenerAsFunction->scope()));
     }
-
-    // Since we only parse once, there's no need to keep data used for parsing around anymore.
-    m_functionName = String();
-    m_code = String();
-    m_eventParameterName = String();
-    m_sourceURL = String();
     return jsFunction;
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to