Bug 146358 – ScriptMessageHandlerDelegate::didPostMessage() should reuse its JSContext instance

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Mark Lam]

Currently, ScriptMessageHandlerDelegate::didPostMessage() creates a new JSContext on every invocation.  It would be more efficient to just create it once and reuse it for all subsequent calls.

[Mark Lam]

<rdar://problem/21349359>

[Mark Lam]

Created attachment 255662 [details]
the patch.

[Mark Lam]

Comment on attachment 255662 [details]
the patch.

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

> Source/WebKit2/ChangeLog:9
> +        time it is called.  This is JSContext is used only once to deserialized a JSON object

typo: extra "is" before JSContext.  Will fix this before landing.

[Mark Lam]

Created attachment 255665 [details]
patch 2: use a static JSContext.

Sam pointed out offline that didPostMessage() is always called from the UI thread, and we only need a static instance if the JSContext.  Updated the patch to reflect this.

[Anders Carlsson]

Comment on attachment 255665 [details]
patch 2: use a static JSContext.

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

> Source/WebKit2/UIProcess/API/Cocoa/WKUserContentController.mm:97
> +            static JSContext* context = nil;
> +            if (!context)
> +                context = [[JSContext alloc] init];

You can just assign to the variable directly.

> Source/WebKit2/UIProcess/API/Cocoa/WKUserContentController.mm:101
> +            id body = [value toObject];

value.toObject?

> Source/WebKit2/UIProcess/API/Cocoa/WKUserContentController.mm:103
> +
> +            RetainPtr<WKScriptMessage> message = adoptNS([[WKScriptMessage alloc] _initWithBody:body webView:fromWebPageProxy(page) frameInfo:frameInfo.get() name:m_name.get()]);

I'd use auto here.

[Mark Lam]

Thanks for the reviews.  Ander's feedback as been applied.  Landed in r186010: <http://trac.webkit.org/r186010>.

[Mark Lam]

Some build bots were not liking the #import <WTF/MainThread.h>.  I removed it for now in r186014: <http://trac.webkit.org/r186014> to green the bots while I investigate.

[Mark Lam]

#inport and assertion re-added in r186017: <http://trac.webkit.org/r186017>.