Skip to site navigation (Press enter)

[webkit-changes] [257482] trunk/Source/WebKit

cdumez Wed, 26 Feb 2020 10:59:24 -0800

Title: [257482] trunk/Source/WebKit

Revision: 257482
Author: [email protected]
Date: 2020-02-26 10:58:28 -0800 (Wed, 26 Feb 2020)

Log Message

Unreviewed, rolling out r257389.


Reverted changeset:

"Make sure a client cannot cause a whole DOM tree to get
leaked by simply holding on to a WKBundleNodeHandle"
https://bugs.webkit.org/show_bug.cgi?id=208218
https://trac.webkit.org/changeset/257389

Modified Paths

trunk/Source/WebKit/ChangeLog
trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp
trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.h

Diff

Modified: trunk/Source/WebKit/ChangeLog (257481 => 257482)


--- trunk/Source/WebKit/ChangeLog	2020-02-26 18:45:17 UTC (rev 257481)
+++ trunk/Source/WebKit/ChangeLog	2020-02-26 18:58:28 UTC (rev 257482)
@@ -1,3 +1,14 @@
+2020-02-26  Chris Dumez  <[email protected]>
+
+        Unreviewed, rolling out r257389.
+
+        Reverted changeset:
+
+        "Make sure a client cannot cause a whole DOM tree to get
+        leaked by simply holding on to a WKBundleNodeHandle"
+        https://bugs.webkit.org/show_bug.cgi?id=208218
+        https://trac.webkit.org/changeset/257389
+
 2020-02-26  Jacob Uphoff  <[email protected]>
 
         Unreviewed, rolling out r257470.

Modified: trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp (257481 => 257482)


--- trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp	2020-02-26 18:45:17 UTC (rev 257481)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp	2020-02-26 18:58:28 UTC (rev 257482)
@@ -101,27 +101,22 @@
 }
 
 InjectedBundleNodeHandle::InjectedBundleNodeHandle(Node& node)
-    : ActiveDOMObject(node.document())
-    , m_node(&node)
+    : m_node(node)
 {
 }
 
 InjectedBundleNodeHandle::~InjectedBundleNodeHandle()
 {
-    if (m_node)
-        domNodeHandleCache().remove(m_node.get());
+    domNodeHandleCache().remove(m_node.ptr());
 }
 
 Node* InjectedBundleNodeHandle::coreNode()
 {
-    return m_node.get();
+    return m_node.ptr();
 }
 
-RefPtr<InjectedBundleNodeHandle> InjectedBundleNodeHandle::document()
+Ref<InjectedBundleNodeHandle> InjectedBundleNodeHandle::document()
 {
-    if (!m_node)
-        return nullptr;
-
     return getOrCreate(m_node->document());
 }
 
@@ -133,14 +128,11 @@
     if (!is<Element>(m_node))
         return IntRect();
 
-    return downcast<Element>(*m_node).boundsInRootViewSpace();
+    return downcast<Element>(m_node.get()).boundsInRootViewSpace();
 }
     
 IntRect InjectedBundleNodeHandle::renderRect(bool* isReplaced)
 {
-    if (!m_node)
-        return { };
-
     return m_node->pixelSnappedRenderRect(isReplaced);
 }
 
@@ -198,9 +190,6 @@
 
 RefPtr<WebImage> InjectedBundleNodeHandle::renderedImage(SnapshotOptions options, bool shouldExcludeOverflow, const Optional<float>& bitmapWidth)
 {
-    if (!m_node)
-        return nullptr;
-
     Frame* frame = m_node->document().frame();
     if (!frame)
         return nullptr;
@@ -223,7 +212,7 @@
         paintingRect = snappedIntRect(renderer->paintingRootRect(topLevelRect));
     }
 
-    frameView->setNodeToDraw(m_node.get());
+    frameView->setNodeToDraw(m_node.ptr());
     auto image = imageForRect(frameView, paintingRect, bitmapWidth, options);
     frameView->setNodeToDraw(0);
 
@@ -232,12 +221,9 @@
 
 RefPtr<InjectedBundleRangeHandle> InjectedBundleNodeHandle::visibleRange()
 {
-    if (!m_node)
-        return nullptr;
+    VisiblePosition start = firstPositionInNode(m_node.ptr());
+    VisiblePosition end = lastPositionInNode(m_node.ptr());
 
-    VisiblePosition start = firstPositionInNode(m_node.get());
-    VisiblePosition end = lastPositionInNode(m_node.get());
-
     RefPtr<Range> range = makeRange(start, end);
     return InjectedBundleRangeHandle::getOrCreate(range.get());
 }
@@ -247,7 +233,7 @@
     if (!is<HTMLInputElement>(m_node))
         return;
 
-    downcast<HTMLInputElement>(*m_node).setValueForUser(value);
+    downcast<HTMLInputElement>(m_node.get()).setValueForUser(value);
 }
 
 void InjectedBundleNodeHandle::setHTMLInputElementSpellcheckEnabled(bool enabled)
@@ -255,7 +241,7 @@
     if (!is<HTMLInputElement>(m_node))
         return;
 
-    downcast<HTMLInputElement>(*m_node).setSpellcheckDisabledExceptTextReplacement(!enabled);
+    downcast<HTMLInputElement>(m_node.get()).setSpellcheckDisabledExceptTextReplacement(!enabled);
 }
 
 bool InjectedBundleNodeHandle::isHTMLInputElementAutoFilled() const
@@ -263,7 +249,7 @@
     if (!is<HTMLInputElement>(m_node))
         return false;
     
-    return downcast<HTMLInputElement>(*m_node).isAutoFilled();
+    return downcast<HTMLInputElement>(m_node.get()).isAutoFilled();
 }
 
 bool InjectedBundleNodeHandle::isHTMLInputElementAutoFilledAndViewable() const
@@ -271,7 +257,7 @@
     if (!is<HTMLInputElement>(m_node))
         return false;
 
-    return downcast<HTMLInputElement>(*m_node).isAutoFilledAndViewable();
+    return downcast<HTMLInputElement>(m_node.get()).isAutoFilledAndViewable();
 }
 
 void InjectedBundleNodeHandle::setHTMLInputElementAutoFilled(bool filled)
@@ -279,7 +265,7 @@
     if (!is<HTMLInputElement>(m_node))
         return;
 
-    downcast<HTMLInputElement>(*m_node).setAutoFilled(filled);
+    downcast<HTMLInputElement>(m_node.get()).setAutoFilled(filled);
 }
 
 void InjectedBundleNodeHandle::setHTMLInputElementAutoFilledAndViewable(bool autoFilledAndViewable)
@@ -287,7 +273,7 @@
     if (!is<HTMLInputElement>(m_node))
         return;
 
-    downcast<HTMLInputElement>(*m_node).setAutoFilledAndViewable(autoFilledAndViewable);
+    downcast<HTMLInputElement>(m_node.get()).setAutoFilledAndViewable(autoFilledAndViewable);
 }
 
 bool InjectedBundleNodeHandle::isHTMLInputElementAutoFillButtonEnabled() const
@@ -295,7 +281,7 @@
     if (!is<HTMLInputElement>(m_node))
         return false;
     
-    return downcast<HTMLInputElement>(*m_node).autoFillButtonType() != AutoFillButtonType::None;
+    return downcast<HTMLInputElement>(m_node.get()).autoFillButtonType() != AutoFillButtonType::None;
 }
 
 void InjectedBundleNodeHandle::setHTMLInputElementAutoFillButtonEnabled(AutoFillButtonType autoFillButtonType)
@@ -303,7 +289,7 @@
     if (!is<HTMLInputElement>(m_node))
         return;
 
-    downcast<HTMLInputElement>(*m_node).setShowAutoFillButton(autoFillButtonType);
+    downcast<HTMLInputElement>(m_node.get()).setShowAutoFillButton(autoFillButtonType);
 }
 
 AutoFillButtonType InjectedBundleNodeHandle::htmlInputElementAutoFillButtonType() const
@@ -310,7 +296,7 @@
 {
     if (!is<HTMLInputElement>(m_node))
         return AutoFillButtonType::None;
-    return downcast<HTMLInputElement>(*m_node).autoFillButtonType();
+    return downcast<HTMLInputElement>(m_node.get()).autoFillButtonType();
 }
 
 AutoFillButtonType InjectedBundleNodeHandle::htmlInputElementLastAutoFillButtonType() const
@@ -317,7 +303,7 @@
 {
     if (!is<HTMLInputElement>(m_node))
         return AutoFillButtonType::None;
-    return downcast<HTMLInputElement>(*m_node).lastAutoFillButtonType();
+    return downcast<HTMLInputElement>(m_node.get()).lastAutoFillButtonType();
 }
 
 bool InjectedBundleNodeHandle::isAutoFillAvailable() const
@@ -325,7 +311,7 @@
     if (!is<HTMLInputElement>(m_node))
         return false;
 
-    return downcast<HTMLInputElement>(*m_node).isAutoFillAvailable();
+    return downcast<HTMLInputElement>(m_node.get()).isAutoFillAvailable();
 }
 
 void InjectedBundleNodeHandle::setAutoFillAvailable(bool autoFillAvailable)
@@ -333,7 +319,7 @@
     if (!is<HTMLInputElement>(m_node))
         return;
 
-    downcast<HTMLInputElement>(*m_node).setAutoFillAvailable(autoFillAvailable);
+    downcast<HTMLInputElement>(m_node.get()).setAutoFillAvailable(autoFillAvailable);
 }
 
 IntRect InjectedBundleNodeHandle::htmlInputElementAutoFillButtonBounds()
@@ -341,7 +327,7 @@
     if (!is<HTMLInputElement>(m_node))
         return IntRect();
 
-    auto autoFillButton = downcast<HTMLInputElement>(*m_node).autoFillButtonElement();
+    auto autoFillButton = downcast<HTMLInputElement>(m_node.get()).autoFillButtonElement();
     if (!autoFillButton)
         return IntRect();
 
@@ -353,7 +339,7 @@
     if (!is<HTMLInputElement>(m_node))
         return false;
 
-    return downcast<HTMLInputElement>(*m_node).lastChangeWasUserEdit();
+    return downcast<HTMLInputElement>(m_node.get()).lastChangeWasUserEdit();
 }
 
 bool InjectedBundleNodeHandle::htmlTextAreaElementLastChangeWasUserEdit()
@@ -361,7 +347,7 @@
     if (!is<HTMLTextAreaElement>(m_node))
         return false;
 
-    return downcast<HTMLTextAreaElement>(*m_node).lastChangeWasUserEdit();
+    return downcast<HTMLTextAreaElement>(m_node.get()).lastChangeWasUserEdit();
 }
 
 bool InjectedBundleNodeHandle::isTextField() const
@@ -369,7 +355,7 @@
     if (!is<HTMLInputElement>(m_node))
         return false;
 
-    return downcast<HTMLInputElement>(*m_node).isTextField();
+    return downcast<HTMLInputElement>(m_node.get()).isTextField();
 }
 
 bool InjectedBundleNodeHandle::isSelectElement() const
@@ -382,15 +368,15 @@
     if (!is<HTMLTableCellElement>(m_node))
         return nullptr;
 
-    return getOrCreate(downcast<HTMLTableCellElement>(*m_node).cellAbove());
+    return getOrCreate(downcast<HTMLTableCellElement>(m_node.get()).cellAbove());
 }
 
 RefPtr<WebFrame> InjectedBundleNodeHandle::documentFrame()
 {
-    if (!m_node || !m_node->isDocumentNode())
+    if (!m_node->isDocumentNode())
         return nullptr;
 
-    Frame* frame = downcast<Document>(*m_node).frame();
+    Frame* frame = downcast<Document>(m_node.get()).frame();
     if (!frame)
         return nullptr;
 
@@ -402,7 +388,7 @@
     if (!is<HTMLFrameElement>(m_node))
         return nullptr;
 
-    Frame* frame = downcast<HTMLFrameElement>(*m_node).contentFrame();
+    Frame* frame = downcast<HTMLFrameElement>(m_node.get()).contentFrame();
     if (!frame)
         return nullptr;
 
@@ -414,7 +400,7 @@
     if (!is<HTMLIFrameElement>(m_node))
         return nullptr;
 
-    Frame* frame = downcast<HTMLIFrameElement>(*m_node).contentFrame();
+    Frame* frame = downcast<HTMLIFrameElement>(m_node.get()).contentFrame();
     if (!frame)
         return nullptr;
 
@@ -421,18 +407,4 @@
     return WebFrame::fromCoreFrame(*frame);
 }
 
-void InjectedBundleNodeHandle::stop()
-{
-    // Invalidate handles to nodes inside documents that are about to be destroyed in order to prevent leaks.
-    if (m_node) {
-        domNodeHandleCache().remove(m_node.get());
-        m_node = nullptr;
-    }
-}
-
-const char* InjectedBundleNodeHandle::activeDOMObjectName() const
-{
-    return "InjectedBundleNodeHandle";
-}
-
 } // namespace WebKit

Modified: trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.h (257481 => 257482)


--- trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.h	2020-02-26 18:45:17 UTC (rev 257481)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.h	2020-02-26 18:58:28 UTC (rev 257482)
@@ -28,7 +28,6 @@
 #include "APIObject.h"
 #include "ImageOptions.h"
 #include <_javascript_Core/JSBase.h>
-#include <WebCore/ActiveDOMObject.h>
 #include <wtf/Forward.h>
 #include <wtf/Optional.h>
 #include <wtf/RefPtr.h>
@@ -46,7 +45,7 @@
 class WebFrame;
 class WebImage;
 
-class InjectedBundleNodeHandle : public API::ObjectImpl<API::Object::Type::BundleNodeHandle>, public WebCore::ActiveDOMObject {
+class InjectedBundleNodeHandle : public API::ObjectImpl<API::Object::Type::BundleNodeHandle> {
 public:
     static RefPtr<InjectedBundleNodeHandle> getOrCreate(JSContextRef, JSObjectRef);
     static RefPtr<InjectedBundleNodeHandle> getOrCreate(WebCore::Node*);
@@ -57,7 +56,7 @@
     WebCore::Node* coreNode();
 
     // Convenience DOM Operations
-    RefPtr<InjectedBundleNodeHandle> document();
+    Ref<InjectedBundleNodeHandle> document();
 
     // Additional DOM Operations
     // Note: These should only be operations that are not exposed to _javascript_.
@@ -93,11 +92,7 @@
     static Ref<InjectedBundleNodeHandle> create(WebCore::Node&);
     InjectedBundleNodeHandle(WebCore::Node&);
 
-    // ActiveDOMObject.
-    void stop() final;
-    const char* activeDOMObjectName() const final;
-
-    RefPtr<WebCore::Node> m_node;
+    Ref<WebCore::Node> m_node;
 };
 
 } // namespace WebKit

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes