[RAL] Fix nested non-composited RenderView repainting

[RAL] Fix nested non-composited RenderView repainting

When a nested RenderView isn't composited, it acts like a repaint
boundary, but then relies on repaintViewRectangle to translate
the repaint coordinates into the coordinate space of the actual
repaint container. This patch overrides repaintTreeAfterLayout for
RenderView to simply repaint the viewRect. It also pushes some
repaint logic out of RenderObject and into RenderView.

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

[Julien - ping for review, 2014-05-30 02:39:31]

https://codereview.chromium.org/271803002/diff/20001/Source/core/rendering/Re...
File Source/core/rendering/RenderBox.cpp (right):

https://codereview.chromium.org/271803002/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderBox.cpp:1592: LayoutStateMaintainer
statePusher(*this, isTableRow() ? LayoutSize() : locationOffset());
It is weird that we push LayoutState if we don't expect to invalidate (your
patch adds some invalidation though).

https://codereview.chromium.org/271803002/diff/20001/Source/core/rendering/Re...
File Source/core/rendering/RenderView.cpp (right):

https://codereview.chromium.org/271803002/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderView.cpp:441: void
RenderView::repaintTreeAfterLayout(const RenderLayerModelObject&
repaintContainer)
This code path seems very artificial to me as I would expect the FrameView to
handle the full repaint case as it used to do before repaint-after-layout (see
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)

[leviw_travelin_and_unemployed, 2014-05-30 15:52:57]

https://codereview.chromium.org/271803002/diff/20001/Source/core/rendering/Re...
File Source/core/rendering/RenderBox.cpp (right):

https://codereview.chromium.org/271803002/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderBox.cpp:1592: LayoutStateMaintainer
statePusher(*this, isTableRow() ? LayoutSize() : locationOffset());
On 2014/05/30 02:39:32, Julien Chaffraix - PST wrote:
> It is weird that we push LayoutState if we don't expect to invalidate (your
> patch adds some invalidation though).

We still need to generate our cached repaint rects. LayoutState makes that
faster.

https://codereview.chromium.org/271803002/diff/20001/Source/core/rendering/Re...
File Source/core/rendering/RenderView.cpp (right):

https://codereview.chromium.org/271803002/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderView.cpp:441: void
RenderView::repaintTreeAfterLayout(const RenderLayerModelObject&
repaintContainer)
On 2014/05/30 02:39:32, Julien Chaffraix - PST wrote:
> This code path seems very artificial to me as I would expect the FrameView to
> handle the full repaint case as it used to do before repaint-after-layout (see
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)

I'm not sure I understand what the "artificial" part of this comment really
means.

[dsinclair, 2014-06-02 00:38:28]

lgtm

https://codereview.chromium.org/271803002/diff/20001/Source/core/rendering/Re...
File Source/core/rendering/RenderView.cpp (right):

https://codereview.chromium.org/271803002/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderView.cpp:441: void
RenderView::repaintTreeAfterLayout(const RenderLayerModelObject&
repaintContainer)
On 2014/05/30 02:39:32, Julien Chaffraix - PST wrote:
> This code path seems very artificial to me as I would expect the FrameView to
> handle the full repaint case as it used to do before repaint-after-layout (see
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)

That code path is never hit in RAL. I don't necessarily think it should be. I
think it makes sense for all of the invalidating to happen the same way, if
possible, which means the RenderObjects doing the invalidation.

[leviw_travelin_and_unemployed, 2014-06-02 18:49:26]

Jchaffraix, let me know if you still have objections. I'd love to get this
resolved. We're close to being ready to turn RAL on again.

[Julien - ping for review, 2014-06-02 23:21:18]

lgtm

https://codereview.chromium.org/271803002/diff/20001/Source/core/rendering/Re...
File Source/core/rendering/RenderView.cpp (right):

https://codereview.chromium.org/271803002/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderView.cpp:441: void
RenderView::repaintTreeAfterLayout(const RenderLayerModelObject&
repaintContainer)
On 2014/06/02 00:38:29, dsinclair wrote:
> On 2014/05/30 02:39:32, Julien Chaffraix - PST wrote:
> > This code path seems very artificial to me as I would expect the FrameView
to
> > handle the full repaint case as it used to do before repaint-after-layout
(see
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)
> 
> That code path is never hit in RAL. I don't necessarily think it should be. I
> think it makes sense for all of the invalidating to happen the same way, if
> possible, which means the RenderObjects doing the invalidation.

It's not hit by RAL because we made it so, not because we can't have RAL hit it.
That was the fix I had in mind for this change but Levi pointed out it was
busted but hidden by our massive overinvalidation.

[leviw_travelin_and_unemployed, 2014-06-02 23:30:08]

The CQ bit was checked by leviw@chromium.org

[commit-bot: I haz the power, 2014-06-02 23:30:54]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/leviw@chromium.org/271803002/20001

[commit-bot: I haz the power, 2014-06-02 23:31:06]

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

[commit-bot: I haz the power, 2014-06-02 23:31:06]

Failed to apply patch for
LayoutTests/platform/mac/fast/repaint/overflow-scroll-body-appear-expected.txt:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index:
LayoutTests/platform/mac/fast/repaint/overflow-scroll-body-appear-expected.txt
  |diff --git
a/LayoutTests/platform/mac/fast/repaint/overflow-scroll-body-appear-expected.txt
b/LayoutTests/platform/mac/fast/repaint/overflow-scroll-body-appear-expected.txt
  |index
7bebc7c60b1535e3ebc34a83beb55e85d3314cb6..1b856e6b591556998b62ce3817135e3aa7920aeb
100644
  |---
a/LayoutTests/platform/mac/fast/repaint/overflow-scroll-body-appear-expected.txt
  |+++
b/LayoutTests/platform/mac/fast/repaint/overflow-scroll-body-appear-expected.txt
  --------------------------
  No file to patch.  Skipping patch.
  1 out of 1 hunk ignored

Patch:      
LayoutTests/platform/mac/fast/repaint/overflow-scroll-body-appear-expected.txt
Index:
LayoutTests/platform/mac/fast/repaint/overflow-scroll-body-appear-expected.txt
diff --git
a/LayoutTests/platform/mac/fast/repaint/overflow-scroll-body-appear-expected.txt
b/LayoutTests/platform/mac/fast/repaint/overflow-scroll-body-appear-expected.txt
index
7bebc7c60b1535e3ebc34a83beb55e85d3314cb6..1b856e6b591556998b62ce3817135e3aa7920aeb
100644
---
a/LayoutTests/platform/mac/fast/repaint/overflow-scroll-body-appear-expected.txt
+++
b/LayoutTests/platform/mac/fast/repaint/overflow-scroll-body-appear-expected.txt
@@ -6,11 +6,10 @@
       (contentsOpaque 1)
       (drawsContent 1)
       (repaint rects
-        (rect 785.00 0.00 15.00 600.00)
-        (rect 0.00 585.00 800.00 15.00)
         (rect 0.00 0.00 2008.00 2092.00)
         (rect 0.00 0.00 2008.00 2092.00)
         (rect 0.00 0.00 800.00 600.00)
+        (rect 0.00 0.00 785.00 585.00)
       )
     )
   )

[leviw_travelin_and_unemployed, 2014-06-02 23:32:31]

The CQ bit was unchecked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2014-06-03 00:31:55]

The CQ bit was checked by leviw@chromium.org

[commit-bot: I haz the power, 2014-06-03 00:32:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/leviw@chromium.org/271803002/40001

[commit-bot: I haz the power, 2014-06-03 01:35:55]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6736)
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/9963)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10265)

[commit-bot: I haz the power, 2014-06-03 02:21:18]

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

[commit-bot: I haz the power, 2014-06-03 02:21:18]

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

[leviw_travelin_and_unemployed, 2014-06-03 16:03:20]

The CQ bit was checked by leviw@chromium.org

[commit-bot: I haz the power, 2014-06-03 16:03:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/leviw@chromium.org/271803002/60001

[leviw_travelin_and_unemployed, 2014-06-03 16:45:48]

The CQ bit was unchecked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2014-06-03 17:48:47]

The CQ bit was checked by leviw@chromium.org

[commit-bot: I haz the power, 2014-06-03 17:49:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/leviw@chromium.org/271803002/80001

[commit-bot: I haz the power, 2014-06-03 19:19:14]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6824)

[commit-bot: I haz the power, 2014-06-03 19:28:37]

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

[commit-bot: I haz the power, 2014-06-03 19:28:37]

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

[leviw_travelin_and_unemployed, 2014-06-03 19:35:56]

The CQ bit was checked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2014-06-03 19:36:12]

Gotta re-run because of https://codereview.chromium.org/293063016

[commit-bot: I haz the power, 2014-06-03 19:36:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/leviw@chromium.org/271803002/100001

[commit-bot: I haz the power, 2014-06-03 21:04:32]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10126)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10440)

[commit-bot: I haz the power, 2014-06-03 21:19:23]

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

[commit-bot: I haz the power, 2014-06-03 21:19:24]

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

[leviw_travelin_and_unemployed, 2014-06-03 22:10:34]

The CQ bit was checked by leviw@chromium.org

[commit-bot: I haz the power, 2014-06-03 22:11:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/leviw@chromium.org/271803002/120001

[commit-bot: I haz the power, 2014-06-04 00:03:13]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10459)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-06-04 00:04:03]

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

[commit-bot: I haz the power, 2014-06-04 00:04:04]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[leviw_travelin_and_unemployed, 2014-06-04 00:40:21]

The CQ bit was checked by leviw@chromium.org

[commit-bot: I haz the power, 2014-06-04 00:40:42]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/leviw@chromium.org/271803002/120001

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

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-06-04 01:07:05]

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

[commit-bot: I haz the power, 2014-06-04 01:07:06]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[leviw_travelin_and_unemployed, 2014-06-04 01:11:34]

The CQ bit was checked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2014-06-04 01:12:13]

On 2014/06/04 01:07:06, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   win_gpu_triggered_tests on tryserver.chromium.gpu
>
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

win_gpu_triggered_tests passed on the last patchset...  linux_blink_rel *AGAIN*
failed with "Aborting due to failed build state"

[commit-bot: I haz the power, 2014-06-04 01:12:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/leviw@chromium.org/271803002/120001

[commit-bot: I haz the power, 2014-06-04 01:59:06]

Change committed as 175423