⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 201808 in webkit


Ignore:
Timestamp:
Jun 8, 2016, 10:31:12 AM (10 years ago)
Author:
Chris Dumez
Message:

self.hasOwnProperty() does not work inside Web workers
​https://bugs.webkit.org/show_bug.cgi?id=158446
<rdar://problem/26638397>

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

Add a factory function to JSProxy to create a JSProxy without a target.
Also make the setTarget() method public so that the target can now be
set after creation. This is needed so that we can create a proxy for
JSWorkerGlobalScope, then create the JSWorkerGlobalScope object,
passing it the proxy and finally set the target on the proxy.

  • runtime/JSProxy.h:

(JSC::JSProxy::create):

Source/WebCore:

W3C tests for workers were severely broken on WebKit because
self.hasOwnProperty() did not work inside workers. The reason is that
hasOwnProperty() (and other methods like toString()) call toThis() in
StrictMode on thisValue. However, in the case of 'self' in workers,
self was a DedicatedWorkerGlobalScope, which is a JSGlobalObject.
JSGlobalObject::toThis() returns jsUndefined() when called in strict
mode. As a result, we would end up with exceptions such as "undefined
is not an object" when calling self.hasOwnProperty() in workers.

To address the problem, this patch introduces a JSProxy whose proxy
type is PureForwardingProxyType and whose target is the
WorkerGlobalScope. This JSProxy is what we expose to the JavaScript,
instead of the JSWorkerGlobalScope itself. As a result, toThis() now
behaves as expected and self.hasOwnProperty() works inside workers.

This patch greatly improves our pass rate on several W3C tests:
​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)

Tests: fast/workers/self-hasOwnProperty.html

fast/workers/self-toString.html

  • bindings/js/JSWorkerGlobalScopeBase.cpp:

(WebCore::JSWorkerGlobalScopeBase::finishCreation):
(WebCore::JSWorkerGlobalScopeBase::visitChildren):
(WebCore::toJS):

  • bindings/js/JSWorkerGlobalScopeBase.h:

(WebCore::JSWorkerGlobalScopeBase::proxy):

  • bindings/js/WorkerScriptController.cpp:

(WebCore::WorkerScriptController::initScript):

  • bindings/scripts/CodeGeneratorJS.pm:

(GenerateHeader):
(GenerateImplementation):

LayoutTests:

Add tests to make sure that self.toString() and self.hasOwnProperty()
now work in workers.

  • fast/workers/self-hasOwnProperty-expected.txt: Added.
  • fast/workers/self-hasOwnProperty.html: Added.
  • fast/workers/self-toString-expected.txt: Added.
  • fast/workers/self-toString.html: Added.
Location:
trunk
Files:
4 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r201805 r201808  
     12016-06-08  Chris Dumez  <cdumez@apple.com>
     2
     3        self.hasOwnProperty() does not work inside Web workers
     4        https://bugs.webkit.org/show_bug.cgi?id=158446
     5        <rdar://problem/26638397>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        Add tests to make sure that self.toString() and self.hasOwnProperty()
     10        now work in workers.
     11
     12        * fast/workers/self-hasOwnProperty-expected.txt: Added.
     13        * fast/workers/self-hasOwnProperty.html: Added.
     14        * fast/workers/self-toString-expected.txt: Added.
     15        * fast/workers/self-toString.html: Added.
     16
    1172016-06-06  Antti Koivisto  <antti@apple.com>
    218

  • trunk/Source/JavaScriptCore/ChangeLog

    r201807 r201808  
     12016-06-08  Chris Dumez  <cdumez@apple.com>
     2
     3        self.hasOwnProperty() does not work inside Web workers
     4        https://bugs.webkit.org/show_bug.cgi?id=158446
     5        <rdar://problem/26638397>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        Add a factory function to JSProxy to create a JSProxy without a target.
     10        Also make the setTarget() method public so that the target can now be
     11        set after creation. This is needed so that we can create a proxy for
     12        JSWorkerGlobalScope, then create the JSWorkerGlobalScope object,
     13        passing it the proxy and finally set the target on the proxy.
     14
     15        * runtime/JSProxy.h:
     16        (JSC::JSProxy::create):
     17
    1182016-06-07  Filip Pizlo  <fpizlo@apple.com>
    219

  • trunk/Source/JavaScriptCore/runtime/JSProxy.h

    r198270 r201808  
    4343    }
    4444
     45    static JSProxy* create(VM& vm, Structure* structure)
     46    {
     47        JSProxy* proxy = new (NotNull, allocateCell<JSProxy>(vm.heap)) JSProxy(vm, structure);
     48        proxy->finishCreation(vm);
     49        return proxy;
     50    }
     51
    4552    static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype, JSType proxyType)
    4653    {
    … …  
    5360    JSObject* target() const { return m_target.get(); }
    5461    static ptrdiff_t targetOffset() { return OBJECT_OFFSETOF(JSProxy, m_target); }
     62
     63    JS_EXPORT_PRIVATE void setTarget(VM&, JSGlobalObject*);
    5564
    5665protected:
    … …  
    7281
    7382    JS_EXPORT_PRIVATE static void visitChildren(JSCell*, SlotVisitor&);
    74 
    75     JS_EXPORT_PRIVATE void setTarget(VM&, JSGlobalObject*);
    7683
    7784    JS_EXPORT_PRIVATE static String className(const JSObject*);

  • trunk/Source/WebCore/ChangeLog

    r201805 r201808  
     12016-06-08  Chris Dumez  <cdumez@apple.com>
     2
     3        self.hasOwnProperty() does not work inside Web workers
     4        https://bugs.webkit.org/show_bug.cgi?id=158446
     5        <rdar://problem/26638397>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        W3C tests for workers were severely broken on WebKit because
     10        self.hasOwnProperty() did not work inside workers. The reason is that
     11        hasOwnProperty() (and other methods like toString()) call toThis() in
     12        StrictMode on thisValue. However, in the case of 'self' in workers,
     13        self was a DedicatedWorkerGlobalScope, which is a JSGlobalObject.
     14        JSGlobalObject::toThis() returns jsUndefined() when called in strict
     15        mode. As a result, we would end up with exceptions such as "undefined
     16        is not an object" when calling self.hasOwnProperty() in workers.
     17
     18        To address the problem, this patch introduces a JSProxy whose proxy
     19        type is PureForwardingProxyType and whose target is the
     20        WorkerGlobalScope. This JSProxy is what we expose to the JavaScript,
     21        instead of the JSWorkerGlobalScope itself. As a result, toThis() now
     22        behaves as expected and self.hasOwnProperty() works inside workers.
     23
     24        This patch greatly improves our pass rate on several W3C tests:
     25        http://w3c-test.org/workers/interfaces.worker: 20 passes -> 50 passes (out of 128)
     26        http://w3c-test.org/IndexedDB/interfaces.worker 0 passes -> 145 passes (out of 156)
     27
     28        Tests: fast/workers/self-hasOwnProperty.html
     29               fast/workers/self-toString.html
     30
     31        * bindings/js/JSWorkerGlobalScopeBase.cpp:
     32        (WebCore::JSWorkerGlobalScopeBase::finishCreation):
     33        (WebCore::JSWorkerGlobalScopeBase::visitChildren):
     34        (WebCore::toJS):
     35        * bindings/js/JSWorkerGlobalScopeBase.h:
     36        (WebCore::JSWorkerGlobalScopeBase::proxy):
     37        * bindings/js/WorkerScriptController.cpp:
     38        (WebCore::WorkerScriptController::initScript):
     39        * bindings/scripts/CodeGeneratorJS.pm:
     40        (GenerateHeader):
     41        (GenerateImplementation):
     42
    1432016-06-08  Antti Koivisto  <antti@apple.com>
    244

  • trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp

    r201253 r201808  
    3636#include "Language.h"
    3737#include "WorkerGlobalScope.h"
     38#include <runtime/JSCInlines.h>
    3839#include <runtime/JSCJSValueInlines.h>
    3940#include <runtime/Microtask.h>
    … …  
    5354}
    5455
    55 void JSWorkerGlobalScopeBase::finishCreation(VM& vm)
     56void JSWorkerGlobalScopeBase::finishCreation(VM& vm, JSProxy* proxy)
    5657{
    57     Base::finishCreation(vm);
     58    m_proxy.set(vm, this, proxy);
     59
     60    Base::finishCreation(vm, m_proxy.get());
    5861    ASSERT(inherits(info()));
     62}
     63
     64void JSWorkerGlobalScopeBase::visitChildren(JSCell* cell, SlotVisitor& visitor)
     65{
     66    JSWorkerGlobalScopeBase* thisObject = jsCast<JSWorkerGlobalScopeBase*>(cell);
     67    ASSERT_GC_OBJECT_INHERITS(thisObject, info());
     68    Base::visitChildren(thisObject, visitor);
     69    visitor.append(&thisObject->m_proxy);
    5970}
    6071
    … …  
    112123    JSWorkerGlobalScope* contextWrapper = script->workerGlobalScopeWrapper();
    113124    ASSERT(contextWrapper);
    114     return contextWrapper;
     125    return contextWrapper->proxy();
    115126}
    116127

  • trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.h

    r201253 r201808  
    4444
    4545        WorkerGlobalScope& wrapped() const { return *m_wrapped; }
     46        JSC::JSProxy* proxy() const { ASSERT(m_proxy); return m_proxy.get(); }
    4647        ScriptExecutionContext* scriptExecutionContext() const;
    4748
    … …  
    6263    protected:
    6364        JSWorkerGlobalScopeBase(JSC::VM&, JSC::Structure*, RefPtr<WorkerGlobalScope>&&);
    64         void finishCreation(JSC::VM&);
     65        void finishCreation(JSC::VM&, JSC::JSProxy*);
     66
     67        static void visitChildren(JSC::JSCell*, JSC::SlotVisitor&);
    6568
    6669    private:
    6770        RefPtr<WorkerGlobalScope> m_wrapped;
     71        JSC::WriteBarrier<JSC::JSProxy> m_proxy;
    6872    };
    6973

  • trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp

    r197648 r201808  
    8989        Strong<JSDedicatedWorkerGlobalScopePrototype> dedicatedContextPrototype(*m_vm, JSDedicatedWorkerGlobalScopePrototype::create(*m_vm, 0, dedicatedContextPrototypeStructure));
    9090        Structure* structure = JSDedicatedWorkerGlobalScope::createStructure(*m_vm, 0, dedicatedContextPrototype.get());
    91 
    92         m_workerGlobalScopeWrapper.set(*m_vm, JSDedicatedWorkerGlobalScope::create(*m_vm, structure, static_cast<DedicatedWorkerGlobalScope&>(*m_workerGlobalScope)));
     91        auto* proxyStructure = JSProxy::createStructure(*m_vm, nullptr, jsNull(), PureForwardingProxyType);
     92        auto* proxy = JSProxy::create(*m_vm, proxyStructure);
     93
     94        m_workerGlobalScopeWrapper.set(*m_vm, JSDedicatedWorkerGlobalScope::create(*m_vm, structure, static_cast<DedicatedWorkerGlobalScope&>(*m_workerGlobalScope), proxy));
    9395        workerGlobalScopePrototypeStructure->setGlobalObject(*m_vm, m_workerGlobalScopeWrapper.get());
    9496        dedicatedContextPrototypeStructure->setGlobalObject(*m_vm, m_workerGlobalScopeWrapper.get());
    … …  
    98100        workerGlobalScopePrototype->structure()->setPrototypeWithoutTransition(*m_vm, JSEventTarget::prototype(*m_vm, m_workerGlobalScopeWrapper.get()));
    99101        dedicatedContextPrototype->structure()->setGlobalObject(*m_vm, m_workerGlobalScopeWrapper.get());
     102
     103        proxy->setTarget(*m_vm, m_workerGlobalScopeWrapper.get());
     104        proxy->structure()->setGlobalObject(*m_vm, m_workerGlobalScopeWrapper.get());
    100105    }
    101106    ASSERT(m_workerGlobalScopeWrapper->globalObject() == m_workerGlobalScopeWrapper);

  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

    r201757 r201808  
    11021102        push(@headerContent, "    }\n\n");
    11031103    } elsif ($codeGenerator->InheritsInterface($interface, "WorkerGlobalScope")) {
    1104         push(@headerContent, "    static $className* create(JSC::VM& vm, JSC::Structure* structure, Ref<$implType>&& impl)\n");
     1104        push(@headerContent, "    static $className* create(JSC::VM& vm, JSC::Structure* structure, Ref<$implType>&& impl, JSC::JSProxy* proxy)\n");
    11051105        push(@headerContent, "    {\n");
    11061106        push(@headerContent, "        $className* ptr = new (NotNull, JSC::allocateCell<$className>(vm.heap)) ${className}(vm, structure, WTFMove(impl));\n");
    1107         push(@headerContent, "        ptr->finishCreation(vm);\n");
     1107        push(@headerContent, "        ptr->finishCreation(vm, proxy);\n");
    11081108        push(@headerContent, "        vm.heap.addFinalizer(ptr, destroy);\n");
    11091109        push(@headerContent, "        return ptr;\n");
    … …  
    14171417            push(@headerContent, "    void finishCreation(JSC::VM&, JSDOMWindowShell*);\n");
    14181418        } else {
    1419             push(@headerContent, "    void finishCreation(JSC::VM&);\n");
     1419            push(@headerContent, "    void finishCreation(JSC::VM&, JSC::JSProxy*);\n");
    14201420        }
    14211421    }
    … …  
    23602360            push(@implContent, "    Base::finishCreation(vm, shell);\n\n");
    23612361        } else {
    2362             push(@implContent, "void ${className}::finishCreation(VM& vm)\n");
     2362            push(@implContent, "void ${className}::finishCreation(VM& vm, JSProxy* proxy)\n");
    23632363            push(@implContent, "{\n");
    2364             push(@implContent, "    Base::finishCreation(vm);\n\n");
     2364            push(@implContent, "    Base::finishCreation(vm, proxy);\n\n");
    23652365        }
    23662366        # Support for RuntimeEnabled attributes on global objects.
Note: See TracChangeset for help on using the changeset viewer.