Fix clippingContainer for fixed position elements

Fix clippingContainer for fixed position elements.

Fixed position elements are clipped by CSS clip ancestors,
which may be between the fixed position element and its
containingBlock. So clippingAncestor needs to walk up the
ancestors of a fixed positioned element manually instead
of using containingBlock directly.

I believe this only applies to fixed position elements.
Other elements can use containingBlock because an absolutely
positioned ancestor will never get skipped in the
containingBlock traversal and CSS clip only applies to
absolutely positioned elements.

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

[ojan, 2014-02-26 20:55:27]

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

https://codereview.chromium.org/179833006/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderObject.cpp:820: RenderObject* container = this;
Had to remove the const for this line. I think I could do some shenanigans with
having a const renderer local variable and a non-const container local variable
to keep the const, but that seemed not worth it.

[esprehn, 2014-02-26 21:26:02]

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

https://codereview.chromium.org/179833006/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderObject.cpp:823: for (container =
container->parent(); container && !container->canContainFixedPositionObjects();
container = container->parent()) {
Is this right? Now if you run into something that claims it can contain fixed
position objects we'll use the hasClipOrOverflowClip() check below.

If I do <div style="overflow: hidden; transform: translateZ(0)">
  <div style="position: fixed;"></div>
</div>

will the fixed thing really be clipped by the transformed overflow thing?

[ojan, 2014-02-27 00:28:46]

On 2014/02/26 21:26:02, esprehn wrote:
>
https://codereview.chromium.org/179833006/diff/1/Source/core/rendering/Render...
> File Source/core/rendering/RenderObject.cpp (right):
> 
>
https://codereview.chromium.org/179833006/diff/1/Source/core/rendering/Render...
> Source/core/rendering/RenderObject.cpp:823: for (container =
> container->parent(); container &&
!container->canContainFixedPositionObjects();
> container = container->parent()) {
> Is this right? Now if you run into something that claims it can contain fixed
> position objects we'll use the hasClipOrOverflowClip() check below.
> 
> If I do <div style="overflow: hidden; transform: translateZ(0)">
>   <div style="position: fixed;"></div>
> </div>
> 
> will the fixed thing really be clipped by the transformed overflow thing?

Yes. I'll add a test case to this patch though.

http://plexode.com/eval3/#s=aekVQXANJVQMbAx1yAXgePQN8GwFJSkVFRk8cAQ5YRkNMSlUO...

[ojan, 2014-02-27 01:53:45]

Test case added.

[esprehn, 2014-02-27 03:03:45]

On 2014/02/27 01:53:45, ojan wrote:
> Test case added.

Why did you lose the const?

[ojan, 2014-02-27 03:06:19]

On 2014/02/27 03:03:45, esprehn wrote:
> On 2014/02/27 01:53:45, ojan wrote:
> > Test case added.
> 
> Why did you lose the const?

"Had to remove the const for this line. I think I could do some shenanigans with
having a const renderer local variable and a non-const container local variable
to keep the const, but that seemed not worth it."

[ojan, 2014-02-28 03:04:46]

On 2014/02/27 03:06:19, ojan wrote:
> On 2014/02/27 03:03:45, esprehn wrote:
> > On 2014/02/27 01:53:45, ojan wrote:
> > > Test case added.
> > 
> > Why did you lose the const?
> 
> "Had to remove the const for this line. I think I could do some shenanigans
with
> having a const renderer local variable and a non-const container local
variable
> to keep the const, but that seemed not worth it."

Sorry, that lacks context...it's from my original comment on this review:
"RenderObject* container = this;".

[ojan, 2014-03-04 02:36:07]

ping

[abarth-chromium, 2014-03-04 05:04:48]

@esprehn: I think this one is for you.  :)

[esprehn, 2014-03-04 05:22:24]

lgtm, one comment

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

https://codereview.chromium.org/179833006/diff/10001/Source/core/rendering/Re...
Source/core/rendering/RenderObject.cpp:820: RenderObject* container = this;
I think I'd prefer you just const_cast on |this| to make it unconst before
returning. This function can never actually return |this| anyway.

[ojan, 2014-03-04 19:25:00]

The CQ bit was checked by ojan@chromium.org

[commit-bot: I haz the power, 2014-03-04 19:26:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ojan@chromium.org/179833006/30001

[commit-bot: I haz the power, 2014-03-05 12:31:17]

Change committed as 168473