Bug 130554 – toThis() on a JSWorkerGlobalScope should return a JSProxy and not undefined

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

This is the root cause for a radar defect <rdar://problem/15940627> where we can't render the website https://developer.mozilla.org/en-US/demos/detail/zen-photon-garden/launch.  The website uses workers to do rendering.  The script file is basically of the format of:

"use strict";

(function () {
    var locals.....

    this.myFunc1 = function () { ... }
    this.myFunc2 = function () { ... }
}).call(this);

Due to the string mode, when we process the to_this byte code, we return undefined per strict mode.  We then try to set "myFunc1" on undefined and throw an exception.  Instead, the worker's WorkerGlobalScope should have a proxy that we can set attributes on.

We'll do this based on the work for https://bugs.webkit.org/show_bug.cgi?id=130553

[Michael Saboff]

Created attachment 227382 [details]
Patch

This patch depends on <https://bugs.webkit.org/show_bug.cgi?id=130553> - "Change CodeGeneratorJS.pm special cases for "DOMWindow" to be general purpose" and won't build without it.  Therefore the EWS bots will fail trying to build.  I can resubmit this patch for testing after 130553 lands.

[Geoffrey Garen]

Comment on attachment 227382 [details]
Patch

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

r=me with a bug fix below, and please fix the build before landing.

> Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:134
> +    if (classInfo == JSProxy::info())
> +        return static_cast<JSDedicatedWorkerGlobalScope*>(jsCast<JSProxy*>(asObject(value))->target());

This needs to be jsDynamicCast<JSDedicatedWorkerGlobalScope*> because it's not guaranteed that all proxies proxy to JSDedicatedWorkerGlobalScope.

[Michael Saboff]

This patch needs some more work as some jsregress tests fail because of it.

[Michael Saboff]

Created attachment 227498 [details]
Patch for Landing

Already reviewed by Geoff.

Made two small fixes to reviewed patch:
 1. Eliminated ASSERT in testapi.c that the object used to create the GlobalObject is the same as the result from JSContextGetGlobalObject(context).  It is now a proxy to the original object.
 2. Move the call to setGlobalThis() in both JSGlobalObject::finishCreation() to after init has been called so our prototype can be created before we create the JSProxy that needs the prototype.

These changes rubber stamped by Geoff.

[Build Bot]

Comment on attachment 227498 [details]
Patch for Landing

Attachment 227498 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5082087598063616

New failing tests:
media/adopt-node-crash.html
js/dom/global-constructors-attributes-shared-worker.html
js/dom/global-constructors-attributes-dedicated-worker.html

[Build Bot]

Created attachment 227510 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 227498 [details]
Patch for Landing

Attachment 227498 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4974679055925248

New failing tests:
js/dom/global-constructors-attributes-shared-worker.html
js/dom/global-constructors-attributes-dedicated-worker.html

[Build Bot]

Created attachment 227513 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 227498 [details]
Patch for Landing

Attachment 227498 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6342983427293184

New failing tests:
js/dom/global-constructors-attributes-shared-worker.html
js/dom/global-constructors-attributes-dedicated-worker.html

[Build Bot]

Created attachment 227514 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 227498 [details]
Patch for Landing

Attachment 227498 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4779732637843456

New failing tests:
js/dom/global-constructors-attributes-shared-worker.html
js/dom/global-constructors-attributes-dedicated-worker.html

[Build Bot]

Created attachment 227519 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Michael Saboff]

Created attachment 227568 [details]
Updated patch to fix test failures

Expect this to fix js/dom/global-constructors-attributes-dedicated-worker.html and js/dom/global-constructors-attributes-shared-worker.html.

[Michael Saboff]

Committed r166126: <http://trac.webkit.org/changeset/166126>

[Michael Saboff]

<rdar://problem/15940627>

[Michael Saboff]

Change rolled out in <http://trac.webkit.org/changeset/166248> due to page load time performance regression.

[Michael Saboff]

Relanded in change set r166415 - <http://trac.webkit.org/changeset/166415>