Commit 80c7cd99 authored by Hayato Ito's avatar Hayato Ito Committed by Commit Bot

[IncrementalShadowDOM] Add slot-assignement-recalc-forbidden scoped check for DOM mutations

This is similar to https://crrev.com/c/1068571, but does check for DOM mutations.

There are two code paths which violate the assumption:

- ListItemOrdinal::ItemInsertedOrRemoved(), which was fixed at https://crrev.com/c/1068646
- Document::HoveredElementDetached, which is fixed in this CL

The reason we can't add a scoped check at the beginning of ContainerNode::RemoveChild is
that synchronous DOM mutation events can happen in WillRemoveChild(*child) or
DispatchSubtreeModifiedEvent().

Ditto for ContainerNode::AppendChild.

There are other DOM mutation operations where we should add check. That can be done later.
Once I am sure that the coverage is enough, I'll refactor so that this kind of check can be
done in more better places.

Bug: 776656,845770

Change-Id: I7ce0e99292165b698b69c6b90d71d71a90c19135
Reviewed-on: https://chromium-review.googlesource.com/1070169Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Hayato Ito <hayato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561792}
parent c63d70ed
......@@ -47,6 +47,7 @@
#include "third_party/blink/renderer/core/html/html_collection.h"
#include "third_party/blink/renderer/core/html/html_frame_owner_element.h"
#include "third_party/blink/renderer/core/html/html_tag_collection.h"
#include "third_party/blink/renderer/core/html/parser/nesting_level_incrementer.h"
#include "third_party/blink/renderer/core/layout/layout_block_flow.h"
#include "third_party/blink/renderer/core/layout/layout_inline.h"
#include "third_party/blink/renderer/core/layout/layout_text.h"
......@@ -689,6 +690,10 @@ Node* ContainerNode::RemoveChild(Node* old_child,
}
{
#if DCHECK_IS_ON()
NestingLevelIncrementer slot_assignment_recalc_forbidden_scope(
GetDocument().SlotAssignmentRecalcForbiddenRecursionDepth());
#endif
HTMLFrameOwnerElement::PluginDisposeSuspendScope suspend_plugin_dispose;
TreeOrderedMap::RemoveScope tree_remove_scope;
......@@ -830,6 +835,10 @@ Node* ContainerNode::AppendChild(Node* new_child,
NodeVector post_insertion_notification_targets;
{
#if DCHECK_IS_ON()
NestingLevelIncrementer slot_assignment_recalc_forbidden_scope(
GetDocument().SlotAssignmentRecalcForbiddenRecursionDepth());
#endif
ChildListMutationScope mutation(*this);
InsertNodeVector(targets, nullptr, AdoptAndAppendChild(),
&post_insertion_notification_targets);
......
......@@ -4459,8 +4459,14 @@ void Document::HoveredElementDetached(Element& element) {
if (element != hover_element_)
return;
hover_element_->UpdateDistributionForUnknownReasons();
hover_element_ = SkipDisplayNoneAncestors(&element);
// While in detaching, we shouldn't use FlatTreeTraversal if slot assignemnt
// is dirty because it might triger assignement recalc. hover_element_ will be
// updated after recalc assignment is calculated (and re-layout is done).
if (element.IsSlotAssignmentOrLegacyDistributionDirty()) {
hover_element_ = nullptr;
} else {
hover_element_ = SkipDisplayNoneAncestors(&element);
}
// If the mouse cursor is not visible, do not clear existing
// hover effects on the ancestors of |element| and do not invoke
......
......@@ -1419,6 +1419,9 @@ class CORE_EXPORT Document : public ContainerNode,
SlotAssignmentEngine& GetSlotAssignmentEngine();
#if DCHECK_IS_ON()
unsigned& SlotAssignmentRecalcForbiddenRecursionDepth() {
return slot_assignment_recalc_forbidden_recursion_depth_;
}
bool IsSlotAssignmentRecalcForbidden() {
return slot_assignment_recalc_forbidden_recursion_depth_ > 0;
}
......
......@@ -845,6 +845,19 @@ void Node::MarkAncestorsWithChildNeedsDistributionRecalc() {
GetDocument().ScheduleLayoutTreeUpdateIfNeeded();
}
bool Node::IsSlotAssignmentOrLegacyDistributionDirty() const {
if (GetDocument().ChildNeedsDistributionRecalc())
return true;
if (RuntimeEnabledFeatures::IncrementalShadowDOMEnabled()) {
if (ShadowRoot* shadow_root = ContainingShadowRoot()) {
if (shadow_root->NeedsSlotAssignmentRecalc()) {
return true;
}
}
}
return false;
}
inline void Node::SetStyleChange(StyleChangeType change_type) {
node_flags_ = (node_flags_ & ~kStyleChangeMask) | change_type;
}
......
......@@ -458,6 +458,8 @@ class CORE_EXPORT Node : public EventTarget {
}
void MarkAncestorsWithChildNeedsDistributionRecalc();
bool IsSlotAssignmentOrLegacyDistributionDirty() const;
bool ChildNeedsStyleInvalidation() const {
return GetFlag(kChildNeedsStyleInvalidationFlag);
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment