[webkit-reviews] review requested: [Bug 28614] Unnecessary horizontal scrollbar in Gmail under Chromium. ScrollView::updateScrollbars uses stale value for off/on/auto scrollbar mode. : [Attachment 38436] Perform layout before checking scrollbar modes

[bugzilla-daemon at webkit.org]

Mark Mentovai <mark at chromium.org> has asked Dave Hyatt <hyatt at apple.com> for
review:
Bug 28614: Unnecessary horizontal scrollbar in Gmail under Chromium. 
ScrollView::updateScrollbars uses stale value for off/on/auto scrollbar mode.
https://bugs.webkit.org/show_bug.cgi?id=28614

Attachment 38436: Perform layout before checking scrollbar modes
https://bugs.webkit.org/attachment.cgi?id=38436&action=review

------- Additional Comments from Mark Mentovai <mark at chromium.org>
> I am not convinced this is right.  You aren't supposed to just always invoke
> visibleContentsResized.

The existing code always calls visibleContentsResized, though.	I'm just moving
the call up earlier to avoid using stale data for the off/on/auto value.

There's one additional behavior change.  The existing code avoids
visibleContentsResized if if both the horizontal and vertical scrollbars are
forced on or off.  With the proposed patch, this is not taken into account.  I
considered writing the patch to respect these values, but felt that would be
unwise.  The bug as it relates to Gmail involves the vertical scrollbar mode
being changed from auto to forced-on during the call to visibleContentsResized.
 It seems just as plausible for forced-on/off scrollbars to change back to auto
in that call.

> Also note that any change to this logic has to be replicated in
> WebDynamicScrollbarsView on Mac to keep the two in sync.

This version of the patch takes that into account.  It also moves a couple of
minor things around, with no functional changes, to keep it synchronized with
ScrollView::updateScrollbars.

> Finally I see no reason to change the word "recur" to "recurse."  They mean
> exactly the same thing, and changing around English just because you don't
like
> it pisses me off.

OK.  It should be pretty clear that I didn't intend to piss anyone off.

I've run this through the layout test suite and it doesn't introduce any
regressions.  It fixes the Gmail bug in Chromium.