Bug 144604 – [Content Extensions] Use less memory to store the json input

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alex Christensen]

We have several copies of the JSON data that is an input to the content extension compiler.  We need to copy it at least once to convert the utf8 characters to unicode characters for the JSON parser (until we make the JSON parser nicer), but we can free that memory as soon as we are done with it to reduce the peak memory usage.

[Alex Christensen]

Created attachment 252346 [details]
Patch

[Darin Adler]

Comment on attachment 252346 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/_WKUserContentExtensionStore.mm:61
> +    encodedContentExtension = nil; // shouldn't be necessary.

This is wrong. This is not ARC, so you need to explicitly call release. Setting to nil won’t do anything at all. This will simply cause the data to leak.

[Alex Christensen]

Created attachment 252394 [details]
Patch

[Alex Christensen]

Created attachment 252396 [details]
Patch

[Alex Christensen]

I imagine I don't need to retain the NSStrings in _WKUserContentExtensionStoreTest because they are static, right?
Will I need to simultaneously change any code in any programs that call compileContentExtensionForIdentifier?  What about ARC code that calls compileContentExtensionForIdentifier?

[Benjamin Poulain]

Comment on attachment 252396 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/_WKUserContentExtensionStore.h:37
> -- (void)compileContentExtensionForIdentifier:(NSString *)identifier encodedContentExtension:(NSString *)encodedContentExtension completionHandler:(void (^)(_WKUserContentFilter *, NSError *))completionHandler;
> +- (void)compileContentExtensionForIdentifier:(NSString *)identifier encodedContentExtension:(NSString *) NS_RELEASES_ARGUMENT encodedContentExtension completionHandler:(void (^)(_WKUserContentFilter *, NSError *))completionHandler;

The patch looks like a good move, but I don't know about NS_RELEASES_ARGUMENT.
Does that work with ARC?

[Alex Christensen]

Created attachment 252410 [details]
Patch

[Alex Christensen]

Releasing the NSString should be done in another patch.  It's another small gain, but I'm not as familiar with ObjC reference counting.

[Benjamin Poulain]

Comment on attachment 252410 [details]
Patch

lgtm

[Alex Christensen]

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