Invalidate objects when containing block changed

Invalidate objects when containing block changed

When containing block changed, the positioning and paint invalidation
container may also change. Invalidate objects when containing block
changes.

Also added missing logic about fixed-pos element changing containing
block (when some ancestor changes transform related properties).

BUG=548495
TEST=compositing/will-change/containing-block-added.html
TEST=compositing/will-change/containing-block-removed.html

Committed: https://crrev.com/65d58504297a8eaf8225854806e7c66bf12fdd4c
Cr-Commit-Position: refs/heads/master@{#357228}

[Xianzhu, 2015-10-30 18:20:57]

wangxianzhu@chromium.org changed reviewers:
+ chrishtr@chromium.org

[chrishtr, 2015-10-30 18:28:18]

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/compositing/will-change/containing-block-added.html
(right):

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/compositing/will-change/containing-block-added.html:31:
document.getElementById("container").style.transform = 'translateZ(0)'; //
willChange = "transform";
It's not possible to set willChange directly?

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/repaint/stacking-context-lost-expected.txt
(left):

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/repaint/stacking-context-lost-expected.txt:8:
"repaintRects": [
Why would this CL lead to less invalidation?

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBlock.cpp (right):

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBlock.cpp:232: if (oldStyle &&
parent() && !isLayoutView()) {
Why this change?

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.h:773: // Finds the container
as if this object is a fixed position element.
Is it ever called for an object which is not fixed position?

[Xianzhu, 2015-10-30 19:15:28]

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/compositing/will-change/containing-block-added.html
(right):

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/compositing/will-change/containing-block-added.html:31:
document.getElementById("container").style.transform = 'translateZ(0)'; //
willChange = "transform";
On 2015/10/30 18:28:18, chrishtr wrote:
> It's not possible to set willChange directly?

Oops, I forgot to revert a temporary local change for testing.

Reverted.

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/repaint/stacking-context-lost-expected.txt
(left):

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/repaint/stacking-context-lost-expected.txt:8:
"repaintRects": [
On 2015/10/30 18:28:18, chrishtr wrote:
> Why would this CL lead to less invalidation?

Previously spurious invalidation occurs in the following case:
1. the object is invalidated on the previous paint invalidation container;
2. the object is moved to the new paint invalidation container;
3. the object is removed or changed.

The invalidation is issued at step 3 for the object's
previousPaintInvalidationRect on the original paint invalidation container which
is incorrect. Now we clear it when previous paint invalidation rect is
invalidated on the original paint invalidation container, and there will be no
invalidation for previousPaintInvalidation after paint invalidation container
changes.

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBlock.cpp (right):

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBlock.cpp:232: if (oldStyle &&
parent() && !isLayoutView()) {
On 2015/10/30 18:28:18, chrishtr wrote:
> Why this change?

Added this because I thought layout view never changes its relationship with its
contained positioned objects because of its style change. The check seems
unnecessary because LayoutView never changes position or transformed related
properties.

Reverted.

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.h:773: // Finds the container
as if this object is a fixed position element.
On 2015/10/30 18:28:18, chrishtr wrote:
> Is it ever called for an object which is not fixed position?

Now LayoutBlock::styleWillChange() calls this to find the current containing
block of its fixed-position descendants. The block may become a containing
block, so it needs to find the original containing block of the descendants and
change their containing block relationships.

Now the method works in the similar way as containingBlockForAbsolutePosition()
which also doesn't require the caller is absolute-position.

[Xianzhu, 2015-10-30 19:44:40]

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/repaint/stacking-context-lost-expected.txt
(left):

https://codereview.chromium.org/1413823005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/repaint/stacking-context-lost-expected.txt:8:
"repaintRects": [
On 2015/10/30 19:15:28, Xianzhu wrote:
> On 2015/10/30 18:28:18, chrishtr wrote:
> > Why would this CL lead to less invalidation?
> 
> Previously spurious invalidation occurs in the following case:
> 1. the object is invalidated on the previous paint invalidation container;
> 2. the object is moved to the new paint invalidation container;
> 3. the object is removed or changed.
> 
> The invalidation is issued at step 3 for the object's
> previousPaintInvalidationRect on the original paint invalidation container
which
> is incorrect. Now we clear it when previous paint invalidation rect is
> invalidated on the original paint invalidation container, and there will be no
> invalidation for previousPaintInvalidation after paint invalidation container
> changes.

Rereading found this sentence is not clear:

"The invalidation is issued at step 3 for the object's
previousPaintInvalidationRect on the original paint invalidation container which
is incorrect."

Explanation: The previousPaintInvalidationRect was calculated on the original
paint invalidation container, so it's incorrect to invalidate it on the new
paint invalidation container.

[chrishtr, 2015-10-30 20:45:51]

https://codereview.chromium.org/1413823005/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/platform/linux/fast/repaint/invalidation-after-opacity-change-subtree-expected.txt
(right):

https://codereview.chromium.org/1413823005/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/platform/linux/fast/repaint/invalidation-after-opacity-change-subtree-expected.txt:13:
[8, 2000, 777, 350],
Why the new invalidations on this test?

https://codereview.chromium.org/1413823005/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/1413823005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.h:775: LayoutBlock*
containingBlockForAbsolutePosition() const;
Add the same type of comment here as for fixed position.

[Xianzhu, 2015-10-30 20:52:15]

https://codereview.chromium.org/1413823005/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/platform/linux/fast/repaint/invalidation-after-opacity-change-subtree-expected.txt
(right):

https://codereview.chromium.org/1413823005/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/platform/linux/fast/repaint/invalidation-after-opacity-change-subtree-expected.txt:13:
[8, 2000, 777, 350],
On 2015/10/30 20:45:51, chrishtr wrote:
> Why the new invalidations on this test?

The added invalidatPaintIncludingNonCompositingDescendants() causes this extra
invalidation, while the test previously also resulted the same rect in other
paths. This additional invalidaiton is harmless because it doesn't invalidate
more than needed.

https://codereview.chromium.org/1413823005/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/1413823005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.h:775: LayoutBlock*
containingBlockForAbsolutePosition() const;
On 2015/10/30 20:45:51, chrishtr wrote:
> Add the same type of comment here as for fixed position.

Done.

[chrishtr, 2015-10-30 21:11:28]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2015-10-30 21:11:29]

lgtm

[commit-bot: I haz the power, 2015-10-30 21:12:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1413823005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1413823005/80001

[commit-bot: I haz the power, 2015-10-30 22:54:02]

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

[commit-bot: I haz the power, 2015-10-30 22:54:02]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Xianzhu, 2015-10-30 23:08:54]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2015-10-30 23:10:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1413823005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1413823005/80001

[commit-bot: I haz the power, 2015-10-30 23:46:32]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-10-30 23:47:21]

Patchset 5 (id:??) landed as
https://crrev.com/65d58504297a8eaf8225854806e7c66bf12fdd4c
Cr-Commit-Position: refs/heads/master@{#357228}