Attachment 156168 Details for Bug 93031 – Patch

[patch] Patch

bug-93031-20120802143326.patch (text/plain), 7.69 KB, created by Oliver Hunt on 2012-08-02 14:33:45 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Oliver Hunt

Created: 2012-08-02 14:33:45 PDT

Size: 7.69 KB

patch

obsolete

>Subversion Revision: 124487
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ebce09ea92171c9711508d13c811dbd6bac0315b..87011784220381a770869ff2e8491f3556fa49bc 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2012-08-02  Oliver Hunt  <oliver@apple.com>
>+
>+        A few objects aren't being safely protected from GC in all cases
>+        https://bugs.webkit.org/show_bug.cgi?id=93031
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!).
>+
>+        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  Addy Osmani  <addyo@chromium.org>
> 
>         Web Inspector: Rename 'User agent' to 'Overrides' in settings screen
>diff --git a/Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp b/Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp
>index a897b6fbc5b283865e8035017db723ae8f1f630c..847bce4d6ae7c4c3751afb12eaa6448b6278e064 100644
>--- a/Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp
>+++ b/Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp
>@@ -39,7 +39,7 @@ namespace WebCore {
> 
> 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 @@ PassRefPtr<JSCustomXPathNSResolver> JSCustomXPathNSResolver::create(JSC::ExecSta
>         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 @@ String JSCustomXPathNSResolver::lookupNamespaceURI(const String& prefix)
>     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 @@ String JSCustomXPathNSResolver::lookupNamespaceURI(const String& prefix)
>     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;
>diff --git a/Source/WebCore/bindings/js/JSCustomXPathNSResolver.h b/Source/WebCore/bindings/js/JSCustomXPathNSResolver.h
>index a58cb4946379057403907e11ac5eff1179ae25b7..753f41908e59757ace6cb6e35b3c917362d5b23f 100644
>--- a/Source/WebCore/bindings/js/JSCustomXPathNSResolver.h
>+++ b/Source/WebCore/bindings/js/JSCustomXPathNSResolver.h
>@@ -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 @@ namespace WebCore {
>         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
>diff --git a/Source/WebCore/bindings/js/JSDictionary.cpp b/Source/WebCore/bindings/js/JSDictionary.cpp
>index d88d535f2624f3d470713f5294c4f0078b735a6d..4957ba11d78ca765e2231d0ee8aae01b4581b407 100644
>--- a/Source/WebCore/bindings/js/JSDictionary.cpp
>+++ b/Source/WebCore/bindings/js/JSDictionary.cpp
>@@ -47,9 +47,9 @@ namespace WebCore {
> 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())
>diff --git a/Source/WebCore/bindings/js/JSDictionary.h b/Source/WebCore/bindings/js/JSDictionary.h
>index 717b5880a5923e831b4e60d351a502d5d6730091..5cadb14130fdf39296a5459587bdf750bb7cdcdf 100644
>--- a/Source/WebCore/bindings/js/JSDictionary.h
>+++ b/Source/WebCore/bindings/js/JSDictionary.h
>@@ -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 @@ class JSDictionary {
> public:
>     JSDictionary(JSC::ExecState* exec, JSC::JSObject* initializerObject)
>         : m_exec(exec)
>-        , m_initializerObject(initializerObject)
>+        , m_initializerObject(exec->globalData(), initializerObject)
>     {
>     }
> 
>@@ -62,7 +64,7 @@ public:
>     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 @@ private:
>     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>

Flags:

fpizlo: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 93031: 156168