Bug 31859 – Make world selection for JSC IsolatedWorlds automagical.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Gavin Barraclough]

WebCore presently has to explicitly specify the world before entering into JSC, which is a little fragile (particularly since property access via a getter/setter might invoke execution). Instead derive the current world from the lexical global object.

[Gavin Barraclough]

Created attachment 43827 [details]
The Patch

[Adam Barth]

This is similar to how we're doing things in V8.  Although, in V8, we're using the dynamic global object.  The lexical global object is probably slightly better from a robustness point of view, but I'm not sure there's an observable difference.

[Gavin Barraclough]

Ah, that's interesting to know.  Agreed, using the LGO or DGO should probably not make any difference – all accessible global objects should be in the same world.

I chose to use the LGO following the Garen Hypothesis, which simply states that, "All uses of the dynamic global object are wrong." ;-)

[Adam Barth]

Yeah.  I'll switch V8 to lexical once this lands.  I'd like to move the V8 implementation to use the WebCore machinery that JSC is using, but I'm holding off on that until the WebCore stuff is relatively "stable."  Let me know when might be a good time for me to do that.

[Geoffrey Garen]

I think you can just write this:

+    static DOMWrapperWorld* cachedNormalWorld = 0;
+    if (LIKELY(cachedNormalWorld != 0))
+        return cachedNormalWorld;
+    
+    cachedNormalWorld = normalWorld(*JSDOMWindow::commonJSGlobalData());
+    return cachedNormalWorld;

as this:

static DOMWrapperWorld* cachedNormalWorld = normalWorld(*JSDOMWindow::commonJSGlobalData());
return cachedNormalWorld;

The initializer should only run once.

Let's beef up this comment:

+        // For non-JS bindings. Silently ignores the JavaScript exception if any, only work in the normal world.

Maybe:

"Do not call these functions. They are just scaffolding to support the Objective-C bindings. They operate in the main thread normal world, and they swallow JS exceptions."

r=me

[Gavin Barraclough]

Sending        JavaScriptCore/API/JSBase.cpp
Sending        JavaScriptCore/API/JSObjectRef.cpp
Sending        JavaScriptCore/ChangeLog
Sending        JavaScriptCore/JavaScriptCore.exp
Sending        JavaScriptCore/runtime/JSGlobalData.cpp
Sending        JavaScriptCore/runtime/JSGlobalData.h
Sending        WebCore/ChangeLog
Sending        WebCore/WebCore.base.exp
Sending        WebCore/bindings/js/JSCallbackData.cpp
Sending        WebCore/bindings/js/JSCallbackData.h
Sending        WebCore/bindings/js/JSCustomXPathNSResolver.cpp
Sending        WebCore/bindings/js/JSDOMBinding.cpp
Sending        WebCore/bindings/js/JSDOMBinding.h
Sending        WebCore/bindings/js/JSDOMWindowBase.cpp
Sending        WebCore/bindings/js/JSDOMWindowBase.h
Sending        WebCore/bindings/js/JSEventListener.cpp
Sending        WebCore/bindings/js/JSHTMLDocumentCustom.cpp
Sending        WebCore/bindings/js/JSNodeFilterCondition.cpp
Sending        WebCore/bindings/js/JSQuarantinedObjectWrapper.cpp
Sending        WebCore/bindings/js/ScheduledAction.cpp
Sending        WebCore/bindings/js/ScriptController.cpp
Sending        WebCore/bindings/js/ScriptFunctionCall.cpp
Sending        WebCore/bindings/js/ScriptObjectQuarantine.cpp
Sending        WebCore/bindings/js/ScriptState.cpp
Sending        WebCore/bindings/js/ScriptState.h
Sending        WebCore/bindings/js/WorkerScriptController.cpp
Sending        WebCore/bindings/objc/WebScriptObject.mm
Sending        WebCore/bridge/NP_jsobject.cpp
Sending        WebCore/bridge/jni/jni_jsobject.mm
Sending        WebCore/dom/NodeFilter.h
Sending        WebCore/dom/NodeIterator.h
Sending        WebCore/dom/TreeWalker.h
Sending        WebCore/inspector/InspectorController.cpp
Sending        WebCore/inspector/JavaScriptCallFrame.cpp
Sending        WebKit/mac/ChangeLog
Sending        WebKit/mac/WebView/WebScriptDebugDelegate.mm
Transmitting file data ....................................
Committed revision 51512.

[Eric Seidel (no email)]

I assume you're manually copying the long commit log into the bug?  (You're certainly welcome to do so!  I just wanted to make sure none of our tools was still doing that as Mark Rowe had previously given me grief about it in the past...)

bugzilla-tool land-diff --no-build
functions as a drop-in replacement for "svn ci", but it may not be a perfect replacement for you.  Without --no-build it will build and test your patch before committing (which is useful if you are getting up for lunch or something).  If there is a tool you'd like us to build to improve your workflow, I certainly would love to hear about it and very much welcome WebKit bugs assigned to me on the subject!