Improve API::SerializedScriptValue::deserialize to not allocate a new JSContext every second

• Component: WebKit API
• Status: RESOLVED FIXED

[Simon Taylor]

In r186229 (related to bug 146416) a static API::SerializedScriptValue::deserialize() function was introduced.

This function converts from a SerializedScriptValue to an Objective C object, and is currently used in the implementation of these APIs that return results from the content process:
 - evaluateJavaScript:completionHandler: for WKWebView, and
 - userContentController:didReceiveScriptMessage: for WKScriptMessageHandler

SerializedScriptValue::deserialize uses a JSContext to do the conversion, with a SharedJSContext helper to cache this between calls so it does not need to create a completely fresh one each time (this was bug 146416).

However the SharedJSContext clears the cached JSContext one second after its initial creation due to this line:
https://trac.webkit.org/browser/webkit/trunk/Source/WebKit/UIProcess/API/Cocoa/APISerializedScriptValueCocoa.mm#L49

Any apps that make heavy use of passing data from the WKWebView to the native side therefore end up recreating the JSContext for the Objective C value conversion once per second.

The most annoying implication of this is when debugging - this only-for-conversion JSContext shows up in Safari's Develop menu, and makes the options to automatically show inspector and pause JSContexts essentially useless. It was digging into what those were that led to this entire adventure :)

There is also a small performance impact - from Instruments System Trace it looks like my iPod 7 spends around 1.5ms doing [[JSContext alloc] init]. It's only at most once per second, so not huge overall, but still would be nice to get rid of this cost.

Here's my initial thoughts for improvements:
 - Reset the timeout to clear the shared JSContext in every ensureContext() call (so active use of evaluateJavascript will keep it around indefinitely)
 - Increment a ref count on the shared context for each WKScriptMessageHandler attached

I don't know the memory cost of the shared JSContext, but attaching a WKScriptMessageHandler is a pretty clear signal the developer expects messages to be coming back from the WebView, so I'd argue keeping the shared context around in this case makes sense.

[chad]

Looks like a solution for this issue: WebKit::WebUserContentControllerProxy::didReceiveMessage spents long time on JSContext initailization. https://developer.apple.com/forums/thread/702626

[Chris Dumez]

Created attachment 456819 [details]
Patch

[Geoffrey Garen]

Comment on attachment 456819 [details]
Patch

r=me

[EWS]

Committed r292483 (249334@main): <https://commits.webkit.org/249334@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456819 [details].

[Radar WebKit Bug Importer]

<rdar://problem/91366573>