Bug 132309

Summary: Scrollbars do not update properly when topContentInset changes dynamically
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: Layout and RenderingAssignee: Beth Dakin <bdakin>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, bdakin, rniwa, sam, simon.fraser, thorton
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch thorton: review+

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.