Move selection invalidations to the invalidation phase

Move selection invalidations to the invalidation phase

This change is a very simple translation of the current code:
instead of generating invalidations on the fly, we just mark
renderers for selection invalidations later.

I am not super satisfied with this change but it removes
several stale compositing queries and puts us on the path
to integrating selection invalidations more tightly with
the paint invalidation path.

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

[Julien - ping for review, 2014-10-01 16:54:06]

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

[Julien - ping for review, 2014-10-01 16:54:06]

Selection -> RACUN, take I!

[Xianzhu, 2014-10-01 17:04:06]

lgtm

https://codereview.chromium.org/620553009/diff/20001/LayoutTests/TestExpectat...
File LayoutTests/TestExpectations (right):

https://codereview.chromium.org/620553009/diff/20001/LayoutTests/TestExpectat...
LayoutTests/TestExpectations:354: Bug(jchaffraix)
fast/repaint/repaint-across-writing-mode-boundary.html [ NeedsRebaseline ]
Why don't you use crbug.com/407416?

[dsinclair, 2014-10-01 17:13:32]

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

https://codereview.chromium.org/620553009/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderObject.cpp:1281: void
RenderObject::invalidateSelectionIfNeeded(const RenderLayerModelObject&
paintInvalidationContainer) const
Should this be non-const? Its issuing invalidations so, it has side effects, but
they don't affect the RenderObject?

https://codereview.chromium.org/620553009/diff/20001/Source/core/rendering/Re...
File Source/core/rendering/RenderObject.h (right):

https://codereview.chromium.org/620553009/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderObject.h:1255: // 32 bits have been used in the
first word, and 14 in the second.
Does the 14 need to be updated to 15? (Should we just remove it as I'm guessing
its gotten out of date by now)

[Julien - ping for review, 2014-10-01 18:18:22]

Thanks for the prompt reviews!

https://codereview.chromium.org/620553009/diff/20001/LayoutTests/TestExpectat...
File LayoutTests/TestExpectations (right):

https://codereview.chromium.org/620553009/diff/20001/LayoutTests/TestExpectat...
LayoutTests/TestExpectations:354: Bug(jchaffraix)
fast/repaint/repaint-across-writing-mode-boundary.html [ NeedsRebaseline ]
On 2014/10/01 17:04:05, Xianzhu wrote:
> Why don't you use crbug.com/407416?

Forgot that we had a bug number. Changed.

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

https://codereview.chromium.org/620553009/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderObject.cpp:1281: void
RenderObject::invalidateSelectionIfNeeded(const RenderLayerModelObject&
paintInvalidationContainer) const
On 2014/10/01 17:13:32, dsinclair wrote:
> Should this be non-const? Its issuing invalidations so, it has side effects,
but
> they don't affect the RenderObject?

Done.

https://codereview.chromium.org/620553009/diff/20001/Source/core/rendering/Re...
File Source/core/rendering/RenderObject.h (right):

https://codereview.chromium.org/620553009/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderObject.h:1255: // 32 bits have been used in the
first word, and 14 in the second.
On 2014/10/01 17:13:32, dsinclair wrote:
> Does the 14 need to be updated to 15? (Should we just remove it as I'm
guessing
> its gotten out of date by now)

This is actually up-to-date and I have updated it to make sure we stay this way.
That's why reviews are super important sir!

[dsinclair, 2014-10-01 18:22:04]

lgtm

[dsinclair, 2014-10-01 18:22:17]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2014-10-01 19:41:26]

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

[commit-bot: I haz the power, 2014-10-01 19:56:53]

Committed patchset #3 (id:40001) as 183082

[yoichio, 2014-10-02 01:49:01]

yoichio@chromium.org changed reviewers:
+ yoichio@chromium.org

[yoichio, 2014-10-02 01:49:03]

https://codereview.chromium.org/620553009/diff/40001/Source/core/rendering/Re...
File Source/core/rendering/RenderText.h (right):

https://codereview.chromium.org/620553009/diff/40001/Source/core/rendering/Re...
Source/core/rendering/RenderText.h:186: // The parent invalidates for
RenderText, so RenderText does nothing.
Should we remove this comment?

[yosin_UTC9, 2014-10-02 02:18:28]

Thanks for re-structuring!

BTW, I would like to introduce |RenderSelection| class to move selection
rendering from |RenderView|. 

WDYTH?

https://codereview.chromium.org/620553009/diff/40001/Source/core/rendering/Re...
File Source/core/rendering/RenderSelectionInfo.h (right):

https://codereview.chromium.org/620553009/diff/40001/Source/core/rendering/Re...
Source/core/rendering/RenderSelectionInfo.h:49: RenderObject* object() const {
return m_object.get(); }
nit: I prefer introducing |SelectionInfo::setShouldSelection()| rather than
|object()| to encapsulate selection related operation info |SelectionInfo|
class.

[Julien - ping for review, 2014-10-15 14:51:25]

On 2014/10/02 at 02:18:28, yosin wrote:
> Thanks for re-structuring!
> 
> BTW, I would like to introduce |RenderSelection| class to move selection
rendering from |RenderView|. 
> 
> WDYTH?

Sorry for the delay, I completely missed the question (thank Dan as he pointed
this out :)).

That's a great idea. I would love for the selection code to self-contained.