- Revision
- 235830
- Author
- [email protected]
- Date
- 2018-09-08 13:19:22 -0700 (Sat, 08 Sep 2018)
Log Message
Clean up code related to Document node removal
https://bugs.webkit.org/show_bug.cgi?id=189452
Reviewed by Wenson Hsieh.
Replace the "amongChildrenOnly" boolean argument with an enum for clarity.
Rename the remove*OfSubtree functions, because that naming is very unclear.
Instead, use adjust*OnNodeRemoval which better describes what the code does.
* dom/Document.cpp:
(WebCore::isNodeInSubtree):
(WebCore::Document::adjustFocusedNodeOnNodeRemoval):
(WebCore::Document::nodeChildrenWillBeRemoved):
(WebCore::Document::nodeWillBeRemoved):
(WebCore::Document::adjustFocusNavigationNodeOnNodeRemoval):
(WebCore::Document::adjustFullScreenElementOnNodeRemoval):
(WebCore::Document::removeFocusedNodeOfSubtree): Deleted.
(WebCore::Document::removeFocusNavigationNodeOfSubtree): Deleted.
(WebCore::Document::removeFullScreenElementOfSubtree): Deleted.
* dom/Document.h:
* dom/Element.cpp:
(WebCore::Element::removeShadowRoot):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::clear):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (235829 => 235830)
--- trunk/Source/WebCore/ChangeLog 2018-09-08 19:25:10 UTC (rev 235829)
+++ trunk/Source/WebCore/ChangeLog 2018-09-08 20:19:22 UTC (rev 235830)
@@ -1,3 +1,31 @@
+2018-09-08 Simon Fraser <[email protected]>
+
+ Clean up code related to Document node removal
+ https://bugs.webkit.org/show_bug.cgi?id=189452
+
+ Reviewed by Wenson Hsieh.
+
+ Replace the "amongChildrenOnly" boolean argument with an enum for clarity.
+
+ Rename the remove*OfSubtree functions, because that naming is very unclear.
+ Instead, use adjust*OnNodeRemoval which better describes what the code does.
+
+ * dom/Document.cpp:
+ (WebCore::isNodeInSubtree):
+ (WebCore::Document::adjustFocusedNodeOnNodeRemoval):
+ (WebCore::Document::nodeChildrenWillBeRemoved):
+ (WebCore::Document::nodeWillBeRemoved):
+ (WebCore::Document::adjustFocusNavigationNodeOnNodeRemoval):
+ (WebCore::Document::adjustFullScreenElementOnNodeRemoval):
+ (WebCore::Document::removeFocusedNodeOfSubtree): Deleted.
+ (WebCore::Document::removeFocusNavigationNodeOfSubtree): Deleted.
+ (WebCore::Document::removeFullScreenElementOfSubtree): Deleted.
+ * dom/Document.h:
+ * dom/Element.cpp:
+ (WebCore::Element::removeShadowRoot):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::clear):
+
2018-09-08 Yusuke Suzuki <[email protected]>
[CSSJIT] Use lshiftPtr instead of mul32
Modified: trunk/Source/WebCore/dom/Document.cpp (235829 => 235830)
--- trunk/Source/WebCore/dom/Document.cpp 2018-09-08 19:25:10 UTC (rev 235829)
+++ trunk/Source/WebCore/dom/Document.cpp 2018-09-08 20:19:22 UTC (rev 235830)
@@ -3824,15 +3824,15 @@
audioProducer->pageMutedStateDidChange();
}
-static bool isNodeInSubtree(Node& node, Node& container, bool amongChildrenOnly)
+static bool isNodeInSubtree(Node& node, Node& container, Document::NodeRemoval nodeRemoval)
{
- if (amongChildrenOnly)
+ if (nodeRemoval == Document::NodeRemoval::ChildrenOfNode)
return node.isDescendantOf(container);
- else
- return &node == &container || node.isDescendantOf(container);
+
+ return &node == &container || node.isDescendantOf(container);
}
-void Document::removeFocusedNodeOfSubtree(Node& node, bool amongChildrenOnly)
+void Document::adjustFocusedNodeOnNodeRemoval(Node& node, NodeRemoval nodeRemoval)
{
if (!m_focusedElement || pageCacheState() != NotInPageCache) // If the document is in the page cache, then we don't need to clear out the focused node.
return;
@@ -3840,8 +3840,8 @@
Element* focusedElement = node.treeScope().focusedElementInScope();
if (!focusedElement)
return;
-
- if (isNodeInSubtree(*focusedElement, node, amongChildrenOnly)) {
+
+ if (isNodeInSubtree(*focusedElement, node, nodeRemoval)) {
// FIXME: We should avoid synchronously updating the style inside setFocusedElement.
// FIXME: Object elements should avoid loading a frame synchronously in a post style recalc callback.
SubframeLoadingDisabler disabler(is<ContainerNode>(node) ? &downcast<ContainerNode>(node) : nullptr);
@@ -4201,11 +4201,11 @@
{
ASSERT(!ScriptDisallowedScope::InMainThread::isScriptAllowed());
- removeFocusedNodeOfSubtree(container, true /* amongChildrenOnly */);
- removeFocusNavigationNodeOfSubtree(container, true /* amongChildrenOnly */);
+ adjustFocusedNodeOnNodeRemoval(container, NodeRemoval::ChildrenOfNode);
+ adjustFocusNavigationNodeOnNodeRemoval(container, NodeRemoval::ChildrenOfNode);
#if ENABLE(FULLSCREEN_API)
- removeFullScreenElementOfSubtree(container, true /* amongChildrenOnly */);
+ adjustFullScreenElementOnNodeRemoval(container, NodeRemoval::ChildrenOfNode);
#endif
for (auto* range : m_ranges)
@@ -4234,11 +4234,11 @@
{
ASSERT(!ScriptDisallowedScope::InMainThread::isScriptAllowed());
- removeFocusedNodeOfSubtree(node);
- removeFocusNavigationNodeOfSubtree(node);
+ adjustFocusedNodeOnNodeRemoval(node);
+ adjustFocusNavigationNodeOnNodeRemoval(node);
#if ENABLE(FULLSCREEN_API)
- removeFullScreenElementOfSubtree(node);
+ adjustFullScreenElementOnNodeRemoval(node);
#endif
for (auto* it : m_nodeIterators)
@@ -4262,13 +4262,13 @@
return node.previousSibling() ? node.previousSibling() : node.parentNode();
}
-void Document::removeFocusNavigationNodeOfSubtree(Node& node, bool amongChildrenOnly)
+void Document::adjustFocusNavigationNodeOnNodeRemoval(Node& node, NodeRemoval nodeRemoval)
{
if (!m_focusNavigationStartingNode)
return;
- if (isNodeInSubtree(*m_focusNavigationStartingNode, node, amongChildrenOnly)) {
- m_focusNavigationStartingNode = amongChildrenOnly ? &node : fallbackFocusNavigationStartingNodeAfterRemoval(node);
+ if (isNodeInSubtree(*m_focusNavigationStartingNode, node, nodeRemoval)) {
+ m_focusNavigationStartingNode = (nodeRemoval == NodeRemoval::ChildrenOfNode) ? &node : fallbackFocusNavigationStartingNodeAfterRemoval(node);
m_focusNavigationStartingNodeIsRemoved = true;
}
}
@@ -6466,13 +6466,13 @@
webkitCancelFullScreen();
}
-void Document::removeFullScreenElementOfSubtree(Node& node, bool amongChildrenOnly)
+void Document::adjustFullScreenElementOnNodeRemoval(Node& node, NodeRemoval nodeRemoval)
{
if (!m_fullScreenElement)
return;
bool elementInSubtree = false;
- if (amongChildrenOnly)
+ if (nodeRemoval == NodeRemoval::ChildrenOfNode)
elementInSubtree = m_fullScreenElement->isDescendantOf(node);
else
elementInSubtree = (m_fullScreenElement == &node) || m_fullScreenElement->isDescendantOf(node);
Modified: trunk/Source/WebCore/dom/Document.h (235829 => 235830)
--- trunk/Source/WebCore/dom/Document.h 2018-09-08 19:25:10 UTC (rev 235829)
+++ trunk/Source/WebCore/dom/Document.h 2018-09-08 20:19:22 UTC (rev 235830)
@@ -757,7 +757,10 @@
void setFocusNavigationStartingNode(Node*);
Element* focusNavigationStartingNode(FocusDirection) const;
- void removeFocusedNodeOfSubtree(Node&, bool amongChildrenOnly = false);
+ enum class NodeRemoval { Node, ChildrenOfNode };
+ void adjustFocusedNodeOnNodeRemoval(Node&, NodeRemoval = NodeRemoval::Node);
+ void adjustFocusNavigationNodeOnNodeRemoval(Node&, NodeRemoval = NodeRemoval::Node);
+
void hoveredElementDidDetach(Element*);
void elementInActiveChainDidDetach(Element*);
@@ -802,7 +805,7 @@
void nodeChildrenWillBeRemoved(ContainerNode&);
// nodeWillBeRemoved is only safe when removing one node at a time.
void nodeWillBeRemoved(Node&);
- void removeFocusNavigationNodeOfSubtree(Node&, bool amongChildrenOnly = false);
+
enum class AcceptChildOperation { Replace, InsertOrAdd };
bool canAcceptChild(const Node& newChild, const Node* refChild, AcceptChildOperation) const;
@@ -1177,7 +1180,8 @@
void dispatchFullScreenChangeEvents();
bool fullScreenIsAllowedForElement(Element*) const;
void fullScreenElementRemoved();
- void removeFullScreenElementOfSubtree(Node&, bool amongChildrenOnly = false);
+ void adjustFullScreenElementOnNodeRemoval(Node&, NodeRemoval = NodeRemoval::Node);
+
WEBCORE_EXPORT bool isAnimatingFullScreen() const;
WEBCORE_EXPORT void setAnimatingFullScreen(bool);
Modified: trunk/Source/WebCore/dom/Element.cpp (235829 => 235830)
--- trunk/Source/WebCore/dom/Element.cpp 2018-09-08 19:25:10 UTC (rev 235829)
+++ trunk/Source/WebCore/dom/Element.cpp 2018-09-08 20:19:22 UTC (rev 235830)
@@ -2017,7 +2017,7 @@
return;
InspectorInstrumentation::willPopShadowRoot(*this, *oldRoot);
- document().removeFocusedNodeOfSubtree(*oldRoot);
+ document().adjustFocusedNodeOnNodeRemoval(*oldRoot);
ASSERT(!oldRoot->renderer());
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (235829 => 235830)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2018-09-08 19:25:10 UTC (rev 235829)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2018-09-08 20:19:22 UTC (rev 235830)
@@ -631,7 +631,7 @@
bool hadLivingRenderTree = m_frame.document()->hasLivingRenderTree();
m_frame.document()->prepareForDestruction();
if (hadLivingRenderTree)
- m_frame.document()->removeFocusedNodeOfSubtree(*m_frame.document());
+ m_frame.document()->adjustFocusedNodeOnNodeRemoval(*m_frame.document());
}
// Do this after detaching the document so that the unload event works.
