Reschedule sibling invalidations as descendant on removal.

Reschedule sibling invalidations as descendant on removal.

When removing elements we schedule sibling invalidations based on
element attributes and state as descendant invalidations when
necessary. However, that didn't work correctly if we removed an
attribute and then removed the element before the sibling invalidation
for the attribute was effectuated.

For instance, if you remove a class affecting succeeding siblings
through selectors, we schedule an invalidation set for that change, but
it will be cleared right after if we remove the element (see the added
test).

Instead we reschedule sibling invalidations on the parent node before
the invalidations for the removed element are cleared.

R=esprehn@chromium.org
BUG=674326
Committed: https://crrev.com/aa0b505f520f9d14f7f90048e4a256dd408da849
Cr-Commit-Position: refs/heads/master@{#440598}

[rune, 2016-12-22 14:40:46]

The CQ bit was checked by rune@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-22 14:40:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rune, 2016-12-22 14:40:55]

ptal

[commit-bot: I haz the power, 2016-12-22 16:08:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-22 16:08:18]

Dry run: This issue passed the CQ dry run.

[esprehn, 2016-12-22 17:19:59]

lgtm

https://codereview.chromium.org/2592423002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/invalidation/StyleInvalidator.cpp
(right):

https://codereview.chromium.org/2592423002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/invalidation/StyleInvalidator.cpp:150:
DCHECK(element.parentNode());
I personally like to put these at the top of the function to make the call
invariants clear.

https://codereview.chromium.org/2592423002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/2592423002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.cpp:4223:
styleEngine().elementWillBeRemoved(toElement(n));
Hooking here is unusual, we'd normally do Element::removed from and pass the
parent node into the thing. We should figure out how to reconcile the code here
and the code there. :/

[rune, 2016-12-23 00:06:37]

https://codereview.chromium.org/2592423002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/2592423002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.cpp:4223:
styleEngine().elementWillBeRemoved(toElement(n));
On 2016/12/22 17:19:59, esprehn wrote:
> Hooking here is unusual, we'd normally do Element::removed from and pass the
> parent node into the thing. We should figure out how to reconcile the code
here
> and the code there. :/

You were thinking along the lines of having a willBeRemovedFrom in Node along
insertedInto and removedFrom? And have the implementation on Element call
StyleEngine to reschedule its invalidation set?

[rune, 2016-12-23 00:15:20]

https://codereview.chromium.org/2592423002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/invalidation/StyleInvalidator.cpp
(right):

https://codereview.chromium.org/2592423002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/invalidation/StyleInvalidator.cpp:150:
DCHECK(element.parentNode());
On 2016/12/22 17:19:59, esprehn wrote:
> I personally like to put these at the top of the function to make the call
> invariants clear.

Done.

[rune, 2016-12-23 00:15:34]

The CQ bit was checked by rune@opera.com

[rune, 2016-12-23 00:15:34]

The patchset sent to the CQ was uploaded after l-g-t-m from esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/2592423002/#ps20001
(title: "Moved DCHECK.")

[commit-bot: I haz the power, 2016-12-23 00:16:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-23 02:10:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-23 02:10:43]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[rune, 2016-12-23 07:03:26]

The CQ bit was checked by rune@opera.com

[commit-bot: I haz the power, 2016-12-23 07:03:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-23 07:51:30]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1482476606244790,
"parent_rev": "34a46b469f00fb104eefaa8edacb4b7fbdc4edeb", "commit_rev":
"e8fe54fe0e3dd9a6f655fa4bef972ff752dece38"}

[commit-bot: I haz the power, 2016-12-23 07:51:54]

Description was changed from

==========
Reschedule sibling invalidations as descendant on removal.

When removing elements we schedule sibling invalidations based on
element attributes and state as descendant invalidations when
necessary. However, that didn't work correctly if we removed an
attribute and then removed the element before the sibling invalidation
for the attribute was effectuated.

For instance, if you remove a class affecting succeeding siblings
through selectors, we schedule an invalidation set for that change, but
it will be cleared right after if we remove the element (see the added
test).

Instead we reschedule sibling invalidations on the parent node before
the invalidations for the removed element are cleared.

R=esprehn@chromium.org
BUG=674326
==========

to

==========
Reschedule sibling invalidations as descendant on removal.

When removing elements we schedule sibling invalidations based on
element attributes and state as descendant invalidations when
necessary. However, that didn't work correctly if we removed an
attribute and then removed the element before the sibling invalidation
for the attribute was effectuated.

For instance, if you remove a class affecting succeeding siblings
through selectors, we schedule an invalidation set for that change, but
it will be cleared right after if we remove the element (see the added
test).

Instead we reschedule sibling invalidations on the parent node before
the invalidations for the removed element are cleared.

R=esprehn@chromium.org
BUG=674326

Review-Url: https://codereview.chromium.org/2592423002
==========

[commit-bot: I haz the power, 2016-12-23 07:51:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-23 07:53:49]

Description was changed from

==========
Reschedule sibling invalidations as descendant on removal.

When removing elements we schedule sibling invalidations based on
element attributes and state as descendant invalidations when
necessary. However, that didn't work correctly if we removed an
attribute and then removed the element before the sibling invalidation
for the attribute was effectuated.

For instance, if you remove a class affecting succeeding siblings
through selectors, we schedule an invalidation set for that change, but
it will be cleared right after if we remove the element (see the added
test).

Instead we reschedule sibling invalidations on the parent node before
the invalidations for the removed element are cleared.

R=esprehn@chromium.org
BUG=674326

Review-Url: https://codereview.chromium.org/2592423002
==========

to

==========
Reschedule sibling invalidations as descendant on removal.

When removing elements we schedule sibling invalidations based on
element attributes and state as descendant invalidations when
necessary. However, that didn't work correctly if we removed an
attribute and then removed the element before the sibling invalidation
for the attribute was effectuated.

For instance, if you remove a class affecting succeeding siblings
through selectors, we schedule an invalidation set for that change, but
it will be cleared right after if we remove the element (see the added
test).

Instead we reschedule sibling invalidations on the parent node before
the invalidations for the removed element are cleared.

R=esprehn@chromium.org
BUG=674326

Committed: https://crrev.com/aa0b505f520f9d14f7f90048e4a256dd408da849
Cr-Commit-Position: refs/heads/master@{#440598}
==========

[commit-bot: I haz the power, 2016-12-23 07:53:50]

Patchset 2 (id:??) landed as
https://crrev.com/aa0b505f520f9d14f7f90048e4a256dd408da849
Cr-Commit-Position: refs/heads/master@{#440598}