Call ContainerNode::checkForSiblingStyleChanges() for Element insertion / removal only

Call ContainerNode::checkForSiblingStyleChanges() for Element insertion / removal only

Call ContainerNode::checkForSiblingStyleChanges() for Element insertion /
removal only. Other kind of changes (e.g. Text insertion / removal / change)
cannot cause sibling style invalidation.

This is inspired by WebKit r154957 from antti@apple.com:
http://trac.webkit.org/changeset/154957

R=esprehn@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179405

[Inactive, 2014-07-15 17:10:32]

PTAL

[Inactive, 2014-07-23 12:25:11]

ping review?

[Hajime Morrita, 2014-07-23 16:41:28]

On 2014/07/23 12:25:11, Chris Dumez wrote:
> ping review?
^^^ Elliott?

[esprehn, 2014-07-23 16:51:57]

Yeah I'll look

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[esprehn, 2014-07-28 22:02:30]

https://codereview.chromium.org/395633007/diff/40001/Source/core/dom/Containe...
File Source/core/dom/ContainerNode.h (right):

https://codereview.chromium.org/395633007/diff/40001/Source/core/dom/Containe...
Source/core/dom/ContainerNode.h:185: bool isElementChange() const { return type
== ElementInserted || type == ElementRemoved; }
I don't think this is correct, if everything was removed then elements may have
been removed.

[Inactive, 2014-07-28 22:23:28]

https://codereview.chromium.org/395633007/diff/40001/Source/core/dom/Containe...
File Source/core/dom/ContainerNode.h (right):

https://codereview.chromium.org/395633007/diff/40001/Source/core/dom/Containe...
Source/core/dom/ContainerNode.h:185: bool isElementChange() const { return type
== ElementInserted || type == ElementRemoved; }
On 2014/07/28 22:02:30, esprehn wrote:
> I don't think this is correct, if everything was removed then elements may
have
> been removed.

You are right that in case all children were removed, some elements might have
been removed. However, the current call site of this does not need to handle the
removal of all children (there is no need to check for sibling style
invalidation since all siblings have been removed).

Basically, I want this to mean that exactly 1 element for added or removed
(similarly to the isChildInsertion / isChildRemoval above. Apparently the naming
is not clear enough. Would it be OK if I renamed this to something better? Maybe
isChildElementChange()? or isSingleElementChange()?
isElementInsertionOrDeletion()?

[Inactive, 2014-07-29 14:49:42]

I rebased the patch and renamed the method to isChildElementChange(). Hopefully
it is clearer. Please feel free to suggest a better name. I thought
isChildElementInsertionOrRemoval() would be too long.

PTAL.

[esprehn, 2014-08-01 14:12:26]

lgtm

https://codereview.chromium.org/395633007/diff/100001/Source/core/dom/Contain...
File Source/core/dom/ContainerNode.cpp (right):

https://codereview.chromium.org/395633007/diff/100001/Source/core/dom/Contain...
Source/core/dom/ContainerNode.cpp:769: 
why extra nl?

https://codereview.chromium.org/395633007/diff/100001/Source/core/dom/Contain...
Source/core/dom/ContainerNode.cpp:790:
childrenChanged(ChildrenChange::forInsertion(root, source));
This code needs to be rebased again.

[Inactive, 2014-08-01 15:40:08]

https://codereview.chromium.org/395633007/diff/100001/Source/core/dom/Contain...
File Source/core/dom/ContainerNode.cpp (right):

https://codereview.chromium.org/395633007/diff/100001/Source/core/dom/Contain...
Source/core/dom/ContainerNode.cpp:769: 
On 2014/08/01 14:12:26, esprehn wrote:
> why extra nl?

Done.

https://codereview.chromium.org/395633007/diff/100001/Source/core/dom/Contain...
Source/core/dom/ContainerNode.cpp:790:
childrenChanged(ChildrenChange::forInsertion(root, source));
On 2014/08/01 14:12:26, esprehn wrote:
> This code needs to be rebased again.

Done.

[Inactive, 2014-08-01 15:40:13]

The CQ bit was checked by ch.dumez@samsung.com

[commit-bot: I haz the power, 2014-08-01 15:41:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/395633007/120001

[commit-bot: I haz the power, 2014-08-01 16:32:19]

Change committed as 179405