Title: [134495] trunk/Source/WebCore
- Revision
- 134495
- Author
- [email protected]
- Date
- 2012-11-13 15:21:38 -0800 (Tue, 13 Nov 2012)
Log Message
JSEventListener should not access m_jsFunction when its wrapper is gone.
https://bugs.webkit.org/show_bug.cgi?id=101985.
Reviewed by Geoffrey Garen.
Added a few null checks for m_wrapper before we do anything with m_jsFunction.
No new tests.
* bindings/js/JSEventListener.cpp:
(WebCore::JSEventListener::initializeJSFunction):
- Removed a now invalid assertion. m_wrapper is expected to have a
valid non-zero value when jsFunction is valid. However, in the case
of JSLazyEventListener (which extends JSEventListener), m_wrapper is
initially 0 when m_jsFunction has not been realized yet. When
JSLazyEventListener::initializeJSFunction() realizes m_jsFunction,
it will set m_wrapper to an appropriate wrapper object.
For this reason, JSEventListener::jsFunction() cannot do the null
check on m_wrapper until after the call to initializeJSFunction.
This, in turns, means that in the case of the non-lazy
JSEventListener, initializeJSFunction() will also be called, and
if the GC has collected the m_wrapper but the JSEventListener has
not been removed yet, it is possible to see a null m_wrapper while
m_jsFunction contains a non-zero stale value.
Hence, this assertion of (m_wrapper || !m_jsFunction) in
JSEventListener::initializeJSFunction() is not always true and
should be removed.
(WebCore::JSEventListener::visitJSFunction):
(WebCore::JSEventListener::operator==):
* bindings/js/JSEventListener.h:
(WebCore::JSEventListener::jsFunction):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (134494 => 134495)
--- trunk/Source/WebCore/ChangeLog 2012-11-13 23:21:35 UTC (rev 134494)
+++ trunk/Source/WebCore/ChangeLog 2012-11-13 23:21:38 UTC (rev 134495)
@@ -1,3 +1,41 @@
+2012-11-13 Mark Lam <[email protected]>
+
+ JSEventListener should not access m_jsFunction when its wrapper is gone.
+ https://bugs.webkit.org/show_bug.cgi?id=101985.
+
+ Reviewed by Geoffrey Garen.
+
+ Added a few null checks for m_wrapper before we do anything with m_jsFunction.
+
+ No new tests.
+
+ * bindings/js/JSEventListener.cpp:
+ (WebCore::JSEventListener::initializeJSFunction):
+ - Removed a now invalid assertion. m_wrapper is expected to have a
+ valid non-zero value when jsFunction is valid. However, in the case
+ of JSLazyEventListener (which extends JSEventListener), m_wrapper is
+ initially 0 when m_jsFunction has not been realized yet. When
+ JSLazyEventListener::initializeJSFunction() realizes m_jsFunction,
+ it will set m_wrapper to an appropriate wrapper object.
+
+ For this reason, JSEventListener::jsFunction() cannot do the null
+ check on m_wrapper until after the call to initializeJSFunction.
+
+ This, in turns, means that in the case of the non-lazy
+ JSEventListener, initializeJSFunction() will also be called, and
+ if the GC has collected the m_wrapper but the JSEventListener has
+ not been removed yet, it is possible to see a null m_wrapper while
+ m_jsFunction contains a non-zero stale value.
+
+ Hence, this assertion of (m_wrapper || !m_jsFunction) in
+ JSEventListener::initializeJSFunction() is not always true and
+ should be removed.
+
+ (WebCore::JSEventListener::visitJSFunction):
+ (WebCore::JSEventListener::operator==):
+ * bindings/js/JSEventListener.h:
+ (WebCore::JSEventListener::jsFunction):
+
2012-11-13 Adam Barth <[email protected]>
[V8] instantiateV8Object should encapulate the tricky creationContext logic
Modified: trunk/Source/WebCore/bindings/js/JSEventListener.cpp (134494 => 134495)
--- trunk/Source/WebCore/bindings/js/JSEventListener.cpp 2012-11-13 23:21:35 UTC (rev 134494)
+++ trunk/Source/WebCore/bindings/js/JSEventListener.cpp 2012-11-13 23:21:38 UTC (rev 134495)
@@ -59,12 +59,15 @@
JSObject* JSEventListener::initializeJSFunction(ScriptExecutionContext*) const
{
- ASSERT_NOT_REACHED();
return 0;
}
void JSEventListener::visitJSFunction(SlotVisitor& visitor)
{
+ // If m_wrapper is 0, then jsFunction is zombied, and should never be accessed.
+ if (!m_wrapper)
+ return;
+
if (m_jsFunction)
visitor.append(&m_jsFunction);
}
@@ -160,6 +163,10 @@
bool JSEventListener::operator==(const EventListener& listener)
{
+ // If m_wrapper is 0, then jsFunction is zombied, and should never be accessed.
+ if (!m_wrapper)
+ return false;
+
if (const JSEventListener* jsEventListener = JSEventListener::cast(&listener))
return m_jsFunction == jsEventListener->m_jsFunction && m_isAttribute == jsEventListener->m_isAttribute;
return false;
Modified: trunk/Source/WebCore/bindings/js/JSEventListener.h (134494 => 134495)
--- trunk/Source/WebCore/bindings/js/JSEventListener.h 2012-11-13 23:21:35 UTC (rev 134494)
+++ trunk/Source/WebCore/bindings/js/JSEventListener.h 2012-11-13 23:21:38 UTC (rev 134495)
@@ -85,11 +85,13 @@
m_jsFunction.setMayBeNull(*scriptExecutionContext->globalData(), m_wrapper.get(), function);
}
+ // If m_wrapper is 0, then jsFunction is zombied, and should never be accessed.
+ if (!m_wrapper)
+ return 0;
+
// Verify that we have a valid wrapper protecting our function from
// garbage collection.
ASSERT(m_wrapper || !m_jsFunction);
- if (!m_wrapper)
- return 0;
// Try to verify that m_jsFunction wasn't recycled. (Not exact, since an
// event listener can be almost anything, but this makes test-writing easier).
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
