Bug 132309 - Scrollbars do not update properly when topContentInset changes dynamically
Summary: Scrollbars do not update properly when topContentInset changes dynamically
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Beth Dakin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-04-28 16:03 PDT by Beth Dakin
Modified: 2014-04-28 21:10 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.87 KB, patch)
2014-04-28 16:07 PDT, Beth Dakin
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2014-04-28 16:03:00 PDT
Scrollbars do not update properly when topContentInset changes dynamically.

<rdar://problem/16642232>
Comment 1 Beth Dakin 2014-04-28 16:07:39 PDT
Created attachment 230331 [details]
Patch
Comment 2 Tim Horton 2014-04-28 16:10:04 PDT
Comment on attachment 230331 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230331&action=review

> Source/WebCore/page/FrameView.cpp:962
> +    // To fully update scrollbars, we need to call both updateScrolbars() and frameViewDidChangeSize() which will
> +    // will update all of the scrolling-related layers.

I don't think the comment is necessary.

> Source/WebCore/page/FrameView.cpp:964
> +    if (RenderView* renderView = this->renderView()) {

We already have a local and already early returned for RenderView.
Comment 3 Beth Dakin 2014-04-28 16:19:47 PDT
Thank you! http://trac.webkit.org/changeset/167911
Comment 4 Ryosuke Niwa 2014-04-28 18:23:46 PDT
Looks like this regressed PLT by ~2%
Comment 5 Beth Dakin 2014-04-28 20:24:21 PDT
(In reply to comment #4)
> Looks like this regressed PLT by ~2%

This seems very unlikely…
Comment 6 Ryosuke Niwa 2014-04-28 21:10:12 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > Looks like this regressed PLT by ~2%
> 
> This seems very unlikely…

Yeah, looks like it was a temporary fluke.  The numbers look better now.