Bug 103589 – Add an objective-c object graph wrapper and coder for WKConnection message bodies

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

[Sam Weinig]

Add an objective-c object graph wrapper and coder for WKConnection message bodies

[Sam Weinig]

Created attachment 176629 [details]
Patch

[Anders Carlsson]

Comment on attachment 176629 [details]
Patch

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

> Source/WebKit2/ChangeLog:3
> +        Add an objective-c object graph wrapper and coder for WKConnection message bodies

Objective-C

> Source/WebKit2/ChangeLog:26
> +        Added. Is an APIObject that wraps an Objective-C object graph.

That's some weird grammar.

> Source/WebKit2/Shared/mac/ObjCObjectGraph.h:43
> +    id root() const { return m_root.get(); }

rootObject?

> Source/WebKit2/Shared/mac/ObjCObjectGraph.h:53
> +    RetainPtr<id> m_root;

m_rootObject?

> Source/WebKit2/Shared/mac/ObjCObjectGraphCoders.mm:95
> +        if (type == UnknownType) {
> +            encoder << static_cast<uint32_t>(UnknownType);
> +            return true;
> +        }

Wouldn't it be better to throw an exception here? We never want to allow unknown types.

> Source/WebKit2/Shared/mac/ObjCObjectGraphCoders.mm:126
> +            NSUInteger size = [dictionary count];
> +            NSArray *keys = [dictionary allKeys];
> +            NSArray *values = [dictionary allValues];
> +
> +            encoder << static_cast<uint64_t>(size);
> +
> +            for (NSUInteger i = 0; i < size; ++i) {
> +                encoder << Owner([keys objectAtIndex:i]);
> +                encoder << Owner([values objectAtIndex:i]);
> +            }

I think you should use the block enumeration method here if possible.

[Sam Weinig]

Created attachment 176642 [details]
Patch

[Anders Carlsson]

Comment on attachment 176642 [details]
Patch

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

> Source/WebKit2/Shared/mac/ObjCObjectGraphCoders.mm:94
> +            return true;

No need for the return true here :)

[Sam Weinig]

Committed r136095: <http://trac.webkit.org/changeset/136095>