Bug 143544 – Allow LaunchServices to handle URLs on link navigations

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Sam Weinig]

Allow LaunchServices to handle URLs on link navigations.

[Sam Weinig]

<rdar://problem/19446826>

[Sam Weinig]

Created attachment 250398 [details]
Patch

[WebKit Commit Bot]

Attachment 250398 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:231:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:231:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:237:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:291:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:912:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:924:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:2365:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 7 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Anders Carlsson]

Comment on attachment 250398 [details]
Patch

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

> Source/WebCore/platform/spi/ios/LaunchServicesSPI.h:39
> +typedef void (^ LSAppLinkOpenCompletionHandler)(BOOL success, NSError *error);

No space after ^.

> Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:240
> +            dispatch_async(dispatch_get_main_queue(), [completionHandler, success] {
> +                completionHandler(success);
> +            });

This is copying the completion handler on a background queue - that's not thread safe and will break if the completion handler has bound strings for example. I think you're going to have to create an std::function pointer and pass it across the thread boundary and then explicitly delete it on the main thread.

> Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:255
> +        tryAppLink(localNavigationAction, [localListener, localNavigationAction] (bool followedLinkToApp) {

What's ensuring that the web view doesn't go away while waiting for the completion handler to be invoked?

[Sam Weinig]

(In reply to comment #4)
> Comment on attachment 250398 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=250398&action=review
> 
> > Source/WebCore/platform/spi/ios/LaunchServicesSPI.h:39
> > +typedef void (^ LSAppLinkOpenCompletionHandler)(BOOL success, NSError *error);
> 
> No space after ^.

Fixed.

> 
> > Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:240
> > +            dispatch_async(dispatch_get_main_queue(), [completionHandler, success] {
> > +                completionHandler(success);
> > +            });
> 
> This is copying the completion handler on a background queue - that's not
> thread safe and will break if the completion handler has bound strings for
> example. I think you're going to have to create an std::function pointer and
> pass it across the thread boundary and then explicitly delete it on the main
> thread.

Ok. How about if I just use a block?

> 
> > Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:255
> > +        tryAppLink(localNavigationAction, [localListener, localNavigationAction] (bool followedLinkToApp) {
> 
> What's ensuring that the web view doesn't go away while waiting for the
> completion handler to be invoked?

Nothing, though that should be fine.  The listener will be invalidated and thus when it is called with success or failure, it will just do nothing.

[Sam Weinig]

Committed r182648: <http://trac.webkit.org/changeset/182648>

[Alexey Proskuryakov]

Build fix in <http://trac.webkit.org/r182650>.