Bug 158446

Summary: self.hasOwnProperty() does not work inside Web workers.
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: BindingsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, beidson, cdumez, cgarcia, commit-queue, esprehn+autocc, ggaren, keith_miller, kondapallykalyan, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 149117    
Attachments:
Description Flags
Initial attempt (not working)
none
Initial patch (not working)
none
WIP Patch
none
WIP Patch
none
Patch
none
Archive of layout-test-results from ews112 for mac-yosemite
none
Patch
none
Patch none

Description Chris Dumez 2016-06-06 16:05:56 PDT
I have noticed that we are failing most of the W3C worker tests. E.g:
http://w3c-test.org/workers/interfaces.worker
http://w3c-test.org/IndexedDB/interfaces.worker

Those tests mostly seems to be failing because it incorrectly assumes WorkerGlobalScope.self is undefined in some cases.

For e.g. In a worker:
"Self: " + self;

Throws an exception:
undefined is not an object

self.toString() is [Object Undefined] instead of [object DedicatedWorkerGlobalScope].

However, self == undefined is false.

I looked at objectProtoFuncToString(ExecState* exec) in ObjectPrototype.cpp and it does:
JSValue thisValue = exec->thisValue().toThis(exec, StrictMode);
if (thisValue.isUndefinedOrNull())
    return JSValue::encode(thisValue.isUndefined() ? vm.smallStrings.undefinedObjectString() : vm.smallStrings.nullObjectString()); // We end up in this code path because thisValue is undefined.

However, exec->thisValue() is:
Cell: 0x10f93f100 (0x10f94df00:[DedicatedWorkerGlobalScope, {parseInt:100, Object:101, Function:102, Array:103, RegExp:104, RangeError:105, TypeError:106, PrivateSymbol.Object:107, PrivateSymbol.Array:108, String:109, Symbol:110, Number:111, Error:112, Map:113, Promise:114, ArrayBuffer:115, eval:116, Intl:117, Reflect:118, IDBCursor:119, IDBCursorWithValue:120, IDBDatabase:121, IDBFactory:122, IDBIndex:123, IDBKeyRange:124, IDBObjectStore:125, IDBOpenDBRequest:126, IDBRequest:127, IDBTransaction:128, IDBVersionChangeEvent:129, indexedDB:130, WebSocket:131}, NonArray, Proto:0x10f997fe0, Dictionary, Leaf]), ID: 190

So the issue seems to be that toThis() converts thisValue() which seems valid into undefined for DedicatedWorkerGlobalScope.
Comment 1 Chris Dumez 2016-06-06 16:06:20 PDT
<rdar://problem/26638397>
Comment 2 Chris Dumez 2016-06-06 16:11:35 PDT
For the window, it works fine:
thisValue: Cell: 0x1195fbfd0 (0x119560280:[JSDOMWindowShell, {}, NonArray, Proto:0x1195dbaf0, Leaf]), ID: 748

thisValue (after toThis): Cell: 0x1195fbfd0 (0x119560280:[JSDOMWindowShell, {}, NonArray, Proto:0x1195dbaf0, Leaf]), ID: 748
Comment 3 Chris Dumez 2016-06-06 16:14:13 PDT
JSGlobalObject implements toThis() like so:
JSValue JSGlobalObject::toThis(JSCell*, ExecState* exec, ECMAMode ecmaMode)
{
    if (ecmaMode == StrictMode)
        return jsUndefined();
    return exec->globalThisValue();
}

Since objectProtoFuncToString() calls toThis() with StrictMode, this is why we end up with undefined.

In the case of the Window, it works because thisValue is a JSDOMWindowShell, not a JSDOMWindow. Therefore, it does not subclass JSGlobalObject.

I am not sure yet how to fix this though.
Comment 4 Chris Dumez 2016-06-07 09:06:43 PDT
Note that even though I focused on Stringification of DedicatedWorkerGlobalScope. This toThis() bug is breaking more worker related things. The reason most of the W3C tests are failing is because self.hasOwnProperty() is broken in workers (because of the same toThis() bug), not because of stringification.
Comment 5 Chris Dumez 2016-06-07 09:09:56 PDT
Geoff suggested we expose a Proxy to the WorkerGlobalScope to JS, instead of the WorkerGlobalScope itself. This is similar to what we do with Window / WindowShell already. However, my attempts to do this have not worked so far (weird crashes during GC).
Comment 6 Mark Lam 2016-06-07 09:24:09 PDT
(In reply to comment #5)
> Geoff suggested we expose a Proxy to the WorkerGlobalScope to JS, instead of
> the WorkerGlobalScope itself. This is similar to what we do with Window /
> WindowShell already. However, my attempts to do this have not worked so far
> (weird crashes during GC).

Can you upload your current working patch so that we can help see if there's something amiss?
Comment 7 Geoffrey Garen 2016-06-07 09:56:49 PDT
Specifically, I think we want a JSProxy whose proxy type is PureForwardingProxyType and whose target is the WorkerGlobalScope.
Comment 8 Chris Dumez 2016-06-07 10:12:16 PDT
Created attachment 280717 [details]
Initial attempt (not working)
Comment 9 Chris Dumez 2016-06-07 10:13:10 PDT
Ok, I attached my very initial attempt patch from yesterday, which is likely very wrong as it crashes badly.
Comment 10 Chris Dumez 2016-06-07 10:18:42 PDT
Crash:
Crashed Thread:        15  WebCore: Worker

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x00000000000000c0
Exception Note:        EXC_CORPSE_NOTIFY

VM Regions Near 0xc0:
--> 
    __TEXT                 0000000101aa7000-0000000101aa9000 [    8K] r-x/rwx SM=COW  /Volumes/VOLUME/*/WebKit.framework/Versions/A/XPCServices/com.apple.WebKit.WebContent.Development.xpc/Contents/MacOS/com.apple.WebKit.WebContent.Development

Thread 15 Crashed:: WebCore: Worker
0   com.apple.JavaScriptCore      	0x00000001031cb09f JSC::SlotVisitor::setMarkedAndAppendToMarkStack(JSC::JSCell*) + 95 (memory:2894)
1   com.apple.JavaScriptCore      	0x0000000102fe6182 JSC::JSSymbolTableObject::visitChildren(JSC::JSCell*, JSC::SlotVisitor&) + 18 (WriteBarrier.h:92)
2   com.apple.JavaScriptCore      	0x0000000102fda005 JSC::JSSegmentedVariableObject::visitChildren(JSC::JSCell*, JSC::SlotVisitor&) + 21 (SegmentedVector.h:106)
3   com.apple.JavaScriptCore      	0x0000000102f79cfc JSC::JSGlobalObject::visitChildren(JSC::JSCell*, JSC::SlotVisitor&) + 28 (WriteBarrier.h:92)
4   com.apple.WebCore             	0x0000000103fea829 WebCore::JSDOMGlobalObject::visitChildren(JSC::JSCell*, JSC::SlotVisitor&) + 25 (HashTraits.h:182)
5   com.apple.JavaScriptCore      	0x00000001031cb35c JSC::SlotVisitor::drain() + 284 (SlotVisitor.cpp:226)
6   com.apple.JavaScriptCore      	0x0000000102e465c1 JSC::Heap::markRoots(double, void*, void*, int (&) [37]) + 849 (Heap.cpp:890)
7   com.apple.JavaScriptCore      	0x0000000102e48df8 JSC::Heap::collectImpl(JSC::HeapOperation, void*, void*, int (&) [37]) + 600 (memory:2710)
8   com.apple.JavaScriptCore      	0x0000000102e48b70 JSC::Heap::collect(JSC::HeapOperation) + 96 (Heap.cpp:1124)
9   com.apple.JavaScriptCore      	0x0000000102e3f4cc JSC::GCActivityCallback::doWork() + 76 (GCActivityCallback.cpp:88)
10  com.apple.JavaScriptCore      	0x0000000102e4f1a3 JSC::HeapTimer::timerDidFire(__CFRunLoopTimer*, void*) + 179 (HeapTimer.cpp:103)
11  com.apple.CoreFoundation      	0x00007fffab6ce364 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
12  com.apple.CoreFoundation      	0x00007fffab6cdfef __CFRunLoopDoTimer + 1071
13  com.apple.CoreFoundation      	0x00007fffab6cdb4a __CFRunLoopDoTimers + 298
14  com.apple.CoreFoundation      	0x00007fffab6c5341 __CFRunLoopRun + 2065
15  com.apple.CoreFoundation      	0x00007fffab6c48cd CFRunLoopRunSpecific + 285
16  com.apple.WebCore             	0x00000001049f2bac WebCore::WorkerRunLoop::runInMode(WebCore::WorkerGlobalScope*, WebCore::ModePredicate const&, WebCore::WorkerRunLoop::WaitMode) + 332 (WorkerRunLoop.cpp:193)
17  com.apple.WebCore             	0x00000001049f2e18 WebCore::WorkerRunLoop::runInMode(WebCore::WorkerGlobalScope*, WTF::String const&, WebCore::WorkerRunLoop::WaitMode) + 104 (WorkerRunLoop.cpp:140)
18  com.apple.WebCore             	0x00000001049f6930 WebCore::WorkerThreadableLoader::loadResourceSynchronously(WebCore::WorkerGlobalScope*, WebCore::ResourceRequest const&, WebCore::ThreadableLoaderClient&, WebCore::ThreadableLoaderOptions const&) + 208 (RefPtr.h:71)
19  com.apple.WebCore             	0x00000001049f4a4d WebCore::WorkerScriptLoader::loadSynchronously(WebCore::ScriptExecutionContext*, WebCore::URL const&, WebCore::CrossOriginRequestPolicy, WebCore::ContentSecurityPolicyEnforcement) + 221 (WorkerScriptLoader.cpp:75)
20  com.apple.WebCore             	0x00000001049eee0a WebCore::WorkerGlobalScope::importScripts(WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul> const&, int&) + 490 (WorkerScriptLoader.h:65)
21  com.apple.WebCore             	0x0000000103b352c2 WebCore::DedicatedWorkerGlobalScope::importScripts(WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul> const&, int&) + 18 (WorkerGlobalScope.h:87)
22  com.apple.WebCore             	0x000000010431163d WebCore::JSWorkerGlobalScope::importScripts(JSC::ExecState&) + 301 (JSWorkerGlobalScopeCustom.cpp:76)
23  com.apple.WebCore             	0x0000000104310a0c WebCore::jsWorkerGlobalScopeInstanceFunctionImportScripts(JSC::ExecState*) + 140 (JSWorkerGlobalScope.cpp:1585)
24  ???                           	0x00004d340e603ca8 0 + 84885974826152
25  com.apple.JavaScriptCore      	0x0000000103092d87 llint_entry + 24665 (LowLevelInterpreter.asm:718)
26  com.apple.JavaScriptCore      	0x000000010308cb4b vmEntryToJavaScript + 299 (LowLevelInterpreter64.asm:251)
27  com.apple.JavaScriptCore      	0x0000000102f048ce JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 158 (JITCode.cpp:81)
28  com.apple.JavaScriptCore      	0x0000000102eb8516 JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) + 15334 (Interpreter.cpp:955)
29  com.apple.JavaScriptCore      	0x0000000102b0d925 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) + 469 (Completion.cpp:107)
30  com.apple.WebCore             	0x00000001049f4458 WebCore::WorkerScriptController::evaluate(WebCore::ScriptSourceCode const&, WTF::NakedPtr<JSC::Exception>&) + 120 (MarkedBlock.h:235)
31  com.apple.WebCore             	0x00000001049f4386 WebCore::WorkerScriptController::evaluate(WebCore::ScriptSourceCode const&) + 38 (WorkerScriptController.cpp:115)
32  com.apple.WebCore             	0x00000001049f5a66 WebCore::WorkerThread::workerThread() + 534 (utility:753)
33  com.apple.JavaScriptCore      	0x00000001032f17c2 WTF::threadEntryPoint(void*) + 178 (functional:1766)
34  com.apple.JavaScriptCore      	0x00000001032f1b6f WTF::wtfThreadEntryPoint(void*) + 15 (memory:2723)
35  libsystem_pthread.dylib       	0x00007fffbf9f7c2b _pthread_body + 176
36  libsystem_pthread.dylib       	0x00007fffbf9f7b7b _pthread_start + 274
37  libsystem_pthread.dylib       	0x00007fffbf9f73bd thread_start + 13
Comment 11 Geoffrey Garen 2016-06-07 10:29:16 PDT
Comment on attachment 280717 [details]
Initial attempt (not working)

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

> Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.h:69
> +        JSC::Strong<JSC::JSProxy> m_proxy;

You'd have to change this to a visited GC pointer to avoid a leak.
Comment 12 Chris Dumez 2016-06-07 10:31:47 PDT
(In reply to comment #11)
> Comment on attachment 280717 [details]
> Initial attempt (not working)
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=280717&action=review
> 
> > Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.h:69
> > +        JSC::Strong<JSC::JSProxy> m_proxy;
> 
> You'd have to change this to a visited GC pointer to avoid a leak.

Sure, what's the type/class of a GC pointer? (sorry, not familiar with JSC types)
Comment 13 Chris Dumez 2016-06-07 10:38:42 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > Comment on attachment 280717 [details]
> > Initial attempt (not working)
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=280717&action=review
> > 
> > > Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.h:69
> > > +        JSC::Strong<JSC::JSProxy> m_proxy;
> > 
> > You'd have to change this to a visited GC pointer to avoid a leak.
> 
> Sure, what's the type/class of a GC pointer? (sorry, not familiar with JSC
> types)

Never mind, Mark pointed me to WriteBarrier on IRC. I'll update the patch.
Comment 14 Geoffrey Garen 2016-06-07 10:42:08 PDT
I think you need to set JSGlobalObject::setGlobalThis to the proxy as well. See how JSDOMWindow does this.

The call to jsWorkerGlobalScopeInstanceFunctionImportScripts probably has the wrong 'this' value, causing toJSDedicatedWorkerGlobalScope to return nullptr.

BTW, it seems strange that toJSDedicatedWorkerGlobalScope returns nullptr instead of crashing. When is it OK to nullptr?
Comment 15 Chris Dumez 2016-06-07 11:50:16 PDT
Created attachment 280728 [details]
Initial patch (not working)
Comment 16 Chris Dumez 2016-06-07 11:51:35 PDT
Thread 15 Crashed:: WebCore: Worker
0   com.apple.JavaScriptCore      	0x0000000106ac2c14 WTFCrash + 36 (Assertions.cpp:317)
1   com.apple.WebCore             	0x0000000108f5c7a7 void* JSC::allocateCell<JSC::Structure>(JSC::Heap&, unsigned long) + 215 (JSCellInlines.h:135)
2   com.apple.WebCore             	0x0000000108f5c5dc void* JSC::allocateCell<JSC::Structure>(JSC::Heap&) + 28 (JSCellInlines.h:145)
3   com.apple.WebCore             	0x0000000108f5c48c JSC::Structure::create(JSC::VM&, JSC::JSGlobalObject*, JSC::JSValue, JSC::TypeInfo const&, JSC::ClassInfo const*, unsigned char, unsigned int) + 188 (StructureInlines.h:42)
4   com.apple.WebCore             	0x000000010a4050c3 WebCore::JSWorkerGlobalScopeBase::finishCreation(JSC::VM&) + 99 (JSWorkerGlobalScopeBase.cpp:58)
5   com.apple.WebCore             	0x000000010a3fe37d WebCore::JSWorkerGlobalScope::finishCreation(JSC::VM&) + 45 (JSWorkerGlobalScope.cpp:401)
6   com.apple.WebCore             	0x0000000109df84e8 WebCore::JSDedicatedWorkerGlobalScope::finishCreation(JSC::VM&) + 40 (JSDedicatedWorkerGlobalScope.cpp:114)
7   com.apple.WebCore             	0x000000010b34e21b WebCore::JSDedicatedWorkerGlobalScope::create(JSC::VM&, JSC::Structure*, WTF::Ref<WebCore::DedicatedWorkerGlobalScope>&&) + 123 (JSDedicatedWorkerGlobalScope.h:36)
8   com.apple.WebCore             	0x000000010b34d612 WebCore::WorkerScriptController::initScript() + 642 (WorkerScriptController.cpp:92)
9   com.apple.WebCore             	0x0000000109e320f9 WebCore::WorkerScriptController::initScriptIfNeeded() + 57 (WorkerScriptController.h:94)
10  com.apple.WebCore             	0x000000010b34e502 WebCore::WorkerScriptController::evaluate(WebCore::ScriptSourceCode const&, WTF::NakedPtr<JSC::Exception>&) + 66 (WorkerScriptController.cpp:128)
11  com.apple.WebCore             	0x000000010b34e3cc WebCore::WorkerScriptController::evaluate(WebCore::ScriptSourceCode const&) + 76 (WorkerScriptController.cpp:114)
12  com.apple.WebCore             	0x000000010b3515d5 WebCore::WorkerThread::workerThread() + 853 (WorkerThread.cpp:166)
13  com.apple.WebCore             	0x000000010b351275 WebCore::WorkerThread::workerThreadStart(void*) + 21 (WorkerThread.cpp:140)
14  com.apple.JavaScriptCore      	0x0000000106b21f29 WTF::createThread(void (*)(void*), void*, char const*)::$_0::operator()() const + 25 (Threading.cpp:84)
15  com.apple.JavaScriptCore      	0x0000000106b21efd void std::__1::__invoke_void_return_wrapper<void>::__call<WTF::createThread(void (*)(void*), void*, char const*)::$_0&>(WTF::createThread(void (*)(void*), void*, char const*)::$_0&&&) + 45 (__functional_base:469)
16  com.apple.JavaScriptCore      	0x0000000106b21ea9 std::__1::__function::__func<WTF::createThread(void (*)(void*), void*, char const*)::$_0, std::__1::allocator<WTF::createThread(void (*)(void*), void*, char const*)::$_0>, void ()>::operator()() + 41 (functional:1437)
17  com.apple.JavaScriptCore      	0x000000010617e43a std::__1::function<void ()>::operator()() const + 26 (functional:1817)
18  com.apple.JavaScriptCore      	0x0000000106b20b47 WTF::threadEntryPoint(void*) + 151 (Threading.cpp:60)
19  com.apple.JavaScriptCore      	0x0000000106b22511 WTF::wtfThreadEntryPoint(void*) + 289 (ThreadingPthreads.cpp:164)
20  libsystem_pthread.dylib       	0x00007fffbf9f7c2b _pthread_body + 176
21  libsystem_pthread.dylib       	0x00007fffbf9f7b7b _pthread_start + 274
22  libsystem_pthread.dylib       	0x00007fffbf9f73bd thread_start + 13
Comment 17 Chris Dumez 2016-06-07 11:52:57 PDT
(In reply to comment #16)
> Thread 15 Crashed:: WebCore: Worker
> 0   com.apple.JavaScriptCore      	0x0000000106ac2c14 WTFCrash + 36
> (Assertions.cpp:317)
> 1   com.apple.WebCore             	0x0000000108f5c7a7 void*
> JSC::allocateCell<JSC::Structure>(JSC::Heap&, unsigned long) + 215
> (JSCellInlines.h:135)
> 2   com.apple.WebCore             	0x0000000108f5c5dc void*
> JSC::allocateCell<JSC::Structure>(JSC::Heap&) + 28 (JSCellInlines.h:145)
> 3   com.apple.WebCore             	0x0000000108f5c48c
> JSC::Structure::create(JSC::VM&, JSC::JSGlobalObject*, JSC::JSValue,
> JSC::TypeInfo const&, JSC::ClassInfo const*, unsigned char, unsigned int) +
> 188 (StructureInlines.h:42)
> 4   com.apple.WebCore             	0x000000010a4050c3
> WebCore::JSWorkerGlobalScopeBase::finishCreation(JSC::VM&) + 99
> (JSWorkerGlobalScopeBase.cpp:58)
> 5   com.apple.WebCore             	0x000000010a3fe37d
> WebCore::JSWorkerGlobalScope::finishCreation(JSC::VM&) + 45
> (JSWorkerGlobalScope.cpp:401)
> 6   com.apple.WebCore             	0x0000000109df84e8
> WebCore::JSDedicatedWorkerGlobalScope::finishCreation(JSC::VM&) + 40
> (JSDedicatedWorkerGlobalScope.cpp:114)
> 7   com.apple.WebCore             	0x000000010b34e21b
> WebCore::JSDedicatedWorkerGlobalScope::create(JSC::VM&, JSC::Structure*,
> WTF::Ref<WebCore::DedicatedWorkerGlobalScope>&&) + 123
> (JSDedicatedWorkerGlobalScope.h:36)
> 8   com.apple.WebCore             	0x000000010b34d612
> WebCore::WorkerScriptController::initScript() + 642
> (WorkerScriptController.cpp:92)
> 9   com.apple.WebCore             	0x0000000109e320f9
> WebCore::WorkerScriptController::initScriptIfNeeded() + 57
> (WorkerScriptController.h:94)
> 10  com.apple.WebCore             	0x000000010b34e502
> WebCore::WorkerScriptController::evaluate(WebCore::ScriptSourceCode const&,
> WTF::NakedPtr<JSC::Exception>&) + 66 (WorkerScriptController.cpp:128)
> 11  com.apple.WebCore             	0x000000010b34e3cc
> WebCore::WorkerScriptController::evaluate(WebCore::ScriptSourceCode const&)
> + 76 (WorkerScriptController.cpp:114)
> 12  com.apple.WebCore             	0x000000010b3515d5
> WebCore::WorkerThread::workerThread() + 853 (WorkerThread.cpp:166)
> 13  com.apple.WebCore             	0x000000010b351275
> WebCore::WorkerThread::workerThreadStart(void*) + 21 (WorkerThread.cpp:140)
> 14  com.apple.JavaScriptCore      	0x0000000106b21f29 WTF::createThread(void
> (*)(void*), void*, char const*)::$_0::operator()() const + 25
> (Threading.cpp:84)
> 15  com.apple.JavaScriptCore      	0x0000000106b21efd void
> std::__1::__invoke_void_return_wrapper<void>::__call<WTF::createThread(void
> (*)(void*), void*, char const*)::$_0&>(WTF::createThread(void (*)(void*),
> void*, char const*)::$_0&&&) + 45 (__functional_base:469)
> 16  com.apple.JavaScriptCore      	0x0000000106b21ea9
> std::__1::__function::__func<WTF::createThread(void (*)(void*), void*, char
> const*)::$_0, std::__1::allocator<WTF::createThread(void (*)(void*), void*,
> char const*)::$_0>, void ()>::operator()() + 41 (functional:1437)
> 17  com.apple.JavaScriptCore      	0x000000010617e43a
> std::__1::function<void ()>::operator()() const + 26 (functional:1817)
> 18  com.apple.JavaScriptCore      	0x0000000106b20b47
> WTF::threadEntryPoint(void*) + 151 (Threading.cpp:60)
> 19  com.apple.JavaScriptCore      	0x0000000106b22511
> WTF::wtfThreadEntryPoint(void*) + 289 (ThreadingPthreads.cpp:164)
> 20  libsystem_pthread.dylib       	0x00007fffbf9f7c2b _pthread_body + 176
> 21  libsystem_pthread.dylib       	0x00007fffbf9f7b7b _pthread_start + 274
> 22  libsystem_pthread.dylib       	0x00007fffbf9f73bd thread_start + 13

Ok, the issue seems to be that I cannot call JSC::Structure::create() from inside finishCreation().
Comment 18 Chris Dumez 2016-06-07 12:47:49 PDT
Created attachment 280734 [details]
WIP Patch
Comment 19 Chris Dumez 2016-06-07 12:51:02 PDT
With this latest patch, there are no more crashes but workers are very broken. E.g.
'DedicatedWorkerGlobalScope' in self // returns false
Comment 20 Geoffrey Garen 2016-06-07 12:53:02 PDT
Is DedicatedWorkerGlobalScope a property of DedicatedWorkerGlobalScope?
Comment 21 Chris Dumez 2016-06-07 12:53:49 PDT
(In reply to comment #20)
> Is DedicatedWorkerGlobalScope a property of DedicatedWorkerGlobalScope?

Yes, it is.
Comment 22 Chris Dumez 2016-06-07 12:54:44 PDT
(In reply to comment #21)
> (In reply to comment #20)
> > Is DedicatedWorkerGlobalScope a property of DedicatedWorkerGlobalScope?
> 
> Yes, it is.

static const HashTableValue JSDedicatedWorkerGlobalScopeTableValues[] =
{
    { "onmessage", CustomAccessor, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsDedicatedWorkerGlobalScopeOnmessage), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSDedicatedWorkerGlobalScopeOnmessage) } },
    { "DedicatedWorkerGlobalScope", DontEnum, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsDedicatedWorkerGlobalScopeDedicatedWorkerGlobalScopeConstructor), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSDedicatedWorkerGlobalScopeDedicatedWorkerGlobalScopeConstructor) } },
    { "postMessage", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsDedicatedWorkerGlobalScopeInstanceFunctionPostMessage), (intptr_t) (1) } },
};

In WebKitBuild/Debug/DerivedSources/WebCore/JSDedicatedWorkerGlobalScope.cpp
Comment 23 Chris Dumez 2016-06-07 13:17:00 PDT
Comment on attachment 280734 [details]
WIP Patch

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

> Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:64
> +    auto* structure = Structure::create(vm, 0, jsNull(), TypeInfo(PureForwardingProxyType, JSProxy::StructureFlags), JSProxy::info());

I have tried replacing this line with:
auto* structure = JSProxy::createStructure(vm, this, jsNull(), PureForwardingProxyType);

So that the globalObject is set to this on the JSProxy but this did not help.
Comment 24 Chris Dumez 2016-06-07 14:24:08 PDT
Mark suggested something. I am in the process of trying it out.
Comment 25 Chris Dumez 2016-06-07 15:37:07 PDT
Created attachment 280742 [details]
WIP Patch

New work-in-progress patch. It is not pretty but it seems to work. The pass rates on W3C test suites are way up:
http://w3c-test.org/workers/interfaces.worker: 20 passes -> 50 passes (out of 128)
http://w3c-test.org/IndexedDB/interfaces.worker 0 passes -> 145 passes (out of 156)
Comment 26 Chris Dumez 2016-06-07 16:16:29 PDT
Wow, looks like the layout tests are even passing. I'll add some testing and changelogs.
Comment 27 Chris Dumez 2016-06-07 16:42:54 PDT
Created attachment 280745 [details]
Patch
Comment 28 Build Bot 2016-06-07 17:35:06 PDT
Comment on attachment 280745 [details]
Patch

Attachment 280745 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1462570

Number of test failures exceeded the failure limit.
Comment 29 Build Bot 2016-06-07 17:35:11 PDT
Created attachment 280748 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 30 Chris Dumez 2016-06-07 18:02:00 PDT
Created attachment 280754 [details]
Patch
Comment 31 Chris Dumez 2016-06-07 18:02:38 PDT
*** Bug 157600 has been marked as a duplicate of this bug. ***
Comment 32 Brady Eidson 2016-06-07 18:28:43 PDT
Comment on attachment 280754 [details]
Patch

Seems great.
Comment 33 Chris Dumez 2016-06-08 08:45:30 PDT
Comment on attachment 280754 [details]
Patch

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

> Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp:104
> +    return proxy();

I don't even think I need these custom bindings anymore since toJS() will return the proxy. I'll try it out.
Comment 34 Chris Dumez 2016-06-08 09:05:32 PDT
Created attachment 280808 [details]
Patch
Comment 35 Geoffrey Garen 2016-06-08 10:25:25 PDT
Comment on attachment 280808 [details]
Patch

r=me
Comment 36 Chris Dumez 2016-06-08 10:29:45 PDT
Comment on attachment 280808 [details]
Patch

Clearing flags on attachment: 280808

Committed r201808: <http://trac.webkit.org/changeset/201808>
Comment 37 Chris Dumez 2016-06-08 10:29:51 PDT
All reviewed patches have been landed.  Closing bug.