Bug 212939 – Swift Overlay API refinements

• Component: WebKit API
• Status: RESOLVED FIXED

[James Savage]

* JavaScript completion handlers should be Optional.
* Standardize use of `completionHandler` as argument label.
* Fix missing imports in Umbrella Header, this isn't part of the overlay, but prevents use of WKScriptMessageHandlerWithReply from Swift code.

[Radar WebKit Bug Importer]

<rdar://problem/64140013>

[James Savage]

Created attachment 401400 [details]
Patch

[Darin Adler]

Comment on attachment 401400 [details]
Patch

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

> Source/WebKit/Shared/API/Cocoa/WebKit.h:52
>  #import <WebKit/WKScriptMessageHandler.h>
> +#import <WebKit/WKScriptMessageHandlerWithReply.h>

Doubt we need both of these. Could remove the other one.

[James Savage]

Created attachment 401401 [details]
Patch

[James Savage]

Comment on attachment 401400 [details]
Patch

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

>> Source/WebKit/Shared/API/Cocoa/WebKit.h:52
>> +#import <WebKit/WKScriptMessageHandlerWithReply.h>
> 
> Doubt we need both of these. Could remove the other one.

We could move the definition of WKScriptMessageHandlerWithReply to WKScriptMessageHandler.h, but the umbrella header must import every .h in the WebKit.framework/Headers directory, otherwise it cause issues for modular imports (which are the only type available to Swift, or @import WebKit in Objective-C).

[Darin Adler]

(In reply to James Savage from comment #5)
> the umbrella header must import every .h in
> the WebKit.framework/Headers directory

Directly, not indirectly? OK. Understood, I guess.

[James Savage]

Yeah, there is technically another way to achieve this. I could instead modify the WebKit.modulemap file to include an explicit "header" directive for any headers not included in the umbrella, but that is not a standard practice. The documentation around this is at http://clang.llvm.org/docs/Modules.html#header-declaration. In either case transitive imports are a problem, it just moves around where the explicit declaration goes, and keeping everything in the umbrella header is much more straightforward.

If we did anything I'd suggest defining both protocols in the same WKScriptMessageHandler.h, but the two protocols don't inherit from each other, and it appears that WebKit generally avoids declaring multiple protocol or class types in the same API header.

[EWS]

Committed r262849: <https://trac.webkit.org/changeset/262849>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 401401 [details].