Bug 127085

Summary: Expose scrollView on WKView
Product: WebKit Reporter: Ian Henderson <ian>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, mitz
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch none

Description Ian Henderson 2014-01-16 00:17:23 PST
We need to expose scrollView as a property on WKView.
Comment 1 Ian Henderson 2014-01-16 00:53:32 PST
Created attachment 221342 [details]
patch
Comment 2 mitz 2014-01-16 02:37:55 PST
Comment on attachment 221342 [details]
patch

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

> Source/WebKit2/UIProcess/API/ios/WKScrollView.mm:50
> +- (NSMethodSignature *)methodSignatureForSelector:(SEL)aSelector

We’d normally name the argument “selector”, regardless of what the NSObject header calls it.

> Source/WebKit2/UIProcess/API/ios/WKScrollView.mm:60
> +- (BOOL)respondsToSelector:(SEL)aSelector

Ditto.

> Source/WebKit2/UIProcess/API/ios/WKScrollView.mm:65
> +- (void)forwardInvocation:(NSInvocation *)anInvocation

We’d normally name the argument “invocation”.

> Source/WebKit2/UIProcess/API/ios/WKScrollView.mm:78
> +{
> +    BOOL messageHandled = NO;
> +    if ([_internalDelegate respondsToSelector:[anInvocation selector]]) {
> +        [anInvocation invokeWithTarget:_internalDelegate];
> +        messageHandled = YES;
> +    }
> +    if ([_externalDelegate respondsToSelector:[anInvocation selector]]) {
> +        [anInvocation invokeWithTarget:_externalDelegate];
> +        messageHandled = YES;
> +    }
> +    if (!messageHandled)
> +        [super forwardInvocation:anInvocation];
> +}

I wonder if we should also override -forwardingTargetForSelector: so that we can do faster (possibly by an order of magnitude) forwarding of selectors that only one of the delegates implements. Probably not unless we see that this is slow and the alternative is faster.
Comment 3 mitz 2014-01-16 02:40:18 PST
Comment on attachment 221342 [details]
patch

I didn’t mean to revert Antti’s cq+, I simply started writing my comments before he set the flag. I’m changing it back to cq+ but feel free to make changes.
Comment 4 WebKit Commit Bot 2014-01-16 02:49:06 PST
Comment on attachment 221342 [details]
patch

Clearing flags on attachment: 221342

Committed r162121: <http://trac.webkit.org/changeset/162121>
Comment 5 WebKit Commit Bot 2014-01-16 02:49:08 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 mitz 2014-01-16 07:11:57 PST
(In reply to comment #3)
> (From update of attachment 221342 [details])
> I didn’t mean to revert Antti’s cq+, I simply started writing my comments before he set the flag. I’m changing it back to cq+ but feel free to make changes.

Actually, I should have left it at cq? so that this only got landed after review by a WebKit2 owner.
Comment 7 mitz 2014-05-10 16:55:34 PDT
(In reply to comment #2)
> I wonder if we should also override -forwardingTargetForSelector: so that we can do faster (possibly by an order of magnitude) forwarding of selectors that only one of the delegates implements. Probably not unless we see that this is slow and the alternative is faster.

Tim Horton just discovered that indeed this is slow and the alternative is faster. See bug 132790.