Revert "Move some scroll invalidations to the paint invalidation phase"

Revert "Move some scroll invalidations to the paint invalidation phase"

This change partly reverts
https://codereview.chromium.org/498773007

This is blocked on fixing issues with incorrect paint
invalidation containers. Those are complicated matters
and require a lot of thinking to get right. It's also not
clear the amount of work required to fix this more fundamental
issue and we need something to fix the regression.

This change doesn't re-introduce the
updateFixedElementPaintInvalidationRectsAfterScroll
logic as it should be taken care by the paint
invalidation logic (or covered by some test case).

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

[Julien - ping for review, 2014-11-11 19:42:03]

jchaffraix@chromium.org changed reviewers:
+ dsinclair@chromium.org, wangxianzhu@chromium.org

[Julien - ping for review, 2014-11-11 19:42:03]

Another revert that I think is related to invalidation containers.

[dsinclair, 2014-11-11 19:51:23]

https://codereview.chromium.org/714083003/diff/1/LayoutTests/fast/repaint/scr...
File
LayoutTests/fast/repaint/scroll-stacking-context-backface-visiblity-leaves-traces.html
(right):

https://codereview.chromium.org/714083003/diff/1/LayoutTests/fast/repaint/scr...
LayoutTests/fast/repaint/scroll-stacking-context-backface-visiblity-leaves-traces.html:52:
//window.addEventListener("load", showBug, false);
remove?

https://codereview.chromium.org/714083003/diff/1/Source/core/frame/FrameView.cpp
File Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/714083003/diff/1/Source/core/frame/FrameView....
Source/core/frame/FrameView.cpp:1352: if (child->compositingState() ==
PaintsIntoOwnBacking || child->compositingState() == PaintsIntoGroupedBacking)
Should this be isPaintInvalidationContainer()?

https://codereview.chromium.org/714083003/diff/1/Source/core/frame/FrameView....
Source/core/frame/FrameView.cpp:1378: if (state == PaintsIntoOwnBacking || state
== PaintsIntoGroupedBacking)
Should this stay as layer->isPaintInvalidationContainer()?

https://codereview.chromium.org/714083003/diff/1/Source/core/frame/FrameView....
Source/core/frame/FrameView.cpp:2602: scrollContentsIfNeededRecursive();
Did you mean to move these up into the if (view) {}? If so, you don't need if
(view) again below.

[Julien - ping for review, 2014-11-12 18:46:02]

Thanks Dan, new patch up for review.

https://codereview.chromium.org/714083003/diff/1/LayoutTests/fast/repaint/scr...
File
LayoutTests/fast/repaint/scroll-stacking-context-backface-visiblity-leaves-traces.html
(right):

https://codereview.chromium.org/714083003/diff/1/LayoutTests/fast/repaint/scr...
LayoutTests/fast/repaint/scroll-stacking-context-backface-visiblity-leaves-traces.html:52:
//window.addEventListener("load", showBug, false);
On 2014/11/11 19:51:23, dsinclair wrote:
> remove?

Indeed, cleaned up the test (showBug() wasn't useful too).

https://codereview.chromium.org/714083003/diff/1/Source/core/frame/FrameView.cpp
File Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/714083003/diff/1/Source/core/frame/FrameView....
Source/core/frame/FrameView.cpp:1378: if (state == PaintsIntoOwnBacking || state
== PaintsIntoGroupedBacking)
On 2014/11/11 19:51:23, dsinclair wrote:
> Should this stay as layer->isPaintInvalidationContainer()?

Changed as it made more sense.

https://codereview.chromium.org/714083003/diff/1/Source/core/frame/FrameView....
Source/core/frame/FrameView.cpp:2602: scrollContentsIfNeededRecursive();
On 2014/11/11 19:51:23, dsinclair wrote:
> Did you mean to move these up into the if (view) {}? If so, you don't need if
> (view) again below.

It was a bad merge, fixed.

[dsinclair, 2014-11-12 19:58:33]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2014-11-12 19:58:33]

lgtm

[commit-bot: I haz the power, 2014-11-12 19:59:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/714083003/20001

[Julien - ping for review, 2014-11-12 22:24:37]

The CQ bit was checked by jchaffraix@chromium.org

[commit-bot: I haz the power, 2014-11-12 22:24:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/714083003/40001

[commit-bot: I haz the power, 2014-11-13 00:47:48]

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

[commit-bot: I haz the power, 2014-11-13 00:47:54]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/36178)

[Julien - ping for review, 2014-11-17 18:00:02]

The CQ bit was checked by jchaffraix@chromium.org

[commit-bot: I haz the power, 2014-11-17 18:01:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/714083003/40001

[commit-bot: I haz the power, 2014-11-17 19:51:45]

Committed patchset #3 (id:40001) as 185463