Bug 158095 – Expose content extension failure error codes in SPI

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Expose content extension failure error codes in SPI

[Alex Christensen]

Created attachment 279820 [details]
Patch

[Alex Christensen]

Created attachment 279836 [details]
Patch

[Alex Christensen]

Created attachment 279841 [details]
Patch

[Sam Weinig]

Comment on attachment 279841 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/_WKUserContentExtensionStore.h:45
> +// These correspond with the error codes in UserContentExtensionStore::Error.
> +typedef NS_ENUM(NSInteger, _WKUserContentExtensionStoreErrorCode) {

Instead of requiring these to stay in sync, we should explicitly convert from one to the other with a switch statement like we do for other errors.  Also, SPI headers should not refer to internals.

I also think this should be an extension to WKErrorCode and we should switch to using the WKError domain.  I believe you do that by using constants of type WKErrorCode, but am not sure.  I'll let Anders say.

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/_WKUserContentExtensionStore.mm:68
> +        EXPECT_EQ(error.code, _WKUserContentExtensionStoreErrorCode::CompileFailed);

This should test the domain as well.

[Alex Christensen]

Created attachment 279920 [details]
Patch

[Alex Christensen]

(In reply to comment #4)
> I also think this should be an extension to WKErrorCode and we should switch
> to using the WKError domain.  I believe you do that by using constants of
> type WKErrorCode, but am not sure.  I'll let Anders say.
I disagree and have implemented it as a separate domain.  If Anders tells me otherwise, I'll change it.

[Anders Carlsson]

Comment on attachment 279920 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/_WKUserContentExtensionStore.h:50
> +    LookupFailed = 1,
> +    VersionMismatch,
> +    CompileFailed,
> +    RemoveFailed,

These should be prefixed correctly,

_WKUserContentExtensionStoreErrorLookupFailed, _WKUserContentExtensionStoreErrorVersionMismatch etc.

[Alex Christensen]

Created attachment 279976 [details]
Patch

[Anders Carlsson]

Comment on attachment 279976 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/_WKUserContentExtensionStore.h:44
> +WK_EXTERN NSString * const _ContentExtensionsDomain WK_AVAILABLE(WK_MAC_TBA, WK_IOS_TBA);

Should be _WKUserContentExtensionsDomain.

> Source/WebKit2/UIProcess/API/Cocoa/_WKUserContentExtensionStore.mm:87
> +            auto code = toWKUserContentExtensionStoreErrorCode(API::UserContentExtensionStore::Error::CompileFailed);
> +            rawHandler(nil, [NSError errorWithDomain:_ContentExtensionsDomain code:code userInfo:userInfo]);

No need to use toWKUserContentExtensionStoreErrorCode here - just pass the WebKit code directly.

> Source/WebKit2/UIProcess/API/Cocoa/_WKUserContentExtensionStore.mm:126
> +            auto code = toWKUserContentExtensionStoreErrorCode(API::UserContentExtensionStore::Error::RemoveFailed);
> +            rawHandler([NSError errorWithDomain:_ContentExtensionsDomain code:code userInfo:userInfo]);

No need to use toWKUserContentExtensionStoreErrorCode here - just pass the WebKit code directly.

[Alex Christensen]

Passing the WebKit code doesn't like converting an enum to an NSInteger, so I just used the new API codes.

http://trac.webkit.org/changeset/201457