[webkit-changes] [243686] trunk/Source/WebCore

Sun, 31 Mar 2019 19:04:15 -0700

Title: [243686] trunk/Source/WebCore
Revision
243686
Author
[email protected]
Date
2019-03-31 19:03:14 -0700 (Sun, 31 Mar 2019)

Log Message

Reduce the size of Node::deref by eliminating an explicit parentNode check
https://bugs.webkit.org/show_bug.cgi?id=195776

Reviewed by Darin Adler.

Address post-commit review comments.

* dom/Document.cpp:
(WebCore::Document::removedLastRef):
* dom/Node.cpp:
(WebCore::Node::~Node):
(WebCore::Node::removedLastRef):
* dom/Node.h:
(WebCore::Node::deref):
(WebCore::Node::setParentNode):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (243685 => 243686)


--- trunk/Source/WebCore/ChangeLog	2019-04-01 01:38:09 UTC (rev 243685)
+++ trunk/Source/WebCore/ChangeLog	2019-04-01 02:03:14 UTC (rev 243686)
@@ -1,3 +1,21 @@
+2019-03-31  Ryosuke Niwa  <[email protected]>
+
+        Reduce the size of Node::deref by eliminating an explicit parentNode check
+        https://bugs.webkit.org/show_bug.cgi?id=195776
+
+        Reviewed by Darin Adler.
+
+        Address post-commit review comments.
+
+        * dom/Document.cpp:
+        (WebCore::Document::removedLastRef):
+        * dom/Node.cpp:
+        (WebCore::Node::~Node):
+        (WebCore::Node::removedLastRef):
+        * dom/Node.h:
+        (WebCore::Node::deref):
+        (WebCore::Node::setParentNode):
+
 2019-03-31  Sam Weinig  <[email protected]>
 
         Remove more i386 specific configurations

Modified: trunk/Source/WebCore/dom/Document.cpp (243685 => 243686)


--- trunk/Source/WebCore/dom/Document.cpp	2019-04-01 01:38:09 UTC (rev 243685)
+++ trunk/Source/WebCore/dom/Document.cpp	2019-04-01 02:03:14 UTC (rev 243686)
@@ -668,7 +668,7 @@
     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()
+        // 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

Modified: trunk/Source/WebCore/dom/Node.cpp (243685 => 243686)


--- trunk/Source/WebCore/dom/Node.cpp	2019-04-01 01:38:09 UTC (rev 243685)
+++ trunk/Source/WebCore/dom/Node.cpp	2019-04-01 02:03:14 UTC (rev 243686)
@@ -332,8 +332,6 @@
 Node::~Node()
 {
     ASSERT(isMainThread());
-    // We set m_refCount to 2 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_deletionHasBegun);
     ASSERT(!m_adoptionIsRequired);
@@ -2514,8 +2512,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

Modified: trunk/Source/WebCore/dom/Node.h (243685 => 243686)


--- trunk/Source/WebCore/dom/Node.h	2019-04-01 01:38:09 UTC (rev 243685)
+++ trunk/Source/WebCore/dom/Node.h	2019-04-01 02:03:14 UTC (rev 243686)
@@ -618,7 +618,7 @@
     Node(Document&, ConstructionType);
 
     static constexpr uint32_t s_refCountIncrement = 2;
-    static constexpr uint32_t s_refCountMask = ~static_cast<uint32_t>(0x1);
+    static constexpr uint32_t s_refCountMask = ~static_cast<uint32_t>(1);
 
     virtual void addSubresourceAttributeURLs(ListHashSet<URL>&) const { }
 
@@ -707,8 +707,10 @@
     ASSERT(!m_deletionHasBegun);
     ASSERT(!m_inRemovedLastRefFunction);
     ASSERT(!m_adoptionIsRequired);
-    auto tempRefCount = m_refCountAndParentBit - s_refCountIncrement;
-    if (!tempRefCount) {
+    auto updatedRefCount = m_refCountAndParentBit - s_refCountIncrement;
+    if (!updatedRefCount) {
+        // Don't update m_refCountAndParentBit to 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.)
 #ifndef NDEBUG
         m_inRemovedLastRefFunction = true;
 #endif
@@ -715,7 +717,7 @@
         removedLastRef();
         return;
     }
-    m_refCountAndParentBit = tempRefCount;
+    m_refCountAndParentBit = updatedRefCount;
 }
 
 ALWAYS_INLINE bool Node::hasOneRef() const
@@ -741,8 +743,7 @@
 {
     ASSERT(isMainThread());
     m_parentNode = parent;
-    auto refCountWithoutParentBit = m_refCountAndParentBit & s_refCountMask;
-    m_refCountAndParentBit = refCountWithoutParentBit | !!parent;
+    m_refCountAndParentBit = (m_refCountAndParentBit & s_refCountMask) | !!parent;
 }
 
 inline ContainerNode* Node::parentNode() const

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

Reply via email to