Diff
Modified: trunk/Source/WebCore/ChangeLog (243074 => 243075)
--- trunk/Source/WebCore/ChangeLog 2019-03-18 16:56:21 UTC (rev 243074)
+++ trunk/Source/WebCore/ChangeLog 2019-03-18 17:05:54 UTC (rev 243075)
@@ -1,3 +1,16 @@
+2019-03-18 Ryan Haddad <[email protected]>
+
+ Unreviewed, rolling out r243037.
+
+ Broke the Windows build
+
+ Reverted changeset:
+
+ "Reduce the size of Node::deref by eliminating an explicit
+ parentNode check"
+ https://bugs.webkit.org/show_bug.cgi?id=195776
+ https://trac.webkit.org/changeset/243037
+
2019-03-18 Eric Carlson <[email protected]>
Change some logging levels
Modified: trunk/Source/WebCore/dom/Document.cpp (243074 => 243075)
--- trunk/Source/WebCore/dom/Document.cpp 2019-03-18 16:56:21 UTC (rev 243074)
+++ trunk/Source/WebCore/dom/Document.cpp 2019-03-18 17:05:54 UTC (rev 243075)
@@ -669,10 +669,6 @@
{
ASSERT(!m_deletionHasBegun);
if (m_referencingNodeCount) {
- // Node::removedLastRef doesn't set refCount() to zero because it's not observable.
- // But we need to remember that our refCount reached zero in subsequent calls to decrementReferencingNodeCount()
- m_refCountAndParentBit = 0;
-
// If removing a child removes the last node reference, we don't want the scope to be destroyed
// until after removeDetachedChildren returns, so we protect ourselves.
incrementReferencingNodeCount();
@@ -722,6 +718,7 @@
m_inRemovedLastRefFunction = false;
m_deletionHasBegun = true;
#endif
+ m_refCount = 1; // Avoid double destruction through use of RefPtr<T>. (This is a security mitigation in case of programmer error. It will ASSERT in debug builds.)
delete this;
}
}
Modified: trunk/Source/WebCore/dom/Document.h (243074 => 243075)
--- trunk/Source/WebCore/dom/Document.h 2019-03-18 16:56:21 UTC (rev 243074)
+++ trunk/Source/WebCore/dom/Document.h 2019-03-18 17:05:54 UTC (rev 243075)
@@ -376,7 +376,7 @@
#if !ASSERT_DISABLED
m_deletionHasBegun = true;
#endif
- m_refCountAndParentBit = s_refCountIncrement; // Avoid double destruction through use of Ref<T>/RefPtr<T>. (This is a security mitigation in case of programmer error. It will ASSERT in debug builds.)
+ m_refCount = 1; // Avoid double destruction through use of RefPtr<T>. (This is a security mitigation in case of programmer error. It will ASSERT in debug builds.)
delete this;
}
}
Modified: trunk/Source/WebCore/dom/Node.cpp (243074 => 243075)
--- trunk/Source/WebCore/dom/Node.cpp 2019-03-18 16:56:21 UTC (rev 243074)
+++ trunk/Source/WebCore/dom/Node.cpp 2019-03-18 17:05:54 UTC (rev 243075)
@@ -316,7 +316,8 @@
}
Node::Node(Document& document, ConstructionType type)
- : m_nodeFlags(type)
+ : m_refCount(1)
+ , m_nodeFlags(type)
, m_treeScope(&document)
{
ASSERT(isMainThread());
@@ -331,9 +332,9 @@
Node::~Node()
{
ASSERT(isMainThread());
- // We set m_refCount to 2 before calling delete to avoid double destruction through use of Ref<T>/RefPtr<T>.
+ // We set m_refCount to 1 before calling delete to avoid double destruction through use of Ref<T>/RefPtr<T>.
// This is a security mitigation in case of programmer errorm (caught by a debug assertion).
- ASSERT(m_refCountAndParentBit == s_refCountIncrement);
+ ASSERT(m_refCount == 1);
ASSERT(m_deletionHasBegun);
ASSERT(!m_adoptionIsRequired);
@@ -2522,10 +2523,6 @@
// delete a Node at each deref call site.
void Node::removedLastRef()
{
- // This avoids double destruction even when there is a programming error to use Ref<T> / RefPtr<T> on this node.
- // There are debug assertions in Node::ref() / Node::deref() to catch such a programming error.
- ASSERT(m_refCountAndParentBit == s_refCountIncrement);
-
// An explicit check for Document here is better than a virtual function since it is
// faster for non-Document nodes, and because the call to removedLastRef that is inlined
// at all deref call sites is smaller if it's a non-virtual function.
@@ -2537,6 +2534,7 @@
#ifndef NDEBUG
m_deletionHasBegun = true;
#endif
+ m_refCount = 1; // Avoid double destruction through use of RefPtr<T>. (This is a security mitigation in case of programmer error. It will ASSERT in debug builds.)
delete this;
}
Modified: trunk/Source/WebCore/dom/Node.h (243074 => 243075)
--- trunk/Source/WebCore/dom/Node.h 2019-03-18 16:56:21 UTC (rev 243074)
+++ trunk/Source/WebCore/dom/Node.h 2019-03-18 17:05:54 UTC (rev 243075)
@@ -500,7 +500,7 @@
void ref();
void deref();
bool hasOneRef() const;
- unsigned refCount() const;
+ int refCount() const;
#ifndef NDEBUG
bool m_deletionHasBegun { false };
@@ -618,9 +618,6 @@
};
Node(Document&, ConstructionType);
- static constexpr uint32_t s_refCountIncrement = 2;
- static constexpr uint32_t s_refCountMask = ~0x1;
-
virtual void addSubresourceAttributeURLs(ListHashSet<URL>&) const { }
bool hasRareData() const { return getFlag(HasRareDataFlag); }
@@ -667,7 +664,7 @@
static void moveTreeToNewScope(Node&, TreeScope& oldScope, TreeScope& newScope);
void moveNodeToNewDocument(Document& oldDocument, Document& newDocument);
- uint32_t m_refCountAndParentBit { s_refCountIncrement };
+ int m_refCount;
mutable uint32_t m_nodeFlags;
ContainerNode* m_parentNode { nullptr };
@@ -698,25 +695,22 @@
ASSERT(!m_deletionHasBegun);
ASSERT(!m_inRemovedLastRefFunction);
ASSERT(!m_adoptionIsRequired);
- m_refCountAndParentBit += s_refCountIncrement;
+ ++m_refCount;
}
ALWAYS_INLINE void Node::deref()
{
ASSERT(isMainThread());
- ASSERT(refCount());
+ ASSERT(m_refCount >= 0);
ASSERT(!m_deletionHasBegun);
ASSERT(!m_inRemovedLastRefFunction);
ASSERT(!m_adoptionIsRequired);
- auto tempRefCount = m_refCountAndParentBit - s_refCountIncrement;
- if (!tempRefCount) {
+ if (--m_refCount <= 0 && !parentNode()) {
#ifndef NDEBUG
m_inRemovedLastRefFunction = true;
#endif
removedLastRef();
- return;
}
- m_refCountAndParentBit = tempRefCount;
}
ALWAYS_INLINE bool Node::hasOneRef() const
@@ -723,12 +717,12 @@
{
ASSERT(!m_deletionHasBegun);
ASSERT(!m_inRemovedLastRefFunction);
- return refCount() == 1;
+ return m_refCount == 1;
}
-ALWAYS_INLINE unsigned Node::refCount() const
+ALWAYS_INLINE int Node::refCount() const
{
- return m_refCountAndParentBit / s_refCountIncrement;
+ return m_refCount;
}
// Used in Node::addSubresourceAttributeURLs() and in addSubresourceStyleURLs()
@@ -742,8 +736,6 @@
{
ASSERT(isMainThread());
m_parentNode = parent;
- auto refCountWithoutParentBit = m_refCountAndParentBit & s_refCountMask;
- m_refCountAndParentBit = refCountWithoutParentBit | !!parent;
}
inline ContainerNode* Node::parentNode() const
