Make sure slot assignment recalc doesn't happen in node attach (or detach) |
|||
Issue descriptionRecalc slot assignment while in executing node attach (or detach) makes the life cycle of dom tree, slot assignment and layout tree complicated. That would cause a crash which is hard to debug. e.g. http://crbug.com/844277, http://crbug.com/844301 It would be better to make sure that recalc slog assignment never happens when in executing node attach (or detach).
,
May 23 2018
,
May 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6e3be0e92a8efbbd3b1220133fbd3c10364366cd commit 6e3be0e92a8efbbd3b1220133fbd3c10364366cd Author: Hayato Ito <hayato@chromium.org> Date: Wed May 23 05:49:14 2018 Disable Incremental Shadow DOM It would be safe to disable Incremental Shadow DOM for M68, whose branch cut is soon. Let's defer it to M69 so that we can address http://crbug.com/845770 surely. Bug: 776656 , 845770 Change-Id: I5d2efeca4b44c0b4a84948233d7b3447cdf4f64e Reviewed-on: https://chromium-review.googlesource.com/1069952 Reviewed-by: Takayoshi Kochi <kochi@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Commit-Queue: Hayato Ito <hayato@chromium.org> Cr-Commit-Position: refs/heads/master@{#560959} [modify] https://crrev.com/6e3be0e92a8efbbd3b1220133fbd3c10364366cd/third_party/blink/renderer/platform/runtime_enabled_features.json5
,
May 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f8e70c7f029058c698495950da0c57c4e9a0bdb3 commit f8e70c7f029058c698495950da0c57c4e9a0bdb3 Author: Hayato Ito <hayato@chromium.org> Date: Wed May 23 09:14:43 2018 Make slot-assignment-recalc-forbidden scope on a per-document basis We need to increment (and decrement) a counter on a per-document basis because call stack can span across different documents. BUG 776656 , 845770 Change-Id: Ib9e89448217e7e335ba3c32c9775c9aa9c8f28cf Reviewed-on: https://chromium-review.googlesource.com/1069959 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Hayato Ito <hayato@chromium.org> Cr-Commit-Position: refs/heads/master@{#561014} [modify] https://crrev.com/f8e70c7f029058c698495950da0c57c4e9a0bdb3/third_party/blink/renderer/core/dom/document.cc [modify] https://crrev.com/f8e70c7f029058c698495950da0c57c4e9a0bdb3/third_party/blink/renderer/core/dom/document.h [modify] https://crrev.com/f8e70c7f029058c698495950da0c57c4e9a0bdb3/third_party/blink/renderer/core/dom/slot_assignment.cc [modify] https://crrev.com/f8e70c7f029058c698495950da0c57c4e9a0bdb3/third_party/blink/renderer/core/dom/slot_assignment.h
,
May 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/79925868d14e19bb4bfb38d09bdd6565c8f4c9a1 commit 79925868d14e19bb4bfb38d09bdd6565c8f4c9a1 Author: Hayato Ito <hayato@chromium.org> Date: Thu May 24 03:03:47 2018 Don't recalc assignement in HTMLSlotElement::DetachLayoutTree The workaround of https://crrev.com/c/1062990 is no longer necessary because of https://crrev.com/c/1068646, which fixed ListItemOrdinal's behavior. See also http://crbug.com/845770. In general, we should avoid recalc assignment in detaching a node. Bug: 776656 , 845770 Change-Id: Ia98b35dce0b6d2e16829f79dde4b88470ced0d3f Reviewed-on: https://chromium-review.googlesource.com/1070007 Reviewed-by: Rakina Zata Amni <rakina@chromium.org> Commit-Queue: Hayato Ito <hayato@chromium.org> Cr-Commit-Position: refs/heads/master@{#561371} [modify] https://crrev.com/79925868d14e19bb4bfb38d09bdd6565c8f4c9a1/third_party/blink/renderer/core/html/html_slot_element.cc
,
May 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/80c7cd9980fd9e84013896da48b4f068384c575c commit 80c7cd9980fd9e84013896da48b4f068384c575c Author: Hayato Ito <hayato@chromium.org> Date: Fri May 25 06:54:13 2018 [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/1070169 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Hayato Ito <hayato@chromium.org> Cr-Commit-Position: refs/heads/master@{#561792} [modify] https://crrev.com/80c7cd9980fd9e84013896da48b4f068384c575c/third_party/blink/renderer/core/dom/container_node.cc [modify] https://crrev.com/80c7cd9980fd9e84013896da48b4f068384c575c/third_party/blink/renderer/core/dom/document.cc [modify] https://crrev.com/80c7cd9980fd9e84013896da48b4f068384c575c/third_party/blink/renderer/core/dom/document.h [modify] https://crrev.com/80c7cd9980fd9e84013896da48b4f068384c575c/third_party/blink/renderer/core/dom/node.cc [modify] https://crrev.com/80c7cd9980fd9e84013896da48b4f068384c575c/third_party/blink/renderer/core/dom/node.h
,
May 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/48ce351d5715418e735608badc62c2811bb0b80b commit 48ce351d5715418e735608badc62c2811bb0b80b Author: Hayato Ito <hayato@chromium.org> Date: Thu May 31 13:28:50 2018 Add more slot-assignement-recalc-forbidden scoped check This is a follow-up of https://crrev.com/c/1070169. Added scoped checks in: - InsertBefore - ReplaceChild The future plan: The remaining operation we should check is Element::attachShadow, which will be done in another CL. After that, I'll refactor so that this kind of check can be done in more better places. Bug: 845770 Change-Id: I8463c38a6676fd8cee4d300423b74a88e764f12d Reviewed-on: https://chromium-review.googlesource.com/1074786 Commit-Queue: Hayato Ito <hayato@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#563206} [modify] https://crrev.com/48ce351d5715418e735608badc62c2811bb0b80b/third_party/blink/renderer/core/dom/container_node.cc
,
Jun 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/126c8f6a664384fc7d640b25c2ba67cd69525b2d commit 126c8f6a664384fc7d640b25c2ba67cd69525b2d Author: Hayato Ito <hayato@chromium.org> Date: Tue Jun 12 03:16:00 2018 Add slot-assignment-recalc-forbidden scoped check to RemoveChildren This is a follow-up of https://crrev.com/c/1070169. RemoveChildren should be also checked. Bug: 845770 Change-Id: I57fd9806a7b5ac5badda56495300d60c86ce9c3d Reviewed-on: https://chromium-review.googlesource.com/1095116 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Hayato Ito <hayato@chromium.org> Cr-Commit-Position: refs/heads/master@{#566291} [modify] https://crrev.com/126c8f6a664384fc7d640b25c2ba67cd69525b2d/third_party/blink/renderer/core/dom/container_node.cc
,
Jun 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0d36ef23872293080e5ab32edb7b5205f6643200 commit 0d36ef23872293080e5ab32edb7b5205f6643200 Author: Hayato Ito <hayato@chromium.org> Date: Tue Jun 12 04:17:43 2018 Add slot-assignment-recalc-forbidden scoped check to attachShadow This is a follow-up of https://crrev.com/c/1070169. I'll refactor added scoped checks so that this kind of check can be done in more better places. Bug: 845770 Change-Id: I3ed963699cd42fd617f7263803ea2ddae0ff1649 Reviewed-on: https://chromium-review.googlesource.com/1095120 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Hayato Ito <hayato@chromium.org> Cr-Commit-Position: refs/heads/master@{#566297} [modify] https://crrev.com/0d36ef23872293080e5ab32edb7b5205f6643200/third_party/blink/renderer/core/dom/element.cc
,
Jun 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/233a440f297a09f626a35193fc0d1b3d981f5b5b commit 233a440f297a09f626a35193fc0d1b3d981f5b5b Author: Takayoshi Kochi <kochi@chromium.org> Date: Fri Jun 15 14:25:21 2018 Clear slot's assigned nodes when it's inserted and assigned nodes are dirty Between when a host element with its shadow root gets orphaned and when it gets connected again, its slot assignment can become dirty by some DOM mutation of its host children, but slots still hold stale assigned nodes. When a detached host child becomes an ancestor of the shadow host, unless those cached assigned nodes are cleared, it can make a cycle during DetachLayoutTree() traverses down to those cached nodes. See the case (cyclic-detach-crash2.html) for details. Bug: 847056 , 845770 Change-Id: I44d3c118c9810ad3847fa24d630b7ddb9f9d2e50 Reviewed-on: https://chromium-review.googlesource.com/1100718 Reviewed-by: Hayato Ito <hayato@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Takayoshi Kochi <kochi@chromium.org> Cr-Commit-Position: refs/heads/master@{#567636} [modify] https://crrev.com/233a440f297a09f626a35193fc0d1b3d981f5b5b/third_party/WebKit/LayoutTests/shadow-dom/crashes/cyclic-detach-crash.html [add] https://crrev.com/233a440f297a09f626a35193fc0d1b3d981f5b5b/third_party/WebKit/LayoutTests/shadow-dom/crashes/cyclic-detach-crash2.html [modify] https://crrev.com/233a440f297a09f626a35193fc0d1b3d981f5b5b/third_party/blink/renderer/core/html/html_slot_element.cc [modify] https://crrev.com/233a440f297a09f626a35193fc0d1b3d981f5b5b/third_party/blink/renderer/core/html/html_slot_element.h
,
Jun 25 2018
It looks HTMLElement::Directionality is violating the rule, using FlatTreTraversal while in DOM mutations. That causes the crash in bug 855876 .
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7bb0c846bf13f030b5778929a84c676116c509c2 commit 7bb0c846bf13f030b5778929a84c676116c509c2 Author: Hayato Ito <hayato@chromium.org> Date: Tue Jul 10 09:09:22 2018 Introduce SlotAssignmentRecalcForbiddenScope This is a kind of refactoring. It would be better to Introduce SlotAssignmentRecalcForbiddenScope, instead of re-using NestingLevelIncrementor, because it would make the intention clear. Bug: 845770 Change-Id: I8b81df9aa11dbacd2d49bea3f78f16fcda81ad0c Reviewed-on: https://chromium-review.googlesource.com/1128795 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Hayato Ito <hayato@chromium.org> Cr-Commit-Position: refs/heads/master@{#573655} [modify] https://crrev.com/7bb0c846bf13f030b5778929a84c676116c509c2/third_party/blink/renderer/core/dom/BUILD.gn [modify] https://crrev.com/7bb0c846bf13f030b5778929a84c676116c509c2/third_party/blink/renderer/core/dom/container_node.cc [modify] https://crrev.com/7bb0c846bf13f030b5778929a84c676116c509c2/third_party/blink/renderer/core/dom/document.h [add] https://crrev.com/7bb0c846bf13f030b5778929a84c676116c509c2/third_party/blink/renderer/core/dom/slot_assignment_recalc_forbidden_scope.h |
|||
►
Sign in to add a comment |
|||
Comment 1 by hayato@chromium.org
, May 23 2018