Bug 129140 - Start fixing the view states driven by the WKScrollView
Summary: Start fixing the view states driven by the WKScrollView
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: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-20 20:05 PST by Benjamin Poulain
Modified: 2014-02-20 20:48 PST (History)
2 users (show)

See Also:


Attachments
Patch (20.37 KB, patch)
2014-02-20 20:14 PST, Benjamin Poulain
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2014-02-20 20:05:04 PST
Start fixing the view states driven by the WKScrollView
Comment 1 Benjamin Poulain 2014-02-20 20:14:38 PST
Created attachment 224826 [details]
Patch
Comment 2 Tim Horton 2014-02-20 20:23:57 PST
Comment on attachment 224826 [details]
Patch

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

> Source/WebKit2/UIProcess/API/ios/WKViewIOS.mm:271
> +- (void)_updateContentWindow

"window"?
Comment 3 Tim Horton 2014-02-20 20:24:44 PST
Comment on attachment 224826 [details]
Patch

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

> Source/WebKit2/ChangeLog:8
> +        WKScrollView creates a "window" over WKContentview with an area that is exposed,

capital View

> Source/WebKit2/ChangeLog:9
> +        an area that is unobcured and a certain scale.

typo obscured

> Source/WebKit2/ChangeLog:11
> +        Instead of having 3 loosely related path for updating WKContentView

path*s*

> Source/WebKit2/ChangeLog:12
> +        when the content "window" change, everything is consolidated behind the

"window"?
Comment 4 Simon Fraser (smfr) 2014-02-20 20:26:45 PST
Comment on attachment 224826 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +
> +

Extra space.

> Source/WebKit2/ChangeLog:8
> +        WKScrollView creates a "window" over WKContentview with an area that is exposed,

over the

> Source/WebKit2/ChangeLog:9
> +        an area that is unobcured and a certain scale.

unobscured and with a certain scale?

> Source/WebKit2/ChangeLog:11
> +        Instead of having 3 loosely related path for updating WKContentView

paths

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:384
> +- (void)_updateContentWindow

I think "window" is a bad term to use here; it's too overloaded.

updateViewRects: or updateVisibleRect: would be fine I think.

> Source/WebKit2/UIProcess/API/ios/WKContentView.mm:161
> +    if (oldRoundedPosition != newRoundedPosition)
> +        _page->scrollingCoordinatorProxy()->scrollPositionChangedViaDelegatedScrolling(_page->scrollingCoordinatorProxy()->rootScrollingNodeID(), newRoundedPosition);
> +

It's probably fine to call this when the position didn't actually change?

> Source/WebKit2/UIProcess/API/ios/WKContentView.mm:165
> +        exposedRect.scale(scale);
> +        drawingArea->setExposedRect(exposedRect);

It's not clear why we have to scale this rect; it's already visibleRectInContentCoordinates.
Comment 5 Benjamin Poulain 2014-02-20 20:48:39 PST
Committed r164466: <http://trac.webkit.org/changeset/164466>