Bug 123586 – Add a WKRemoteObject class

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

[Anders Carlsson]

Add a WKRemoteObject class

[Anders Carlsson]

Created attachment 215669 [details]
Patch

Comment on attachment 215669 [details]
Patch

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

> Source/WebKit2/Shared/API/Cocoa/WKRemoteObject.h:26
> +#import <WebKit2/WKFoundation.h>

I prefer that we use ""-style imports in internal headers such as this one.

> Source/WebKit2/Shared/API/Cocoa/WKRemoteObject.mm:33
> +#if WK_API_ENABLED

I normally put this between the #import blocks.

> Source/WebKit2/Shared/API/Cocoa/WKRemoteObject.mm:54
> +        return true;

YES

> Source/WebKit2/Shared/API/Cocoa/WKRemoteObject.mm:56
> +    return protocol_conformsToProtocol([_interface protocol], protocol);

I’d write the whole thing in a single line:
    return [super conformsToProtocol:protocol] || protocol_conformsToProtocol([_interface protocol], protocol);

Is this really the optimal order to check these conditions?

> Source/WebKit2/Shared/API/Cocoa/WKRemoteObject.mm:94
> +

I would expect to see overrides of -respondsToSelector:, -isKindOfClass: and -isMemberOfClass: as well.

> Source/WebKit2/Shared/API/Cocoa/WKRemoteObjectRegistry.mm:68
> +    return [remoteObject.leakRef() autorelease];

This entire method is not thread-safe :-(

[WebKit Commit Bot]

Comment on attachment 215669 [details]
Patch

Clearing flags on attachment: 215669

Committed r158407: <http://trac.webkit.org/changeset/158407>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.