Attachment 368981 Details for Bug 197576 – Patch

[patch] Patch

bug-197576-20190503135702.patch (text/plain), 5.93 KB, created by Chris Dumez on 2019-05-03 13:57:02 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-05-03 13:57:02 PDT

Size: 5.93 KB

patch

obsolete

>Subversion Revision: 244912
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 9a74e09f9f9a019120975b7f2e7842a83e903104..d79af6308449fda5cbbfd69caa4edabb1c6aceb4 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-05-03  Chris Dumez  <cdumez@apple.com>
>+
>+        Use WeakPtr for JSLazyEventListener::m_originalNode for safety
>+        https://bugs.webkit.org/show_bug.cgi?id=197576
>+        <rdar://problem/24314027>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * bindings/js/JSLazyEventListener.cpp:
>+        (WebCore::JSLazyEventListener::JSLazyEventListener):
>+        (WebCore::JSLazyEventListener::create):
>+        * bindings/js/JSLazyEventListener.h:
>+
> 2019-05-03  Antti Koivisto  <antti@apple.com>
> 
>         Add a quirk to make youtube navigation bar scrollable without mouse hover on iOS
>diff --git a/Source/WebCore/bindings/js/JSLazyEventListener.cpp b/Source/WebCore/bindings/js/JSLazyEventListener.cpp
>index b382b1f946a7c54ef38eafa527268e0e70e0c960..8ca1336eff3bf76be3c3dd3351e4b385be831440 100644
>--- a/Source/WebCore/bindings/js/JSLazyEventListener.cpp
>+++ b/Source/WebCore/bindings/js/JSLazyEventListener.cpp
>@@ -33,6 +33,7 @@
> #include <wtf/NeverDestroyed.h>
> #include <wtf/RefCountedLeakCounter.h>
> #include <wtf/StdLibExtras.h>
>+#include <wtf/WeakPtr.h>
> 
> namespace WebCore {
> using namespace JSC;
>@@ -43,7 +44,7 @@ struct JSLazyEventListener::CreationArguments {
>     const QualifiedName& attributeName;
>     const AtomicString& attributeValue;
>     Document& document;
>-    ContainerNode* node;
>+    WeakPtr<ContainerNode> node;
>     JSObject* wrapper;
>     bool shouldUseSVGEventName;
> };
>@@ -64,21 +65,15 @@ static TextPosition convertZeroToOne(const TextPosition& position)
>     return position;
> }
> 
>-JSLazyEventListener::JSLazyEventListener(const CreationArguments& arguments, const String& sourceURL, const TextPosition& sourcePosition)
>+JSLazyEventListener::JSLazyEventListener(CreationArguments&& arguments, const String& sourceURL, const TextPosition& sourcePosition)
>     : JSEventListener(nullptr, arguments.wrapper, true, mainThreadNormalWorld())
>     , m_functionName(arguments.attributeName.localName().string())
>     , m_eventParameterName(eventParameterName(arguments.shouldUseSVGEventName))
>     , m_code(arguments.attributeValue)
>     , m_sourceURL(sourceURL)
>     , m_sourcePosition(convertZeroToOne(sourcePosition))
>-    , m_originalNode(arguments.node)
>+    , m_originalNode(WTFMove(arguments.node))
> {
>-    // We don't ref m_originalNode because we assume it will stay alive as long as this
>-    // handler object is around and we need to avoid a reference cycle. If JS transfers
>-    // this handler to another node, initializeJSFunction will be called and after that
>-    // m_originalNode will never be looked at again.
>-    // FIXME: Doesn't seem clear that is guaranteed to be true in the general case.
>-
> #ifndef NDEBUG
>     eventListenerCounter.increment();
> #endif
>@@ -159,7 +154,7 @@ JSObject* JSLazyEventListener::initializeJSFunction(ScriptExecutionContext& exec
>     return jsFunction;
> }
> 
>-RefPtr<JSLazyEventListener> JSLazyEventListener::create(const CreationArguments& arguments)
>+RefPtr<JSLazyEventListener> JSLazyEventListener::create(CreationArguments&& arguments)
> {
>     if (arguments.attributeValue.isNull())
>         return nullptr;
>@@ -174,19 +169,19 @@ RefPtr<JSLazyEventListener> JSLazyEventListener::create(const CreationArguments&
>         sourceURL = arguments.document.url().string();
>     }
> 
>-    return adoptRef(*new JSLazyEventListener(arguments, sourceURL, position));
>+    return adoptRef(*new JSLazyEventListener(WTFMove(arguments), sourceURL, position));
> }
> 
> RefPtr<JSLazyEventListener> JSLazyEventListener::create(Element& element, const QualifiedName& attributeName, const AtomicString& attributeValue)
> {
>-    return create({ attributeName, attributeValue, element.document(), &element, nullptr, element.isSVGElement() });
>+    return create({ attributeName, attributeValue, element.document(), makeWeakPtr(element), nullptr, element.isSVGElement() });
> }
> 
> RefPtr<JSLazyEventListener> JSLazyEventListener::create(Document& document, const QualifiedName& attributeName, const AtomicString& attributeValue)
> {
>     // FIXME: This always passes false for "shouldUseSVGEventName". Is that correct for events dispatched to SVG documents?
>     // This has been this way for a long time, but became more obvious when refactoring to separate the Element and Document code paths.
>-    return create({ attributeName, attributeValue, document, &document, nullptr, false });
>+    return create({ attributeName, attributeValue, document, makeWeakPtr(document), nullptr, false });
> }
> 
> RefPtr<JSLazyEventListener> JSLazyEventListener::create(DOMWindow& window, const QualifiedName& attributeName, const AtomicString& attributeValue)
>diff --git a/Source/WebCore/bindings/js/JSLazyEventListener.h b/Source/WebCore/bindings/js/JSLazyEventListener.h
>index bdee42a845f9fe28a4ffcb22cd4704cabbf91910..552d0779305eff0150e0881c0b2ab769ad015b71 100644
>--- a/Source/WebCore/bindings/js/JSLazyEventListener.h
>+++ b/Source/WebCore/bindings/js/JSLazyEventListener.h
>@@ -20,6 +20,7 @@
> #pragma once
> 
> #include "JSEventListener.h"
>+#include <wtf/Forward.h>
> 
> namespace WebCore {
> 
>@@ -42,8 +43,8 @@ public:
> 
> private:
>     struct CreationArguments;
>-    static RefPtr<JSLazyEventListener> create(const CreationArguments&);
>-    JSLazyEventListener(const CreationArguments&, const String& sourceURL, const TextPosition&);
>+    static RefPtr<JSLazyEventListener> create(CreationArguments&&);
>+    JSLazyEventListener(CreationArguments&&, const String& sourceURL, const TextPosition&);
> 
>     JSC::JSObject* initializeJSFunction(ScriptExecutionContext&) const final;
>     bool wasCreatedFromMarkup() const final { return true; }
>@@ -53,7 +54,7 @@ private:
>     String m_code;
>     String m_sourceURL;
>     TextPosition m_sourcePosition;
>-    ContainerNode* m_originalNode;
>+    WeakPtr<ContainerNode> m_originalNode;
> };
> 
> } // namespace WebCore

Actions: View | Formatted Diff | Diff

Attachments on bug 197576: 368981