- Revision
- 124510
- Author
- [email protected]
- Date
- 2012-08-02 15:24:08 -0700 (Thu, 02 Aug 2012)
Log Message
A few objects aren't being safely protected from GC in all cases
https://bugs.webkit.org/show_bug.cgi?id=93031
Reviewed by Filip Pizlo.
I haven't seen evidence that anyone is hitting bugs due to this, but any
GC error can lead to later -- hard to diagnose -- bugs if they result in
resurrecting dead objects.
* bindings/js/JSCustomXPathNSResolver.cpp:
(WebCore::JSCustomXPathNSResolver::create):
(WebCore::JSCustomXPathNSResolver::JSCustomXPathNSResolver):
(WebCore::JSCustomXPathNSResolver::lookupNamespaceURI):
* bindings/js/JSCustomXPathNSResolver.h:
(JSCustomXPathNSResolver):
* bindings/js/JSDictionary.cpp:
(WebCore::JSDictionary::tryGetProperty):
* bindings/js/JSDictionary.h:
(WebCore::JSDictionary::JSDictionary):
(WebCore::JSDictionary::initializerObject):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (124509 => 124510)
--- trunk/Source/WebCore/ChangeLog 2012-08-02 22:11:12 UTC (rev 124509)
+++ trunk/Source/WebCore/ChangeLog 2012-08-02 22:24:08 UTC (rev 124510)
@@ -1,3 +1,26 @@
+2012-08-02 Oliver Hunt <[email protected]>
+
+ A few objects aren't being safely protected from GC in all cases
+ https://bugs.webkit.org/show_bug.cgi?id=93031
+
+ Reviewed by Filip Pizlo.
+
+ I haven't seen evidence that anyone is hitting bugs due to this, but any
+ GC error can lead to later -- hard to diagnose -- bugs if they result in
+ resurrecting dead objects.
+
+ * bindings/js/JSCustomXPathNSResolver.cpp:
+ (WebCore::JSCustomXPathNSResolver::create):
+ (WebCore::JSCustomXPathNSResolver::JSCustomXPathNSResolver):
+ (WebCore::JSCustomXPathNSResolver::lookupNamespaceURI):
+ * bindings/js/JSCustomXPathNSResolver.h:
+ (JSCustomXPathNSResolver):
+ * bindings/js/JSDictionary.cpp:
+ (WebCore::JSDictionary::tryGetProperty):
+ * bindings/js/JSDictionary.h:
+ (WebCore::JSDictionary::JSDictionary):
+ (WebCore::JSDictionary::initializerObject):
+
2012-08-02 Emil A Eklund <[email protected]>
Range::isPointInRange incorrectly throws WRONG_DOCUMENT_ERR
Modified: trunk/Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp (124509 => 124510)
--- trunk/Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp 2012-08-02 22:11:12 UTC (rev 124509)
+++ trunk/Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp 2012-08-02 22:24:08 UTC (rev 124510)
@@ -39,7 +39,7 @@
using namespace JSC;
-PassRefPtr<JSCustomXPathNSResolver> JSCustomXPathNSResolver::create(JSC::ExecState* exec, JSC::JSValue value)
+PassRefPtr<JSCustomXPathNSResolver> JSCustomXPathNSResolver::create(ExecState* exec, JSValue value)
{
if (value.isUndefinedOrNull())
return 0;
@@ -50,12 +50,12 @@
return 0;
}
- return adoptRef(new JSCustomXPathNSResolver(resolverObject, asJSDOMWindow(exec->dynamicGlobalObject())));
+ return adoptRef(new JSCustomXPathNSResolver(exec, resolverObject, asJSDOMWindow(exec->dynamicGlobalObject())));
}
-JSCustomXPathNSResolver::JSCustomXPathNSResolver(JSObject* customResolver, JSDOMWindow* globalObject)
- : m_customResolver(customResolver)
- , m_globalObject(globalObject)
+JSCustomXPathNSResolver::JSCustomXPathNSResolver(ExecState* exec, JSObject* customResolver, JSDOMWindow* globalObject)
+ : m_customResolver(exec->globalData(), customResolver)
+ , m_globalObject(exec->globalData(), globalObject)
{
}
@@ -75,13 +75,13 @@
CallData callData;
CallType callType = getCallData(function, callData);
if (callType == CallTypeNone) {
- callType = m_customResolver->methodTable()->getCallData(m_customResolver, callData);
+ callType = m_customResolver->methodTable()->getCallData(m_customResolver.get(), callData);
if (callType == CallTypeNone) {
// FIXME: Pass actual line number and source URL.
m_globalObject->impl()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "XPathNSResolver does not have a lookupNamespaceURI method.");
return String();
}
- function = m_customResolver;
+ function = m_customResolver.get();
}
RefPtr<JSCustomXPathNSResolver> selfProtector(this);
@@ -90,7 +90,7 @@
args.append(jsString(exec, prefix));
m_globalObject->globalData().timeoutChecker.start();
- JSValue retval = JSMainThreadExecState::call(exec, function, callType, callData, m_customResolver, args);
+ JSValue retval = JSMainThreadExecState::call(exec, function, callType, callData, m_customResolver.get(), args);
m_globalObject->globalData().timeoutChecker.stop();
String result;
Modified: trunk/Source/WebCore/bindings/js/JSCustomXPathNSResolver.h (124509 => 124510)
--- trunk/Source/WebCore/bindings/js/JSCustomXPathNSResolver.h 2012-08-02 22:11:12 UTC (rev 124509)
+++ trunk/Source/WebCore/bindings/js/JSCustomXPathNSResolver.h 2012-08-02 22:24:08 UTC (rev 124510)
@@ -27,6 +27,8 @@
#define JSCustomXPathNSResolver_h
#include "XPathNSResolver.h"
+#include <heap/Strong.h>
+#include <heap/StrongInlines.h>
#include <runtime/JSValue.h>
#include <wtf/Forward.h>
#include <wtf/RefPtr.h>
@@ -50,11 +52,11 @@
virtual String lookupNamespaceURI(const String& prefix);
private:
- JSCustomXPathNSResolver(JSC::JSObject*, JSDOMWindow*);
+ JSCustomXPathNSResolver(JSC::ExecState*, JSC::JSObject*, JSDOMWindow*);
- // JSCustomXPathNSResolvers are always temporary, thus no need to GC protect the objects.
- JSC::JSObject* m_customResolver;
- JSDOMWindow* m_globalObject;
+ // JSCustomXPathNSResolvers are always temporary so using a Strong reference is safe here.
+ JSC::Strong<JSC::JSObject> m_customResolver;
+ JSC::Strong<JSDOMWindow> m_globalObject;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/bindings/js/JSDictionary.cpp (124509 => 124510)
--- trunk/Source/WebCore/bindings/js/JSDictionary.cpp 2012-08-02 22:11:12 UTC (rev 124509)
+++ trunk/Source/WebCore/bindings/js/JSDictionary.cpp 2012-08-02 22:24:08 UTC (rev 124510)
@@ -47,9 +47,9 @@
JSDictionary::GetPropertyResult JSDictionary::tryGetProperty(const char* propertyName, JSValue& finalResult) const
{
Identifier identifier(m_exec, propertyName);
- PropertySlot slot(m_initializerObject);
+ PropertySlot slot(m_initializerObject.get());
- if (!m_initializerObject->getPropertySlot(m_exec, identifier, slot))
+ if (!m_initializerObject.get()->getPropertySlot(m_exec, identifier, slot))
return NoPropertyFound;
if (m_exec->hadException())
Modified: trunk/Source/WebCore/bindings/js/JSDictionary.h (124509 => 124510)
--- trunk/Source/WebCore/bindings/js/JSDictionary.h 2012-08-02 22:11:12 UTC (rev 124509)
+++ trunk/Source/WebCore/bindings/js/JSDictionary.h 2012-08-02 22:24:08 UTC (rev 124510)
@@ -27,6 +27,8 @@
#define JSDictionary_h
#include "MessagePort.h"
+#include <heap/Strong.h>
+#include <heap/StrongInlines.h>
#include <interpreter/CallFrame.h>
#include <wtf/Forward.h>
@@ -46,7 +48,7 @@
public:
JSDictionary(JSC::ExecState* exec, JSC::JSObject* initializerObject)
: m_exec(exec)
- , m_initializerObject(initializerObject)
+ , m_initializerObject(exec->globalData(), initializerObject)
{
}
@@ -62,7 +64,7 @@
bool getWithUndefinedOrNullCheck(const String& propertyName, String& value) const;
JSC::ExecState* execState() const { return m_exec; }
- JSC::JSObject* initializerObject() const { return m_initializerObject; }
+ JSC::JSObject* initializerObject() const { return m_initializerObject.get(); }
bool isValid() const { return m_exec && m_initializerObject; }
private:
@@ -109,7 +111,7 @@
static void convertValue(JSC::ExecState*, JSC::JSValue, ArrayValue& result);
JSC::ExecState* m_exec;
- JSC::JSObject* m_initializerObject;
+ JSC::Strong<JSC::JSObject> m_initializerObject;
};
template <typename T, typename Result>
