Bug 79126 - ScrollingCoordinator::coordinatesScrollingForFrameView should be conditional on compositing being active
Summary: ScrollingCoordinator::coordinatesScrollingForFrameView should be conditional ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-21 10:40 PST by James Robinson
Modified: 2012-02-21 13:38 PST (History)
5 users (show)

See Also:


Attachments
Patch (3.81 KB, patch)
2012-02-21 10:42 PST, James Robinson
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2012-02-21 10:40:08 PST
ScrollingCoordinator::coordinatesScrollingForFrameView should be conditional on compositing being active
Comment 1 James Robinson 2012-02-21 10:42:42 PST
Created attachment 127995 [details]
Patch
Comment 2 Anders Carlsson 2012-02-21 10:57:16 PST
Comment on attachment 127995 [details]
Patch

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

> Source/WebCore/page/scrolling/ScrollingCoordinator.cpp:-205
> +    setScrollLayer(scrollLayerForFrameView(frameView));
>      frameViewLayoutUpdated(frameView);
>      recomputeWheelEventHandlerCount();
>      updateShouldUpdateScrollLayerPositionOnMainThread();
> -    setScrollLayer(scrollLayerForFrameView(frameView));

This change isn't mentioned in the change log; why is it needed?
Comment 3 James Robinson 2012-02-21 12:28:54 PST
That slipped into this patch by accident.  The idea is to initialize the scroll layer before anything else because in Chromium we're buffering properties on the layer, not on a separate data structure.  This raises an interesting question about what happens when calls are made in a different order - for example, if something calls frameViewWheelEventHandlerCountChanged() before frameViewRootLayerDidChange() should it behave the same as if the calls were made in the reverse order or not?  Today in the mac implementation it seems that this would vary depending on whether the sync timer fires or not between the calls, although with the way the callsites are coded I don't think it makes a difference.
Comment 4 James Robinson 2012-02-21 13:38:43 PST
Committed r108392: <http://trac.webkit.org/changeset/108392>