Title: [223415] trunk/Source
Revision
223415
Author
fpi...@apple.com
Date
2017-10-16 10:19:11 -0700 (Mon, 16 Oct 2017)

Log Message

Make some asserts into release asserts
https://bugs.webkit.org/show_bug.cgi?id=178324

Reviewed by Saam Barati.
Source/_javascript_Core:

        
These asserts are not on perf critical paths, so they might as well be release asserts.

* runtime/DataView.h:
(JSC::DataView::get):
(JSC::DataView::set):

Source/WebCore:


No new tests because no change in behavior.
        
This introduces some release asserts. Perf testing shows that it's neutral. So, we get some extra
safety without losing any perf.

* dom/ContainerNodeAlgorithms.cpp:
(WebCore::notifyChildNodeInserted):
* dom/Document.cpp:
(WebCore::Document::adoptNode):
(WebCore::Document::frameDestroyed):
(WebCore::Document::attachToCachedFrame):
(WebCore::Document::detachFromCachedFrame):
(WebCore::Document::prepareForDestruction):
(WebCore::Document::dispatchWindowEvent):
(WebCore::Document::dispatchWindowLoadEvent):
(WebCore::Document::applyQuickLookSandbox):
* dom/DocumentOrderedMap.cpp:
(WebCore::DocumentOrderedMap::add):
(WebCore::DocumentOrderedMap::remove):
(WebCore::DocumentOrderedMap::get const):
(WebCore:: const):
* dom/Node.cpp:
(WebCore::Node::~Node):
(WebCore::DidMoveToNewDocumentAssertionScope::~DidMoveToNewDocumentAssertionScope):
(WebCore::DidMoveToNewDocumentAssertionScope::didRecieveCall):
(WebCore::moveNodeToNewDocument):
(WebCore::moveShadowTreeToNewDocument):
(WebCore::Node::moveTreeToNewScope):
(WebCore::Node::didMoveToNewDocument):
(WebCore::Node::dispatchSubtreeModifiedEvent):
(WebCore::Node::dispatchDOMActivateEvent):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (223414 => 223415)


--- trunk/Source/_javascript_Core/ChangeLog	2017-10-16 17:18:26 UTC (rev 223414)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-10-16 17:19:11 UTC (rev 223415)
@@ -1,3 +1,16 @@
+2017-10-15  Filip Pizlo  <fpi...@apple.com>
+
+        Make some asserts into release asserts
+        https://bugs.webkit.org/show_bug.cgi?id=178324
+
+        Reviewed by Saam Barati.
+        
+        These asserts are not on perf critical paths, so they might as well be release asserts.
+
+        * runtime/DataView.h:
+        (JSC::DataView::get):
+        (JSC::DataView::set):
+
 2017-10-16  JF Bastien  <jfbast...@apple.com>
 
         JSRunLoopTimer: reduce likely race when used improperly

Modified: trunk/Source/_javascript_Core/runtime/DataView.h (223414 => 223415)


--- trunk/Source/_javascript_Core/runtime/DataView.h	2017-10-16 17:18:26 UTC (rev 223414)
+++ trunk/Source/_javascript_Core/runtime/DataView.h	2017-10-16 17:19:11 UTC (rev 223415)
@@ -60,7 +60,7 @@
             }
             *status = true;
         } else
-            ASSERT_WITH_SECURITY_IMPLICATION(offset + sizeof(T) <= byteLength());
+            RELEASE_ASSERT(offset + sizeof(T) <= byteLength());
         return flipBytesIfLittleEndian(
             *reinterpret_cast<T*>(static_cast<uint8_t*>(m_baseAddress.get()) + offset),
             littleEndian);
@@ -85,7 +85,7 @@
             }
             *status = true;
         } else
-            ASSERT_WITH_SECURITY_IMPLICATION(offset + sizeof(T) <= byteLength());
+            RELEASE_ASSERT(offset + sizeof(T) <= byteLength());
         *reinterpret_cast<T*>(static_cast<uint8_t*>(m_baseAddress.get()) + offset) =
             flipBytesIfLittleEndian(value, littleEndian);
     }

Modified: trunk/Source/WebCore/ChangeLog (223414 => 223415)


--- trunk/Source/WebCore/ChangeLog	2017-10-16 17:18:26 UTC (rev 223414)
+++ trunk/Source/WebCore/ChangeLog	2017-10-16 17:19:11 UTC (rev 223415)
@@ -1,3 +1,42 @@
+2017-10-15  Filip Pizlo  <fpi...@apple.com>
+
+        Make some asserts into release asserts
+        https://bugs.webkit.org/show_bug.cgi?id=178324
+
+        Reviewed by Saam Barati.
+
+        No new tests because no change in behavior.
+        
+        This introduces some release asserts. Perf testing shows that it's neutral. So, we get some extra
+        safety without losing any perf.
+
+        * dom/ContainerNodeAlgorithms.cpp:
+        (WebCore::notifyChildNodeInserted):
+        * dom/Document.cpp:
+        (WebCore::Document::adoptNode):
+        (WebCore::Document::frameDestroyed):
+        (WebCore::Document::attachToCachedFrame):
+        (WebCore::Document::detachFromCachedFrame):
+        (WebCore::Document::prepareForDestruction):
+        (WebCore::Document::dispatchWindowEvent):
+        (WebCore::Document::dispatchWindowLoadEvent):
+        (WebCore::Document::applyQuickLookSandbox):
+        * dom/DocumentOrderedMap.cpp:
+        (WebCore::DocumentOrderedMap::add):
+        (WebCore::DocumentOrderedMap::remove):
+        (WebCore::DocumentOrderedMap::get const):
+        (WebCore:: const):
+        * dom/Node.cpp:
+        (WebCore::Node::~Node):
+        (WebCore::DidMoveToNewDocumentAssertionScope::~DidMoveToNewDocumentAssertionScope):
+        (WebCore::DidMoveToNewDocumentAssertionScope::didRecieveCall):
+        (WebCore::moveNodeToNewDocument):
+        (WebCore::moveShadowTreeToNewDocument):
+        (WebCore::Node::moveTreeToNewScope):
+        (WebCore::Node::didMoveToNewDocument):
+        (WebCore::Node::dispatchSubtreeModifiedEvent):
+        (WebCore::Node::dispatchDOMActivateEvent):
+
 2017-10-16  Alejandro G. Castro  <a...@igalia.com>
 
         Make RealtimeIncomingAudioSources and RealtimeOutgoingAudioSources port agnostic

Modified: trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp (223414 => 223415)


--- trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp	2017-10-16 17:18:26 UTC (rev 223414)
+++ trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp	2017-10-16 17:19:11 UTC (rev 223415)
@@ -85,7 +85,7 @@
 
 void notifyChildNodeInserted(ContainerNode& insertionPoint, Node& node, NodeVector& postInsertionNotificationTargets)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(insertionPoint));
+    RELEASE_ASSERT(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(insertionPoint));
 
     InspectorInstrumentation::didInsertDOMNode(node.document(), node);
 

Modified: trunk/Source/WebCore/dom/Document.cpp (223414 => 223415)


--- trunk/Source/WebCore/dom/Document.cpp	2017-10-16 17:18:26 UTC (rev 223414)
+++ trunk/Source/WebCore/dom/Document.cpp	2017-10-16 17:19:11 UTC (rev 223415)
@@ -981,8 +981,8 @@
         auto result = source.remove();
         if (result.hasException())
             return result.releaseException();
-        ASSERT_WITH_SECURITY_IMPLICATION(!source.isConnected());
-        ASSERT_WITH_SECURITY_IMPLICATION(!source.parentNode());
+        RELEASE_ASSERT(!source.isConnected());
+        RELEASE_ASSERT(!source.parentNode());
     }
 
     source.setTreeScopeRecursively(*this);
@@ -2209,13 +2209,13 @@
 void Document::frameDestroyed()
 {
     // detachFromFrame() must be called before destroying the Frame.
-    ASSERT_WITH_SECURITY_IMPLICATION(!m_frame);
+    RELEASE_ASSERT(!m_frame);
     FrameDestructionObserver::frameDestroyed();
 }
 
 void Document::attachToCachedFrame(CachedFrameBase& cachedFrame)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(cachedFrame.document() == this);
+    RELEASE_ASSERT(cachedFrame.document() == this);
     ASSERT(cachedFrame.view());
     ASSERT(m_pageCacheState == Document::InPageCache);
     observeFrame(&cachedFrame.view()->frame());
@@ -2224,7 +2224,7 @@
 void Document::detachFromCachedFrame(CachedFrameBase& cachedFrame)
 {
     ASSERT_UNUSED(cachedFrame, cachedFrame.view());
-    ASSERT_WITH_SECURITY_IMPLICATION(cachedFrame.document() == this);
+    RELEASE_ASSERT(cachedFrame.document() == this);
     ASSERT(m_frame == &cachedFrame.view()->frame());
     ASSERT(m_pageCacheState == Document::InPageCache);
     detachFromFrame();
@@ -2358,7 +2358,7 @@
     // Note that m_pageCacheState can be Document::AboutToEnterPageCache if our frame
     // was removed in an onpagehide event handler fired when the top-level frame is
     // about to enter the page cache.
-    ASSERT_WITH_SECURITY_IMPLICATION(m_pageCacheState != Document::InPageCache);
+    RELEASE_ASSERT(m_pageCacheState != Document::InPageCache);
 }
 
 void Document::removeAllEventListeners()
@@ -4230,7 +4230,7 @@
 
 void Document::dispatchWindowEvent(Event& event, EventTarget* target)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventAllowedInMainThread());
+    RELEASE_ASSERT(NoEventDispatchAssertion::isEventAllowedInMainThread());
     if (!m_domWindow)
         return;
     m_domWindow->dispatchEvent(event, target);
@@ -4238,7 +4238,7 @@
 
 void Document::dispatchWindowLoadEvent()
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventAllowedInMainThread());
+    RELEASE_ASSERT(NoEventDispatchAssertion::isEventAllowedInMainThread());
     if (!m_domWindow)
         return;
     m_domWindow->dispatchLoadEvent();
@@ -7085,7 +7085,7 @@
     setSecurityOriginPolicy(SecurityOriginPolicy::create(WTFMove(securityOrigin)));
 
     static NeverDestroyed<String> quickLookCSP = makeString("default-src ", QLPreviewProtocol(), ": 'unsafe-inline'; base-uri 'none'; sandbox allow-same-origin allow-scripts");
-    ASSERT_WITH_SECURITY_IMPLICATION(contentSecurityPolicy());
+    RELEASE_ASSERT(contentSecurityPolicy());
     // The sandbox directive is only allowed if the policy is from an HTTP header.
     contentSecurityPolicy()->didReceiveHeader(quickLookCSP, ContentSecurityPolicyHeaderType::Enforce, ContentSecurityPolicy::PolicyFrom::HTTPHeader);
 

Modified: trunk/Source/WebCore/dom/DocumentOrderedMap.cpp (223414 => 223415)


--- trunk/Source/WebCore/dom/DocumentOrderedMap.cpp	2017-10-16 17:18:26 UTC (rev 223414)
+++ trunk/Source/WebCore/dom/DocumentOrderedMap.cpp	2017-10-16 17:19:11 UTC (rev 223415)
@@ -49,7 +49,7 @@
 void DocumentOrderedMap::add(const AtomicStringImpl& key, Element& element, const TreeScope& treeScope)
 {
     UNUSED_PARAM(treeScope);
-    ASSERT_WITH_SECURITY_IMPLICATION(element.isInTreeScope());
+    RELEASE_ASSERT(element.isInTreeScope());
     ASSERT_WITH_SECURITY_IMPLICATION(treeScope.rootNode().containsIncludingShadowDOM(&element));
 
     if (!element.isInTreeScope())
@@ -67,7 +67,7 @@
     if (addResult.isNewEntry)
         return;
 
-    ASSERT_WITH_SECURITY_IMPLICATION(entry.count);
+    RELEASE_ASSERT(entry.count);
     entry.element = nullptr;
     entry.count++;
     entry.orderedList.clear();
@@ -78,15 +78,15 @@
     m_map.checkConsistency();
     auto it = m_map.find(&key);
 
-    ASSERT_WITH_SECURITY_IMPLICATION(it != m_map.end());
+    RELEASE_ASSERT(it != m_map.end());
     if (it == m_map.end())
         return;
 
     MapEntry& entry = it->value;
     ASSERT_WITH_SECURITY_IMPLICATION(entry.registeredElements.remove(&element));
-    ASSERT_WITH_SECURITY_IMPLICATION(entry.count);
+    RELEASE_ASSERT(entry.count);
     if (entry.count == 1) {
-        ASSERT_WITH_SECURITY_IMPLICATION(!entry.element || entry.element == &element);
+        RELEASE_ASSERT(!entry.element || entry.element == &element);
         m_map.remove(it);
     } else {
         if (entry.element == &element)
@@ -108,8 +108,8 @@
     MapEntry& entry = it->value;
     ASSERT(entry.count);
     if (entry.element) {
-        ASSERT_WITH_SECURITY_IMPLICATION(entry.element->isInTreeScope());
-        ASSERT_WITH_SECURITY_IMPLICATION(&entry.element->treeScope() == &scope);
+        RELEASE_ASSERT(entry.element->isInTreeScope());
+        RELEASE_ASSERT(&entry.element->treeScope() == &scope);
         ASSERT_WITH_SECURITY_IMPLICATION(entry.registeredElements.contains(entry.element));
         return entry.element;
     }
@@ -119,8 +119,8 @@
         if (!keyMatches(key, element))
             continue;
         entry.element = &element;
-        ASSERT_WITH_SECURITY_IMPLICATION(element.isInTreeScope());
-        ASSERT_WITH_SECURITY_IMPLICATION(&element.treeScope() == &scope);
+        RELEASE_ASSERT(element.isInTreeScope());
+        RELEASE_ASSERT(&element.treeScope() == &scope);
         ASSERT_WITH_SECURITY_IMPLICATION(entry.registeredElements.contains(entry.element));
         return &element;
     }
@@ -187,7 +187,7 @@
         return nullptr;
 
     MapEntry& entry = it->value;
-    ASSERT_WITH_SECURITY_IMPLICATION(entry.count);
+    RELEASE_ASSERT(entry.count);
     if (!entry.count)
         return nullptr;
 
@@ -202,7 +202,7 @@
                 continue;
             entry.orderedList.append(&element);
         }
-        ASSERT(entry.orderedList.size() == entry.count);
+        RELEASE_ASSERT(entry.orderedList.size() == entry.count);
     }
 
     return &entry.orderedList;

Modified: trunk/Source/WebCore/dom/Node.cpp (223414 => 223415)


--- trunk/Source/WebCore/dom/Node.cpp	2017-10-16 17:18:26 UTC (rev 223414)
+++ trunk/Source/WebCore/dom/Node.cpp	2017-10-16 17:19:11 UTC (rev 223415)
@@ -284,7 +284,7 @@
     liveNodeSet.remove(this);
 #endif
 
-    ASSERT_WITH_SECURITY_IMPLICATION(!renderer());
+    RELEASE_ASSERT(!renderer());
     ASSERT(!parentNode());
     ASSERT(!m_previous);
     ASSERT(!m_next);
@@ -1931,17 +1931,17 @@
 
     ~DidMoveToNewDocumentAssertionScope()
     {
-        ASSERT_WITH_SECURITY_IMPLICATION(m_called);
+        RELEASE_ASSERT(m_called);
         s_scope = m_previousScope;
     }
 
     static void didRecieveCall(Node& node, Document& oldDocument, Document& newDocument)
     {
-        ASSERT_WITH_SECURITY_IMPLICATION(s_scope);
-        ASSERT_WITH_SECURITY_IMPLICATION(!s_scope->m_called);
-        ASSERT_WITH_SECURITY_IMPLICATION(&s_scope->m_node == &node);
-        ASSERT_WITH_SECURITY_IMPLICATION(&s_scope->m_oldDocument == &oldDocument);
-        ASSERT_WITH_SECURITY_IMPLICATION(&s_scope->m_newDocument == &newDocument);
+        RELEASE_ASSERT(s_scope);
+        RELEASE_ASSERT(!s_scope->m_called);
+        RELEASE_ASSERT(&s_scope->m_node == &node);
+        RELEASE_ASSERT(&s_scope->m_oldDocument == &oldDocument);
+        RELEASE_ASSERT(&s_scope->m_newDocument == &newDocument);
         s_scope->m_called = true;
     }
 
@@ -1970,7 +1970,7 @@
     ASSERT(!node.isConnected() || &oldDocument != &newDocument);
     DidMoveToNewDocumentAssertionScope scope(node, oldDocument, newDocument);
     node.didMoveToNewDocument(oldDocument, newDocument);
-    ASSERT_WITH_SECURITY_IMPLICATION(&node.document() == &newDocument);
+    RELEASE_ASSERT(&node.document() == &newDocument);
 }
 
 template <typename MoveNodeFunction, typename MoveShadowRootFunction>
@@ -1998,7 +1998,7 @@
     traverseSubtreeToUpdateTreeScope(shadowRoot, [&oldDocument, &newDocument](Node& node) {
         moveNodeToNewDocument(node, oldDocument, newDocument);
     }, [&oldDocument, &newDocument](ShadowRoot& innerShadowRoot) {
-        ASSERT_WITH_SECURITY_IMPLICATION(&innerShadowRoot.document() == &oldDocument);
+        RELEASE_ASSERT(&innerShadowRoot.document() == &oldDocument);
         moveShadowTreeToNewDocument(innerShadowRoot, oldDocument, newDocument);
     });
 }
@@ -2006,7 +2006,7 @@
 void Node::moveTreeToNewScope(Node& root, TreeScope& oldScope, TreeScope& newScope)
 {
     ASSERT(&oldScope != &newScope);
-    ASSERT_WITH_SECURITY_IMPLICATION(&root.treeScope() == &oldScope);
+    RELEASE_ASSERT(&root.treeScope() == &oldScope);
 
     Document& oldDocument = oldScope.documentScope();
     Document& newDocument = newScope.documentScope();
@@ -2014,8 +2014,8 @@
         oldDocument.incrementReferencingNodeCount();
         traverseSubtreeToUpdateTreeScope(root, [&](Node& node) {
             ASSERT(!node.isTreeScope());
-            ASSERT_WITH_SECURITY_IMPLICATION(&node.treeScope() == &oldScope);
-            ASSERT_WITH_SECURITY_IMPLICATION(&node.document() == &oldDocument);
+            RELEASE_ASSERT(&node.treeScope() == &oldScope);
+            RELEASE_ASSERT(&node.document() == &oldDocument);
             node.setTreeScope(newScope);
             moveNodeToNewDocument(node, oldDocument, newDocument);
         }, [&](ShadowRoot& shadowRoot) {
@@ -2027,7 +2027,7 @@
     } else {
         traverseSubtreeToUpdateTreeScope(root, [&](Node& node) {
             ASSERT(!node.isTreeScope());
-            ASSERT_WITH_SECURITY_IMPLICATION(&node.treeScope() == &oldScope);
+            RELEASE_ASSERT(&node.treeScope() == &oldScope);
             node.setTreeScope(newScope);
             if (!node.hasRareData())
                 return;
@@ -2041,7 +2041,7 @@
 
 void Node::didMoveToNewDocument(Document& oldDocument, Document& newDocument)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(&document() == &newDocument);
+    RELEASE_ASSERT(&document() == &newDocument);
     DidMoveToNewDocumentAssertionScope::didRecieveCall(*this, oldDocument, newDocument);
 
     newDocument.incrementReferencingNodeCount();
@@ -2382,7 +2382,7 @@
     if (isInShadowTree())
         return;
 
-    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(*this));
+    RELEASE_ASSERT(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(*this));
 
     if (!document().hasListenerType(Document::DOMSUBTREEMODIFIED_LISTENER))
         return;
@@ -2395,7 +2395,7 @@
 
 bool Node::dispatchDOMActivateEvent(int detail, Event& underlyingEvent)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventAllowedInMainThread());
+    RELEASE_ASSERT(NoEventDispatchAssertion::isEventAllowedInMainThread());
     Ref<UIEvent> event = UIEvent::create(eventNames().DOMActivateEvent, true, true, document().defaultView(), detail);
     event->setUnderlyingEvent(&underlyingEvent);
     dispatchScopedEvent(event);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to