Remove 2 calls to paintInvalidationForWholeRenderer

Remove 2 calls to paintInvalidationForWholeRenderer

These calls were needed as we can make the body or the
document element's background bleed into the canvas (ie
replacing the FrameView's background). However the logic
was working around the fact that
clippedOverflowRectForPaintInvalidation didn't account for
the previous cases.

The whole logic works as the 2 renderers where this happens
share the same coordinate space as the viewport. The tests
are all invalidating as much, we just got rid of some stray
invalidations.

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

[dsinclair, 2014-07-21 19:32:29]

lgtm

https://codereview.chromium.org/396073002/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderBox.cpp (right):

https://codereview.chromium.org/396073002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderBox.cpp:1960: return view()->viewRect();
Should we add an ASSERT here just to be safe that we're body or html? Not sure
if it would happen, but we could potentially expand the meaning of
backgroundCanBleedToCanvas in the future.

[Julien - ping for review, 2014-07-21 20:33:34]

https://codereview.chromium.org/396073002/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderBox.cpp (right):

https://codereview.chromium.org/396073002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderBox.cpp:1960: return view()->viewRect();
On 2014/07/21 19:32:28, dsinclair wrote:
> Should we add an ASSERT here just to be safe that we're body or html? Not sure
> if it would happen, but we could potentially expand the meaning of
> backgroundCanBleedToCanvas in the future.

I don't think that would give us much as that's the definition of
backgroundCanBleedToCanvas and I don't think this definition will evolve (this
is an old HTML concept). If we did evolve it, I would expect hackers / reviewers
to check this comment.

[Julien - ping for review, 2014-07-21 20:52:49]

The CQ bit was checked by jchaffraix@chromium.org

[commit-bot: I haz the power, 2014-07-21 20:53:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jchaffraix@chromium.org/396073002/1

[commit-bot: I haz the power, 2014-07-21 21:06:10]

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/1...)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/18063)

[commit-bot: I haz the power, 2014-07-21 21:08:42]

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

[commit-bot: I haz the power, 2014-07-21 21:08:43]

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

[Julien - ping for review, 2014-07-21 22:14:59]

The CQ bit was checked by jchaffraix@chromium.org

[commit-bot: I haz the power, 2014-07-21 22:15:40]

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

[commit-bot: I haz the power, 2014-07-21 23:21:10]

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

[commit-bot: I haz the power, 2014-07-22 00:01:18]

Change committed as 178605

[esprehn, 2014-07-22 02:08:21]

Are you sure they share the same coordinate space? If you transform the body or
the html element ex. rotate them or translate(x,y) them then they won't be in
the same coordinate space as the viewport.

[esprehn, 2014-07-22 02:10:45]

On 2014/07/22 at 02:08:21, esprehn wrote:
> Are you sure they share the same coordinate space? If you transform the body
or the html element ex. rotate them or translate(x,y) them then they won't be in
the same coordinate space as the viewport.

More specifically, I'm pretty sure this patch is wrong. You're going to return
the viewRect() and not correctly invalidate the body's layer. The body and html
can also be totally different sizes from the viewport so returning the
viewRect() from clippedOverflowRectForPaintInvalidation is not correct either.

[Julien - ping for review, 2014-07-24 17:45:38]

On 2014/07/22 at 02:10:45, esprehn wrote:
> On 2014/07/22 at 02:08:21, esprehn wrote:
> > Are you sure they share the same coordinate space? If you transform the body
or the html element ex. rotate them or translate(x,y) them then they won't be in
the same coordinate space as the viewport.
> 
> More specifically, I'm pretty sure this patch is wrong. You're going to return
the viewRect() and not correctly invalidate the body's layer. The body and html
can also be totally different sizes from the viewport so returning the
viewRect() from clippedOverflowRectForPaintInvalidation is not correct either.

Elliott is right, I don't know what I was thinking on this one. This will yield
to over-invalidations in most cases and potentially invalidating the wrong
position. I will revert.

[Julien - ping for review, 2014-07-24 17:46:51]

A revert of this CL has been created in
https://codereview.chromium.org/420483002/ by jchaffraix@chromium.org.

The reason for reverting is: This change is plain wrong as <body> and <html> can
have different coordinate spaces (e.g. if either has a transform).

Thanks to Elliott to pointing this out..