Reconstruct customscrollbars if owningLayoutObject changes.

Reconstruct customscrollbars if owningLayoutObject changes.

CustomScrollbars owner changes when the body's LayoutObject is attached
which occurs during style recalc (before layout). However in the case
of overflow:scroll, the scrollbars are created when we call
styleDidChange() on the LayoutView, which happens even sooner than
that. So during updateAfterLayout,in needsScrollbarReconstruction if
owningLayoutObject changes for customscrollbar reconstruct the
scrollbar.

BUG=522389
TESTED=scrollbars/custom-scrollbar-inactive-only-on-windowinactive-selector-expected.html
now passes with root layer scrolling

Committed: https://crrev.com/6c9abc243561998590a5cfec02d62b86578e4ec0
Cr-Commit-Position: refs/heads/master@{#352644}

[MuVen, 2015-09-07 06:33:37]

sataya.m@samsung.com changed reviewers:
+ skobes@chromium.org

[MuVen, 2015-09-07 09:28:21]

Hi Skobes,

PTAL.

Thanks,

[skobes, 2015-09-12 02:36:52]

https://codereview.chromium.org/1307173007/diff/1/Source/core/paint/Deprecate...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1307173007/diff/1/Source/core/paint/Deprecate...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:969: return
&layoutObject;
Based on the comment on line 966, it looks like the intent of this code is to
use the styles applied to the <iframe> element to draw the scrollbars inside the
iframe.  Won't this break that logic?

[MuVen, 2015-10-01 14:59:55]

On 2015/09/12 02:36:52, skobes wrote:
>
https://codereview.chromium.org/1307173007/diff/1/Source/core/paint/Deprecate...
> File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):
> 
>
https://codereview.chromium.org/1307173007/diff/1/Source/core/paint/Deprecate...
> Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:969: return
> &layoutObject;
> Based on the comment on line 966, it looks like the intent of this code is to
> use the styles applied to the <iframe> element to draw the scrollbars inside
the
> iframe.  Won't this break that logic?

Skobes, found the exact root-cause of 
scrollbars/custom-scrollbar-inactive-only-on-windowinactive-selector-expected.html
failure.

During updateStyle customScrollbar is created, but while creating 
Element* body's layoutObject is not set due to which 
frameLayoutObject is returned and customscrollbar created with 
frameLayoutObject as onwer node. 

But Element* body layoutObject is set which is can be observed after
updateAfterLayout. During updateAfterLayout, updateScrollDimensions 
is called and in that we check for needsScrollbarReconstruction.
In that we need to reConstruct the customs scrollbar if m_owner is changed. 

During UpdateStyleChange : frameLayoutObject  is returned by
layoutObjectForScrollbar. (body/document doesnt has layoutObject so it checks
for frameLayoutObject)
During updateScrollDimensions : body->layoutObject(); is returned by
layoutObjectForScrollbar.

[skobes, 2015-10-06 01:51:21]

On 2015/10/01 14:59:55, MuVen wrote:
> During updateStyle customScrollbar is created, but while creating 
> Element* body's layoutObject is not set due to which 
> frameLayoutObject is returned and customscrollbar created with 
> frameLayoutObject as onwer node. 
> 
> But Element* body layoutObject is set which is can be observed after
> updateAfterLayout. During updateAfterLayout, updateScrollDimensions 
> is called and in that we check for needsScrollbarReconstruction.
> In that we need to reConstruct the customs scrollbar if m_owner is changed. 

Yes, it looks like we need to handle the case where a scrollbar's m_owner needs
to change.  Perhaps we can check for this in
PaintLayerScrollableArea::needsScrollbarReconstruction?  You can get the
scrollbar's current owner with LayoutScrollbar::owningLayoutObject().

[MuVen, 2015-10-06 12:34:18]

PTAL.

[skobes, 2015-10-06 17:06:21]

lgtm

Slight correction on the change description - the owner changes when the body's
LayoutObject is attached, which occurs during style recalc (before layout). 
However in the case of overflow:scroll, the scrollbars are created when we call
styleDidChange() on the LayoutView, which happens even sooner than that.

[MuVen, 2015-10-06 18:01:30]

The CQ bit was checked by sataya.m@samsung.com

[commit-bot: I haz the power, 2015-10-06 18:02:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1307173007/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1307173007/60001

[commit-bot: I haz the power, 2015-10-06 18:10:02]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-06 18:12:05]

Patchset 4 (id:??) landed as
https://crrev.com/6c9abc243561998590a5cfec02d62b86578e4ec0
Cr-Commit-Position: refs/heads/master@{#352644}