Bug 123586 - Add a WKRemoteObject class
Summary: Add a WKRemoteObject class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-31 13:02 PDT by Anders Carlsson
Modified: 2013-10-31 16:54 PDT (History)
1 user (show)

See Also:


Attachments
Patch (15.31 KB, patch)
2013-10-31 13:18 PDT, Anders Carlsson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2013-10-31 13:02:41 PDT
Add a WKRemoteObject class
Comment 1 Anders Carlsson 2013-10-31 13:18:35 PDT
Created attachment 215669 [details]
Patch
Comment 2 mitz 2013-10-31 13:34:36 PDT
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 :-(
Comment 3 WebKit Commit Bot 2013-10-31 16:54:30 PDT
Comment on attachment 215669 [details]
Patch

Clearing flags on attachment: 215669

Committed r158407: <http://trac.webkit.org/changeset/158407>
Comment 4 WebKit Commit Bot 2013-10-31 16:54:31 PDT
All reviewed patches have been landed.  Closing bug.