Where an ancestor has overflow style, a fixed child needs to be composited.

Where an ancestor has overflow style, a fixed child needs to be composited.

A block child with fixed style is composited where frameview has an enough long height to scroll.
But that does not cover the other case. Where an ancestor of the fixed child has a overflow style
but frameview is not scrollable, we don't composite the fixed block. For the result, the fixed child
is scrolled according to its ancestor block.
We need to composite it in a separate composited layer for the case.

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

[changseok, 2015-06-05 18:40:28]

shivamidow@gmail.com changed reviewers:
+ eae@chromium.org, leviw@chromium.org, mstensho@opera.com, skobes@chromium.org

[changseok, 2015-06-05 18:41:28]

Would you have a look at this?

[changseok, 2015-06-08 05:29:20]

shivamidow@gmail.com changed reviewers:
+ chrishtr@chromium.org

[changseok, 2015-06-08 05:29:21]

+chrishtr

[leviw_travelin_and_unemployed, 2015-06-08 19:35:40]

"layered" and "layered out" aren't sufficiently descriptive terms, as the system
has many layers. You specifically should refer to being composited or having a
composited layer (in your description).

[chrishtr, 2015-06-08 23:04:31]

chrishtr@chromium.org changed reviewers:
+ vollick@chromium.org

[chrishtr, 2015-06-08 23:05:56]

chrishtr@chromium.org changed reviewers:
+ vollick@chromium.org i
- vollick@chromium.org

[chrishtr, 2015-06-08 23:05:57]

I can't repro on Chrome 45.0.2421.0. Is it still a real bug?

[changseok, 2015-06-09 03:58:48]

On 2015/06/08 23:05:57, chrishtr wrote:
> I can't repro on Chrome 45.0.2421.0. Is it still a real bug?

Yes it is. I just checked the bug still happens on my build of ToT (Version
45.0.2427.0 (64-bit))

[changseok, 2015-06-09 04:03:30]

On 2015/06/09 03:58:48, changseok wrote:
> On 2015/06/08 23:05:57, chrishtr wrote:
> > I can't repro on Chrome 45.0.2421.0. Is it still a real bug?
> 
> Yes it is. I just checked the bug still happens on my build of ToT (Version
> 45.0.2427.0 (64-bit))

One more.. My environment is a Mac installed Yosemite.

[changseok, 2015-06-09 04:04:13]

On 2015/06/08 19:35:40, leviw wrote:
> "layered" and "layered out" aren't sufficiently descriptive terms, as the
system
> has many layers. You specifically should refer to being composited or having a
> composited layer (in your description).

Thank you for your comment. Updated.

[skobes, 2015-06-09 16:20:54]

I haven't reviewed in depth, but:

"A block child with fixed style is composited where frameview has an enough long
height to scroll."

^ This is not true, in general.  Frame-level scrolling will issue paint
invalidations for non-composited fixed-position elements (in
FrameView::invalidateViewportConstrainedObjects).

[skobes, 2015-06-09 17:05:59]

I think I understand the bug now.  But I am curious why the fixed-position
element is not promoted by overlap testing.

[changseok, 2015-06-11 08:05:17]

On 2015/06/09 17:05:59, skobes wrote:
> I think I understand the bug now.  But I am curious why the fixed-position
> element is not promoted by overlap testing.

Because the parent's bounding rect of the fixed child is not added to overlapMap
yet, so the overlapped test fails.
the overlapMap doesn't have the parent bounding rect when the child tests it is
overlapped.
Rough outline is like below..
UpdateRecursive() for parent -> Call overlapMap.beginNewOverlapTestingContext()
but not add its bounding rect to overlapMap -> updateRecursive() for child ->
overlapTest but no saved boundingBox in overlapMap (the test fails!) -> add
child's bounding rect to overlapMap -> add parent's bounding rect to overlapMap
-> ...

[skobes, 2015-06-11 16:30:43]

On 2015/06/11 08:05:17, changseok wrote:
> On 2015/06/09 17:05:59, skobes wrote:
> > I think I understand the bug now.  But I am curious why the fixed-position
> > element is not promoted by overlap testing.
> 
> Because the parent's bounding rect of the fixed child is not added to
overlapMap
> yet, so the overlapped test fails.
> the overlapMap doesn't have the parent bounding rect when the child tests it
is
> overlapped.
> Rough outline is like below..
> UpdateRecursive() for parent -> Call
overlapMap.beginNewOverlapTestingContext()
> but not add its bounding rect to overlapMap -> updateRecursive() for child ->
> overlapTest but no saved boundingBox in overlapMap (the test fails!) -> add
> child's bounding rect to overlapMap -> add parent's bounding rect to
overlapMap
> -> ...

With your change, do we still correctly handle the case where the scroller has a
transform on it?  E.g. http://jsfiddle.net/33gdnxcb/1/ - here the yellow box
should scroll with the content (see http://crbug.com/20574#c27,
https://www.w3.org/Bugs/Public/show_bug.cgi?id=16328).

[changseok, 2015-06-12 04:55:37]

On 2015/06/11 16:30:43, skobes wrote:
> On 2015/06/11 08:05:17, changseok wrote:
> > On 2015/06/09 17:05:59, skobes wrote:
> > > I think I understand the bug now.  But I am curious why the fixed-position
> > > element is not promoted by overlap testing.
> > 
> > Because the parent's bounding rect of the fixed child is not added to
> overlapMap
> > yet, so the overlapped test fails.
> > the overlapMap doesn't have the parent bounding rect when the child tests it
> is
> > overlapped.
> > Rough outline is like below..
> > UpdateRecursive() for parent -> Call
> overlapMap.beginNewOverlapTestingContext()
> > but not add its bounding rect to overlapMap -> updateRecursive() for child
->
> > overlapTest but no saved boundingBox in overlapMap (the test fails!) -> add
> > child's bounding rect to overlapMap -> add parent's bounding rect to
> overlapMap
> > -> ...
> 
> With your change, do we still correctly handle the case where the scroller has
a
> transform on it?  E.g. http://jsfiddle.net/33gdnxcb/1/ - here the yellow box
> should scroll with the content (see http://crbug.com/20574#c27,
> https://www.w3.org/Bugs/Public/show_bug.cgi?id=16328).
Yes it does, the yellow box is scrolled with my cl in your test.

[skobes, 2015-06-12 05:18:20]

lgtm

[changseok, 2015-06-12 05:46:16]

The CQ bit was checked by shivamidow@gmail.com

[commit-bot: I haz the power, 2015-06-12 05:46:26]

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

[commit-bot: I haz the power, 2015-06-12 07:24:57]

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

[commit-bot: I haz the power, 2015-06-12 07:24:58]

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

[changseok, 2015-06-12 07:54:50]

On 2015/06/12 07:24:58, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_blink_rel on tryserver.blink (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/6...)

https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...
Weird. the expected result is different between mac and linux. The linux's
result has an extra line. 
> "drawsContent": true
Any idea on where it comes from?

[skobes, 2015-06-12 15:00:38]

On 2015/06/12 07:54:50, changseok wrote:
> On 2015/06/12 07:24:58, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   linux_blink_rel on tryserver.blink (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/6...)
> 
>
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...
> Weird. the expected result is different between mac and linux. The linux's
> result has an extra line. 
> > "drawsContent": true
> Any idea on where it comes from?

I think this is because on Aura we paint the scrollbar ourselves, but on Mac we
let the OS do it.  Many of the repaint tests have platform-specific expectation
files that reflect this difference.

But I wonder if you really need layerTreeAsText here.  Can't you just test the
position of the yellow box with getBoundingClientRect?

[changseok, 2015-06-12 20:08:22]

On 2015/06/12 15:00:38, skobes wrote: 
> I think this is because on Aura we paint the scrollbar ourselves, but on Mac
we
> let the OS do it.  Many of the repaint tests have platform-specific
expectation
> files that reflect this difference.
Good to know. =)

> But I wonder if you really need layerTreeAsText here.  Can't you just test the
> position of the yellow box with getBoundingClientRect?
I gave a shot on the getBoundingClientRect way and a reference test way for a
while though,
both didn't reflect the reality of layout.
getBoundintClientRect().top of the fixed element alway returned a layout top
value, not a visible real location
so that I could not differentiate results of before-and-after with the api.
The reference test was also same. I tried to scroll the overflow region through
div.scrollTop,
but both actual and expected results were always same regardless of my cl.
I guess scrolling through eventSender maybe work though... I don't prefer that
way. 

Let me update the existing test result to make linux and window happy and add a
specific result for mac.
If you have a better idea, let me know.

[changseok, 2015-06-15 04:01:32]

On 2015/06/12 20:08:22, changseok wrote:
> Let me update the existing test result to make linux and window happy and add
a
> specific result for mac.

Well. it seems no objection on this so that I will land this.

[changseok, 2015-06-15 04:02:07]

The CQ bit was checked by shivamidow@gmail.com

[changseok, 2015-06-15 04:02:07]

The patchset sent to the CQ was uploaded after l-g-t-m from skobes@chromium.org
Link to the patchset: https://codereview.chromium.org/1167073002/#ps20001
(title: "Update expected results.")

[commit-bot: I haz the power, 2015-06-15 04:02:20]

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

[commit-bot: I haz the power, 2015-06-15 05:19:58]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197099