Create custom scrollbars when html element has overflow:scroll

Create custom scrollbars when html element has overflow:scroll

custom scrollbars are not created when html element has overflow:scroll
style. Reconstruct scrollbars from native to custom scrollbar when 
html element has overflow:scroll style. 

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

[MuVen, 2015-08-13 12:52:41]

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

[MuVen, 2015-08-13 13:30:27]

Patchset #1 (id:1) has been deleted

[MuVen, 2015-08-13 13:32:22]

Patchset #1 (id:20001) has been deleted

[MuVen, 2015-08-13 13:46:46]

Patchset #2 (id:60001) has been deleted

[MuVen, 2015-08-13 13:48:00]

Patchset #1 (id:40001) has been deleted

[MuVen, 2015-08-13 13:49:04]

Skobes, PTAL.

[skobes, 2015-08-13 18:12:40]

https://codereview.chromium.org/1292513002/diff/80001/Source/core/paint/Depre...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/80001/Source/core/paint/Depre...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:974: return
actualLayoutObject->isBox() &&
actualLayoutObject->style()->hasPseudoStyle(SCROLLBAR);
This will reconstruct custom scrollbars after every layout.  We only want to
reconstruct if the scrollbar is currently native, and should be custom, or if it
is currently custom, and should be native.

[MuVen, 2015-08-13 19:36:32]

https://codereview.chromium.org/1292513002/diff/80001/Source/core/paint/Depre...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/80001/Source/core/paint/Depre...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:651:
destroyScrollbar(HorizontalScrollbar);
!m_hBar->isCustomScrollbar() This condition wont trigger reconstruction
every-time after layout. May be i see i need to pull all these conditions in the
needsScrollbarReconstruction logic. wdyt about the api calls in the if loop. is
that fine to call destroyScrollbar and call setHasHorizontalScrollbar(true) ? or

setHasHorizontalScrollbar(false);
setHasHorizontalScrollbar(true);

please share your inputs.

[skobes, 2015-08-13 19:53:43]

https://codereview.chromium.org/1292513002/diff/80001/Source/core/paint/Depre...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/80001/Source/core/paint/Depre...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:651:
destroyScrollbar(HorizontalScrollbar);
On 2015/08/13 19:36:32, MuVen wrote:
> !m_hBar->isCustomScrollbar() This condition wont trigger reconstruction
> every-time after layout. May be i see i need to pull all these conditions in
the
> needsScrollbarReconstruction logic.

Yes, to make it easier to follow, needsScrollbarReconstruction should return
true if and only if one or both scrollbars need to be reconstructed.

> wdyt about the api calls in the if loop. is
> that fine to call destroyScrollbar and call setHasHorizontalScrollbar(true) ?
or
> 
> setHasHorizontalScrollbar(false);
> setHasHorizontalScrollbar(true);
> 
> please share your inputs.

I think if any scrollbar needs reconstruction it should be sufficient to destroy
them both, then let the regular overflow logic create them again below.

[MuVen, 2015-08-14 11:45:34]

Patchset #3 (id:120001) has been deleted

[MuVen, 2015-08-14 18:59:42]

Hi SKobes,

PTAL.

Thanks,
MuVen.

[skobes, 2015-08-14 19:02:56]

https://codereview.chromium.org/1292513002/diff/140001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/140001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:654:
needsHBarConstruction = true;
I don't understand the need to store this.  Why doesn't the existing
overflow:auto logic create the scrollbar?

[MuVen, 2015-08-14 19:03:38]

On 2015/08/14 18:59:42, MuVen wrote:
> Hi SKobes,
> 
> PTAL.
> 
> Thanks,
> MuVen.

just for context:
Regular overflow logic below creates the scrollbar only in the case of auto
attribute.
I have modified and added reconstruction bool variable and linked it to overflow
auto
logic. PTAL.(sorry for spamming).

[skobes, 2015-08-14 19:05:14]

On 2015/08/14 19:03:38, MuVen wrote:
> Regular overflow logic below creates the scrollbar only in the case of auto
> attribute.

Isn't that what we want?

[MuVen, 2015-08-14 19:17:48]

On 2015/08/14 19:05:14, skobes wrote:
> On 2015/08/14 19:03:38, MuVen wrote:
> > Regular overflow logic below creates the scrollbar only in the case of auto
> > attribute.
> 
> Isn't that what we want?

yes we can reuse it in the case of overflow:scroll and
needsScrollbarReconstruction.

[skobes, 2015-08-14 19:23:37]

https://codereview.chromium.org/1292513002/diff/140001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/140001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:697: bool
horizontalScrollBarChanged = (box().hasAutoHorizontalScrollbar() &&
(hasHorizontalScrollbar() != hasHorizontalOverflow)) || needsHBarConstruction;
Oh I see, the problem is with overflow:scroll and not overflow:auto.

I think you can still avoid the needsHBarConstruction and needsVBarConstruction
vars.  Can we use

bool horizontalScrollBarChanged = (box().hasAutoHorizontalScrollbar() &&
(hasHorizontalScrollbar() != hasHorizontalOverflow))
    || (box().style()->overflowX() == OSCROLL && !horizontalScrollbar());
...?

[MuVen, 2015-08-17 07:00:32]

Patchset #5 (id:180001) has been deleted

[MuVen, 2015-08-17 07:00:55]

Patchset #4 (id:160001) has been deleted

[MuVen, 2015-08-17 07:01:02]

Patchset #4 (id:200001) has been deleted

[MuVen, 2015-08-17 09:15:46]

Patchset #4 (id:220001) has been deleted

[MuVen, 2015-08-17 09:16:05]

Patchset #4 (id:240001) has been deleted

[MuVen, 2015-08-17 09:21:03]

Done !!! PTAL.

https://codereview.chromium.org/1292513002/diff/140001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/140001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:697: bool
horizontalScrollBarChanged = (box().hasAutoHorizontalScrollbar() &&
(hasHorizontalScrollbar() != hasHorizontalOverflow)) || needsHBarConstruction;
On 2015/08/14 19:23:37, skobes wrote:
> Oh I see, the problem is with overflow:scroll and not overflow:auto.
> 
> I think you can still avoid the needsHBarConstruction and
needsVBarConstruction
> vars.  Can we use
> 
> bool horizontalScrollBarChanged = (box().hasAutoHorizontalScrollbar() &&
> (hasHorizontalScrollbar() != hasHorizontalOverflow))
>     || (box().style()->overflowX() == OSCROLL && !horizontalScrollbar());
> ...?

Done.

[skobes, 2015-08-17 22:59:02]

https://codereview.chromium.org/1292513002/diff/260001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/260001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:695:
setHasHorizontalScrollbar(hasHorizontalOverflow);
This seems wrong for OSCROLL, which should show the scrollbar regardless of
whether there is overflow.

Perhaps you want:

if (box().style()->overflowX() == OSCROLL)
    setHasHorizontalScrollbar(true);

https://codereview.chromium.org/1292513002/diff/260001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:708: if
(box().style()->overflowX() == OAUTO || box().style()->overflowY() == OAUTO ||
box().style()->overflowX() == OSCROLL || box().style()->overflowY() == OSCROLL)
{
Is this if statement even needed?  We are already guaranteed that
(horizontalScrollBarChanged || verticalScrollBarChanged) by the time we get
here.

[MuVen, 2015-08-18 13:20:02]

https://codereview.chromium.org/1292513002/diff/280001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/280001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:694: if
(box().hasAutoHorizontalScrollbar() || box().style()->overflowX() == OSCROLL)
in case of overflow-x:scroll and overflow-y:auto, for ex
LayoutTests/scrollbars/disabled-scrollbar.html
overflow logic during styling has calculated horizontal scrollbar, but as not
overflow of contents, thumb
is not shown, just painted the track. 

Think that verticalScrollbarChanged, 
horizontalScrollBarChanged = 0 verticalScrollBarChanged = 1

but the below condition will be executed, as overflow-x:scroll and doesnt has
horizontalOverflow. This disables
the scrollbar. I think its better we add && !horizontalScrollbar() check.
           
		   if (box().hasAutoHorizontalScrollbar() ||box().style()->overflowX() ==
OSCROLL)  {
            setHasHorizontalScrollbar(hasHorizontalOverflow);

[MuVen, 2015-08-18 14:38:53]

Ptal. I have analyzed 2 of your review comments.
Please share your thoughts.

https://codereview.chromium.org/1292513002/diff/260001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/260001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:708: if
(box().style()->overflowX() == OAUTO || box().style()->overflowY() == OAUTO ||
box().style()->overflowX() == OSCROLL || box().style()->overflowY() == OSCROLL)
{
On 2015/08/17 22:59:01, skobes wrote:
> Is this if statement even needed?  We are already guaranteed that
> (horizontalScrollBarChanged || verticalScrollBarChanged) by the time we get
> here.

We may need this statement. Technically if we see, not required. But testcases
like fast/repaint/layout-state-only-positioned.html needs this statement.

[skobes, 2015-08-18 15:13:40]

On 2015/08/18 14:38:53, MuVen wrote:
> We may need this statement. Technically if we see, not required. But testcases
> like fast/repaint/layout-state-only-positioned.html needs this statement.

Why?

[MuVen, 2015-08-18 17:11:09]

On 2015/08/18 15:13:40, skobes wrote:
> On 2015/08/18 14:38:53, MuVen wrote:
> > We may need this statement. Technically if we see, not required. But
testcases
> > like fast/repaint/layout-state-only-positioned.html needs this statement.
> 
> Why?

Here is the explaination for the second comment:

In the test case mentioned fast/repaint/layout-state-only-positioned.html
div "q" has an overflow:overlay attribute. due to which after loading
the page, verticaloverflow is been calculated. 
hasAutoVerticalScrollbar:1 hasVerticalScrollbar:0	hasVerticalOverflow:1
because of this values verticalScrollBarChanged is calculated to be true.

verticalScrollBarChanged is true which enters the if loop. As i see 
relayout of the scrollbars is expected only in the case of AUTO, as
we have removed that check this is triggering to relayout the scrollbars
failing the testcase which is not expected for the overflow:overlay.

[skobes, 2015-08-18 18:48:09]

On 2015/08/18 17:11:09, MuVen wrote:
> In the test case mentioned fast/repaint/layout-state-only-positioned.html
> div "q" has an overflow:overlay attribute. due to which after loading
> the page, verticaloverflow is been calculated. 
> hasAutoVerticalScrollbar:1 hasVerticalScrollbar:0	hasVerticalOverflow:1
> because of this values verticalScrollBarChanged is calculated to be true.
> 
> verticalScrollBarChanged is true which enters the if loop. As i see 
> relayout of the scrollbars is expected only in the case of AUTO, as
> we have removed that check this is triggering to relayout the scrollbars
> failing the testcase which is not expected for the overflow:overlay.

Ah, yes overflow:overlay is a strange edge case.  I see there is a comment in
the code about it, but it has drifted away from the relevant location.  Can we
check for it explicitly?  e.g.

    // Re-layout, unless using overflow:overlay which doesn't trigger a layout.
    if ((horizontalScrollBarChanged && box().style()->overflowX() != OOVERLAY)
||
        (verticalScrollBarChanged && box().style()->overflowY() != OOVERLAY)) {
        ...

[MuVen, 2015-08-19 16:52:42]

PTAL.

~Thanks

[skobes, 2015-08-19 16:59:52]

https://codereview.chromium.org/1292513002/diff/340001/LayoutTests/fast/scrol...
File
LayoutTests/fast/scrolling/custom-scrollbar-style-applied-with-overflow-attribute.html
(right):

https://codereview.chromium.org/1292513002/diff/340001/LayoutTests/fast/scrol...
LayoutTests/fast/scrolling/custom-scrollbar-style-applied-with-overflow-attribute.html:29:
function addRule(selector, css){
This addRule function seems unnecessarily complex for this case.  Can you just
call sheet.insertRule directly?

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:691: bool
horizontalScrollBarChanged = (box().hasAutoHorizontalScrollbar() &&
(hasHorizontalScrollbar() != hasHorizontalOverflow)) ||
(box().style()->overflowX() == OSCROLL && !horizontalScrollbar());
Wrap the line at the || for readability.

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:692: bool
verticalScrollBarChanged = (box().hasAutoVerticalScrollbar() &&
(hasVerticalScrollbar() != hasVerticalOverflow)) || (box().style()->overflowY()
== OSCROLL && !verticalScrollbar());
same here

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:694: if
(box().hasAutoHorizontalScrollbar() || (box().style()->overflowX() == OSCROLL &&
!horizontalScrollbar()))
In the OSCROLL case we do not care about hasHorizontalOverflow, right?  So it
should just call setHasHorizontalScrollbar(true).

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:710: // Our proprietary
overflow: overlay value doesn't trigger a layout.
Can you move this comment to appear just above the "if" statement that checks
for OOVERLAY?  That's what it's really referring to.

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.h (right):

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.h:154: bool
needsScrollbarReconstruction() const;
Can this be private?

[MuVen, 2015-08-19 19:20:42]

PTAL.

https://codereview.chromium.org/1292513002/diff/340001/LayoutTests/fast/scrol...
File
LayoutTests/fast/scrolling/custom-scrollbar-style-applied-with-overflow-attribute.html
(right):

https://codereview.chromium.org/1292513002/diff/340001/LayoutTests/fast/scrol...
LayoutTests/fast/scrolling/custom-scrollbar-style-applied-with-overflow-attribute.html:29:
function addRule(selector, css){
On 2015/08/19 16:59:51, skobes wrote:
> This addRule function seems unnecessarily complex for this case.  Can you just
> call sheet.insertRule directly?

Done.

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:691: bool
horizontalScrollBarChanged = (box().hasAutoHorizontalScrollbar() &&
(hasHorizontalScrollbar() != hasHorizontalOverflow)) ||
(box().style()->overflowX() == OSCROLL && !horizontalScrollbar());
On 2015/08/19 16:59:52, skobes wrote:
> Wrap the line at the || for readability.

Done.

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:692: bool
verticalScrollBarChanged = (box().hasAutoVerticalScrollbar() &&
(hasVerticalScrollbar() != hasVerticalOverflow)) || (box().style()->overflowY()
== OSCROLL && !verticalScrollbar());
On 2015/08/19 16:59:52, skobes wrote:
> same here

Done.

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:694: if
(box().hasAutoHorizontalScrollbar() || (box().style()->overflowX() == OSCROLL &&
!horizontalScrollbar()))
On 2015/08/19 16:59:52, skobes wrote:
> In the OSCROLL case we do not care about hasHorizontalOverflow, right?  So it
> should just call setHasHorizontalScrollbar(true).

Done.

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:710: // Our proprietary
overflow: overlay value doesn't trigger a layout.
On 2015/08/19 16:59:52, skobes wrote:
> Can you move this comment to appear just above the "if" statement that checks
> for OOVERLAY?  That's what it's really referring to.

Done.

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.h (right):

https://codereview.chromium.org/1292513002/diff/340001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.h:154: bool
needsScrollbarReconstruction() const;
On 2015/08/19 16:59:52, skobes wrote:
> Can this be private?

Done.

[skobes, 2015-08-19 19:40:18]

https://codereview.chromium.org/1292513002/diff/360001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/360001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:689: // overflow:auto
may need to lay out again if scrollbars got added/removed and even on
reconstruction of scrollbars
Rephrase this comment:

    // We need to layout again if scrollbars are added or removed by
overflow:auto,
    // or by changing between native and custom.

https://codereview.chromium.org/1292513002/diff/360001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:692: ||
(box().style()->overflowX() == OSCROLL && !horizontalScrollbar());
fix indentation

https://codereview.chromium.org/1292513002/diff/360001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:697:
setHasHorizontalScrollbar(true);
This still looks wrong.  Auto and OSCROLL are separate cases.  In the auto case,
we want to pass the value of hasHorizontalOverflow.  In the OSCROLL case, we
want to pass true.

[MuVen, 2015-08-19 20:37:33]

PTAL.

https://codereview.chromium.org/1292513002/diff/360001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/360001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:689: // overflow:auto
may need to lay out again if scrollbars got added/removed and even on
reconstruction of scrollbars
On 2015/08/19 19:40:18, skobes wrote:
> Rephrase this comment:
> 
>     // We need to layout again if scrollbars are added or removed by
> overflow:auto,
>     // or by changing between native and custom.

Done.

https://codereview.chromium.org/1292513002/diff/360001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:692: ||
(box().style()->overflowX() == OSCROLL && !horizontalScrollbar());
On 2015/08/19 19:40:17, skobes wrote:
> fix indentation

Done.

https://codereview.chromium.org/1292513002/diff/360001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:697:
setHasHorizontalScrollbar(true);
On 2015/08/19 19:40:18, skobes wrote:
> This still looks wrong.  Auto and OSCROLL are separate cases.  In the auto
case,
> we want to pass the value of hasHorizontalOverflow.  In the OSCROLL case, we
> want to pass true.

True, Miss-read your previous comment, Even i was thinking the same But just
followed your earlier comment.

[skobes, 2015-08-19 20:43:27]

lgtm

Please update the first line of the change description to describe the positive
effect of the change, i.e.

"Create custom scrollbars when html element has overflow:scroll".

[MuVen, 2015-08-19 20:44:36]

On 2015/08/19 20:43:27, skobes wrote:
> lgtm
> 
> Please update the first line of the change description to describe the
positive
> effect of the change, i.e.
> 
> "Create custom scrollbars when html element has overflow:scroll".

Thanks for your patience !!!

[MuVen, 2015-08-19 20:44:45]

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

[commit-bot: I haz the power, 2015-08-19 20:45:10]

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

[commit-bot: I haz the power, 2015-08-19 21:52:22]

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

[commit-bot: I haz the power, 2015-08-19 21:52:23]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[MuVen, 2015-08-20 08:23:29]

Patch Set 10 has layout tests failure because,
Overlay scrollbars are disabled if
scrollSize(HorizontalScrollbar/VerticalScrollbar)
is zero (http://crbug.com/415031). But in that case 
(box().style()->overflowX() == OSCROLL && !horizontalScrollbar())
is true which shall trigger creation of overlay scrollbars failing the test
cases.

I have added a check !ScrollbarTheme::theme()->usesOverlayScrollbars() in
OSCROLL case.

PTAL.

[skobes, 2015-08-20 19:44:02]

https://codereview.chromium.org/1292513002/diff/400001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/400001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:693: ||
(box().style()->overflowX() == OSCROLL && !horizontalScrollbar() &&
!ScrollbarTheme::theme()->usesOverlayScrollbars());
Won't this fail to reconstruct in the case where we switch from custom back to
(overlay) native and scrollSize returns > 0?

[MuVen, 2015-08-21 14:26:47]

Patchset #13 (id:440001) has been deleted

[MuVen, 2015-08-21 14:27:00]

Patchset #12 (id:420001) has been deleted

[MuVen, 2015-08-21 14:27:38]

https://codereview.chromium.org/1292513002/diff/400001/Source/core/paint/Depr...
File Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/1292513002/diff/400001/Source/core/paint/Depr...
Source/core/paint/DeprecatedPaintLayerScrollableArea.cpp:693: ||
(box().style()->overflowX() == OSCROLL && !horizontalScrollbar() &&
!ScrollbarTheme::theme()->usesOverlayScrollbars());
On 2015/08/20 19:44:02, skobes wrote:
> Won't this fail to reconstruct in the case where we switch from custom back to
> (overlay) native and scrollSize returns > 0?

True, this logic goes for toss when switching from custom to overlay. I have
moved the logic of destroying the scroll-bars when we cant scroll in that
orientation. This should be fine. PTAL.

[skobes, 2015-08-21 15:52:44]

lgtm

[MuVen, 2015-08-21 17:06:50]

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

[commit-bot: I haz the power, 2015-08-21 17:07:04]

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

[commit-bot: I haz the power, 2015-08-21 22:33:32]

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

[commit-bot: I haz the power, 2015-08-21 22:33:33]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

[MuVen, 2015-08-22 01:36:16]

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

[commit-bot: I haz the power, 2015-08-22 01:36:34]

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

[commit-bot: I haz the power, 2015-08-22 05:56:53]

Committed patchset #12 (id:460001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201039