Modified: trunk/Source/WebKit/ChangeLog (257502 => 257503)
--- trunk/Source/WebKit/ChangeLog 2020-02-26 20:05:05 UTC (rev 257502)
+++ trunk/Source/WebKit/ChangeLog 2020-02-26 20:57:49 UTC (rev 257503)
@@ -1,3 +1,48 @@
+2020-02-26 Chris Dumez <[email protected]>
+
+ 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
+
+ Reviewed by Ryosuke Niwa.
+
+ Make sure a client cannot cause a whole DOM tree to get leaked by simply holding on to a WKBundleNodeHandle.
+ Previously, WKBundleNodeHandle would ref its node, which would keep the whole HTML document alive. To protect
+ against this, InjectedBundleNodeHandle is now an ActiveDOMObject which nulls out its node RefPtr when the
+ document is getting ready for destruction (i.e. ActiveDOMObject::stop() is called).
+
+ * WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp:
+ (WebKit::InjectedBundleNodeHandle::InjectedBundleNodeHandle):
+ (WebKit::InjectedBundleNodeHandle::~InjectedBundleNodeHandle):
+ (WebKit::InjectedBundleNodeHandle::coreNode):
+ (WebKit::InjectedBundleNodeHandle::document):
+ (WebKit::InjectedBundleNodeHandle::elementBounds):
+ (WebKit::InjectedBundleNodeHandle::renderRect):
+ (WebKit::InjectedBundleNodeHandle::renderedImage):
+ (WebKit::InjectedBundleNodeHandle::visibleRange):
+ (WebKit::InjectedBundleNodeHandle::setHTMLInputElementValueForUser):
+ (WebKit::InjectedBundleNodeHandle::setHTMLInputElementSpellcheckEnabled):
+ (WebKit::InjectedBundleNodeHandle::isHTMLInputElementAutoFilled const):
+ (WebKit::InjectedBundleNodeHandle::isHTMLInputElementAutoFilledAndViewable const):
+ (WebKit::InjectedBundleNodeHandle::setHTMLInputElementAutoFilled):
+ (WebKit::InjectedBundleNodeHandle::setHTMLInputElementAutoFilledAndViewable):
+ (WebKit::InjectedBundleNodeHandle::isHTMLInputElementAutoFillButtonEnabled const):
+ (WebKit::InjectedBundleNodeHandle::setHTMLInputElementAutoFillButtonEnabled):
+ (WebKit::InjectedBundleNodeHandle::htmlInputElementAutoFillButtonType const):
+ (WebKit::InjectedBundleNodeHandle::htmlInputElementLastAutoFillButtonType const):
+ (WebKit::InjectedBundleNodeHandle::isAutoFillAvailable const):
+ (WebKit::InjectedBundleNodeHandle::setAutoFillAvailable):
+ (WebKit::InjectedBundleNodeHandle::htmlInputElementAutoFillButtonBounds):
+ (WebKit::InjectedBundleNodeHandle::htmlInputElementLastChangeWasUserEdit):
+ (WebKit::InjectedBundleNodeHandle::htmlTextAreaElementLastChangeWasUserEdit):
+ (WebKit::InjectedBundleNodeHandle::isTextField const):
+ (WebKit::InjectedBundleNodeHandle::htmlTableCellElementCellAbove):
+ (WebKit::InjectedBundleNodeHandle::documentFrame):
+ (WebKit::InjectedBundleNodeHandle::htmlFrameElementContentFrame):
+ (WebKit::InjectedBundleNodeHandle::htmlIFrameElementContentFrame):
+ (WebKit::InjectedBundleNodeHandle::stop):
+ (WebKit::InjectedBundleNodeHandle::activeDOMObjectName const):
+ * WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.h:
+
2020-02-26 Daniel Bates <[email protected]>
[iOS] Send focus update immediately on becoming or resigning first responder
Modified: trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp (257502 => 257503)
--- trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp 2020-02-26 20:05:05 UTC (rev 257502)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp 2020-02-26 20:57:49 UTC (rev 257503)
@@ -84,37 +84,44 @@
Ref<InjectedBundleNodeHandle> InjectedBundleNodeHandle::getOrCreate(Node& node)
{
- DOMNodeHandleCache::AddResult result = domNodeHandleCache().add(&node, nullptr);
- if (!result.isNewEntry)
- return Ref<InjectedBundleNodeHandle>(*result.iterator->value);
+ if (auto* existingHandle = domNodeHandleCache().get(&node))
+ return Ref<InjectedBundleNodeHandle>(*existingHandle);
- Ref<InjectedBundleNodeHandle> nodeHandle = InjectedBundleNodeHandle::create(node);
- result.iterator->value = nodeHandle.ptr();
+ auto nodeHandle = InjectedBundleNodeHandle::create(node);
+ if (nodeHandle->coreNode())
+ domNodeHandleCache().add(nodeHandle->coreNode(), nodeHandle.ptr());
return nodeHandle;
}
Ref<InjectedBundleNodeHandle> InjectedBundleNodeHandle::create(Node& node)
{
- return adoptRef(*new InjectedBundleNodeHandle(node));
+ auto handle = adoptRef(*new InjectedBundleNodeHandle(node));
+ handle->suspendIfNeeded();
+ return handle;
}
InjectedBundleNodeHandle::InjectedBundleNodeHandle(Node& node)
- : m_node(node)
+ : ActiveDOMObject(node.document())
+ , m_node(&node)
{
}
InjectedBundleNodeHandle::~InjectedBundleNodeHandle()
{
- domNodeHandleCache().remove(m_node.ptr());
+ if (m_node)
+ domNodeHandleCache().remove(m_node.get());
}
Node* InjectedBundleNodeHandle::coreNode()
{
- return m_node.ptr();
+ return m_node.get();
}
-Ref<InjectedBundleNodeHandle> InjectedBundleNodeHandle::document()
+RefPtr<InjectedBundleNodeHandle> InjectedBundleNodeHandle::document()
{
+ if (!m_node)
+ return nullptr;
+
return getOrCreate(m_node->document());
}
@@ -126,11 +133,14 @@
if (!is<Element>(m_node))
return IntRect();
- return downcast<Element>(m_node.get()).boundsInRootViewSpace();
+ return downcast<Element>(*m_node).boundsInRootViewSpace();
}
IntRect InjectedBundleNodeHandle::renderRect(bool* isReplaced)
{
+ if (!m_node)
+ return { };
+
return m_node->pixelSnappedRenderRect(isReplaced);
}
@@ -188,6 +198,9 @@
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;
@@ -210,7 +223,7 @@
paintingRect = snappedIntRect(renderer->paintingRootRect(topLevelRect));
}
- frameView->setNodeToDraw(m_node.ptr());
+ frameView->setNodeToDraw(m_node.get());
auto image = imageForRect(frameView, paintingRect, bitmapWidth, options);
frameView->setNodeToDraw(0);
@@ -219,9 +232,12 @@
RefPtr<InjectedBundleRangeHandle> InjectedBundleNodeHandle::visibleRange()
{
- VisiblePosition start = firstPositionInNode(m_node.ptr());
- VisiblePosition end = lastPositionInNode(m_node.ptr());
+ if (!m_node)
+ return nullptr;
+ VisiblePosition start = firstPositionInNode(m_node.get());
+ VisiblePosition end = lastPositionInNode(m_node.get());
+
RefPtr<Range> range = makeRange(start, end);
return InjectedBundleRangeHandle::getOrCreate(range.get());
}
@@ -231,7 +247,7 @@
if (!is<HTMLInputElement>(m_node))
return;
- downcast<HTMLInputElement>(m_node.get()).setValueForUser(value);
+ downcast<HTMLInputElement>(*m_node).setValueForUser(value);
}
void InjectedBundleNodeHandle::setHTMLInputElementSpellcheckEnabled(bool enabled)
@@ -239,7 +255,7 @@
if (!is<HTMLInputElement>(m_node))
return;
- downcast<HTMLInputElement>(m_node.get()).setSpellcheckDisabledExceptTextReplacement(!enabled);
+ downcast<HTMLInputElement>(*m_node).setSpellcheckDisabledExceptTextReplacement(!enabled);
}
bool InjectedBundleNodeHandle::isHTMLInputElementAutoFilled() const
@@ -247,7 +263,7 @@
if (!is<HTMLInputElement>(m_node))
return false;
- return downcast<HTMLInputElement>(m_node.get()).isAutoFilled();
+ return downcast<HTMLInputElement>(*m_node).isAutoFilled();
}
bool InjectedBundleNodeHandle::isHTMLInputElementAutoFilledAndViewable() const
@@ -255,7 +271,7 @@
if (!is<HTMLInputElement>(m_node))
return false;
- return downcast<HTMLInputElement>(m_node.get()).isAutoFilledAndViewable();
+ return downcast<HTMLInputElement>(*m_node).isAutoFilledAndViewable();
}
void InjectedBundleNodeHandle::setHTMLInputElementAutoFilled(bool filled)
@@ -263,7 +279,7 @@
if (!is<HTMLInputElement>(m_node))
return;
- downcast<HTMLInputElement>(m_node.get()).setAutoFilled(filled);
+ downcast<HTMLInputElement>(*m_node).setAutoFilled(filled);
}
void InjectedBundleNodeHandle::setHTMLInputElementAutoFilledAndViewable(bool autoFilledAndViewable)
@@ -271,7 +287,7 @@
if (!is<HTMLInputElement>(m_node))
return;
- downcast<HTMLInputElement>(m_node.get()).setAutoFilledAndViewable(autoFilledAndViewable);
+ downcast<HTMLInputElement>(*m_node).setAutoFilledAndViewable(autoFilledAndViewable);
}
bool InjectedBundleNodeHandle::isHTMLInputElementAutoFillButtonEnabled() const
@@ -279,7 +295,7 @@
if (!is<HTMLInputElement>(m_node))
return false;
- return downcast<HTMLInputElement>(m_node.get()).autoFillButtonType() != AutoFillButtonType::None;
+ return downcast<HTMLInputElement>(*m_node).autoFillButtonType() != AutoFillButtonType::None;
}
void InjectedBundleNodeHandle::setHTMLInputElementAutoFillButtonEnabled(AutoFillButtonType autoFillButtonType)
@@ -287,7 +303,7 @@
if (!is<HTMLInputElement>(m_node))
return;
- downcast<HTMLInputElement>(m_node.get()).setShowAutoFillButton(autoFillButtonType);
+ downcast<HTMLInputElement>(*m_node).setShowAutoFillButton(autoFillButtonType);
}
AutoFillButtonType InjectedBundleNodeHandle::htmlInputElementAutoFillButtonType() const
@@ -294,7 +310,7 @@
{
if (!is<HTMLInputElement>(m_node))
return AutoFillButtonType::None;
- return downcast<HTMLInputElement>(m_node.get()).autoFillButtonType();
+ return downcast<HTMLInputElement>(*m_node).autoFillButtonType();
}
AutoFillButtonType InjectedBundleNodeHandle::htmlInputElementLastAutoFillButtonType() const
@@ -301,7 +317,7 @@
{
if (!is<HTMLInputElement>(m_node))
return AutoFillButtonType::None;
- return downcast<HTMLInputElement>(m_node.get()).lastAutoFillButtonType();
+ return downcast<HTMLInputElement>(*m_node).lastAutoFillButtonType();
}
bool InjectedBundleNodeHandle::isAutoFillAvailable() const
@@ -309,7 +325,7 @@
if (!is<HTMLInputElement>(m_node))
return false;
- return downcast<HTMLInputElement>(m_node.get()).isAutoFillAvailable();
+ return downcast<HTMLInputElement>(*m_node).isAutoFillAvailable();
}
void InjectedBundleNodeHandle::setAutoFillAvailable(bool autoFillAvailable)
@@ -317,7 +333,7 @@
if (!is<HTMLInputElement>(m_node))
return;
- downcast<HTMLInputElement>(m_node.get()).setAutoFillAvailable(autoFillAvailable);
+ downcast<HTMLInputElement>(*m_node).setAutoFillAvailable(autoFillAvailable);
}
IntRect InjectedBundleNodeHandle::htmlInputElementAutoFillButtonBounds()
@@ -325,7 +341,7 @@
if (!is<HTMLInputElement>(m_node))
return IntRect();
- auto autoFillButton = downcast<HTMLInputElement>(m_node.get()).autoFillButtonElement();
+ auto autoFillButton = downcast<HTMLInputElement>(*m_node).autoFillButtonElement();
if (!autoFillButton)
return IntRect();
@@ -337,7 +353,7 @@
if (!is<HTMLInputElement>(m_node))
return false;
- return downcast<HTMLInputElement>(m_node.get()).lastChangeWasUserEdit();
+ return downcast<HTMLInputElement>(*m_node).lastChangeWasUserEdit();
}
bool InjectedBundleNodeHandle::htmlTextAreaElementLastChangeWasUserEdit()
@@ -345,7 +361,7 @@
if (!is<HTMLTextAreaElement>(m_node))
return false;
- return downcast<HTMLTextAreaElement>(m_node.get()).lastChangeWasUserEdit();
+ return downcast<HTMLTextAreaElement>(*m_node).lastChangeWasUserEdit();
}
bool InjectedBundleNodeHandle::isTextField() const
@@ -353,7 +369,7 @@
if (!is<HTMLInputElement>(m_node))
return false;
- return downcast<HTMLInputElement>(m_node.get()).isTextField();
+ return downcast<HTMLInputElement>(*m_node).isTextField();
}
bool InjectedBundleNodeHandle::isSelectElement() const
@@ -366,15 +382,15 @@
if (!is<HTMLTableCellElement>(m_node))
return nullptr;
- return getOrCreate(downcast<HTMLTableCellElement>(m_node.get()).cellAbove());
+ return getOrCreate(downcast<HTMLTableCellElement>(*m_node).cellAbove());
}
RefPtr<WebFrame> InjectedBundleNodeHandle::documentFrame()
{
- if (!m_node->isDocumentNode())
+ if (!m_node || !m_node->isDocumentNode())
return nullptr;
- Frame* frame = downcast<Document>(m_node.get()).frame();
+ Frame* frame = downcast<Document>(*m_node).frame();
if (!frame)
return nullptr;
@@ -386,7 +402,7 @@
if (!is<HTMLFrameElement>(m_node))
return nullptr;
- Frame* frame = downcast<HTMLFrameElement>(m_node.get()).contentFrame();
+ Frame* frame = downcast<HTMLFrameElement>(*m_node).contentFrame();
if (!frame)
return nullptr;
@@ -398,7 +414,7 @@
if (!is<HTMLIFrameElement>(m_node))
return nullptr;
- Frame* frame = downcast<HTMLIFrameElement>(m_node.get()).contentFrame();
+ Frame* frame = downcast<HTMLIFrameElement>(*m_node).contentFrame();
if (!frame)
return nullptr;
@@ -405,4 +421,18 @@
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 (257502 => 257503)
--- trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.h 2020-02-26 20:05:05 UTC (rev 257502)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.h 2020-02-26 20:57:49 UTC (rev 257503)
@@ -28,6 +28,7 @@
#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>
@@ -45,7 +46,7 @@
class WebFrame;
class WebImage;
-class InjectedBundleNodeHandle : public API::ObjectImpl<API::Object::Type::BundleNodeHandle> {
+class InjectedBundleNodeHandle : public API::ObjectImpl<API::Object::Type::BundleNodeHandle>, public WebCore::ActiveDOMObject {
public:
static RefPtr<InjectedBundleNodeHandle> getOrCreate(JSContextRef, JSObjectRef);
static RefPtr<InjectedBundleNodeHandle> getOrCreate(WebCore::Node*);
@@ -56,7 +57,7 @@
WebCore::Node* coreNode();
// Convenience DOM Operations
- Ref<InjectedBundleNodeHandle> document();
+ RefPtr<InjectedBundleNodeHandle> document();
// Additional DOM Operations
// Note: These should only be operations that are not exposed to _javascript_.
@@ -92,7 +93,11 @@
static Ref<InjectedBundleNodeHandle> create(WebCore::Node&);
InjectedBundleNodeHandle(WebCore::Node&);
- Ref<WebCore::Node> m_node;
+ // ActiveDOMObject.
+ void stop() final;
+ const char* activeDOMObjectName() const final;
+
+ RefPtr<WebCore::Node> m_node;
};
} // namespace WebKit
