Changeset 141348 in webkit

Timestamp:

Jan 30, 2013, 5:08:25 PM (12 years ago)

Author:

oliver@apple.com

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/js/JSLazyEventListener.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-30  Oliver Hunt  <oliver@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  <haraken@chromium.org>
 

trunk/Source/WebCore/bindings/js/JSLazyEventListener.cpp

@@ -57 +57 @@
         m_position = TextPosition::minimumPosition();
 
+    ASSERT(m_eventParameterName == "evt" || m_eventParameterName == "event");
+
 #ifndef NDEBUG
     eventListenerCounter.increment();
@@ -74 +76 @@
     ASSERT(executionContext->isDocument());
     if (!executionContext)
+        return 0;
+
+    ASSERT(!m_code.isNull());
+    ASSERT(!m_eventParameterName.isNull());
+    if (m_code.isNull() || m_eventParameterName.isNull())
         return 0;
 
@@ -118 +125 @@
         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;
 }