Changeset 145379 in webkit


Ignore:
Timestamp:
Mar 11, 2013 12:16:01 PM (11 years ago)
Author:
adamk@chromium.org
Message:

MutationCallback should be a WebIDL 'callback', not a [Callback] interface
​https://bugs.webkit.org/show_bug.cgi?id=91406

Reviewed by Adam Barth.

Source/WebCore:

Spec: ​http://dom.spec.whatwg.org/#mutationcallback

Besides no longer calling handleEvent methods on passed-in objects,
throw a TypeError if a non-function is passed to the MutationObserver constructor.
This is per WebIDL: ​http://www.w3.org/TR/WebIDL/#es-callback-function

Updated MutationObserver constructor tests to exercise TypeError-throwing behavior.

  • bindings/js/JSMutationCallback.cpp:

(WebCore::JSMutationCallback::call): Call the callback directly instead of handing off to JSCallbackData; make return value void.
Use jsArray() to convert from WTF::Vector -> JSArray.

  • bindings/js/JSMutationCallback.h:

(JSMutationCallback): Rename handleEvent() to call(), make it void.

  • bindings/js/JSMutationObserverCustom.cpp:

(WebCore::JSMutationObserverConstructor::constructJSMutationObserver): Throw if passed a non-function.

  • bindings/v8/V8MutationCallback.cpp:

(WebCore::V8MutationCallback::V8MutationCallback): Take a v8::Function instead of a v8::Object.
(WebCore::V8MutationCallback::call): Call the callback directly instead of handing off to invokeCallback(); make return value void.
Use v8Array() to convert form WTF::Vector -> JSArray.

  • bindings/v8/V8MutationCallback.h:

(WebCore::V8MutationCallback::create): Take a v8::Function instead of a v8::Object.
(V8MutationCallback): ditto

  • bindings/v8/custom/V8MutationObserverCustom.cpp:

(WebCore::V8MutationObserver::constructorCustom): Throw if passed a non-function, cast to a v8::Function when constructing callback.

  • dom/MutationCallback.h:

(WebCore): Remove unnecessary typedef.
(MutationCallback): Rename handleEvent() to call(), make it void.

  • dom/MutationObserver.cpp:

(WebCore::MutationObserver::deliver): Update MutationCallback method name.

LayoutTests:

  • fast/dom/MutationObserver/mutation-observer-constructor-expected.txt:
  • fast/dom/MutationObserver/mutation-observer-constructor.html: Add test for TypeError-throwing.

Note that the Number and String cases already threw before this patch.

Location:
trunk
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r145378 r145379  
     12013-03-11  Adam Klein  <adamk@chromium.org>
     2
     3        MutationCallback should be a WebIDL 'callback', not a [Callback] interface
     4        https://bugs.webkit.org/show_bug.cgi?id=91406
     5
     6        Reviewed by Adam Barth.
     7
     8        * fast/dom/MutationObserver/mutation-observer-constructor-expected.txt:
     9        * fast/dom/MutationObserver/mutation-observer-constructor.html: Add test for TypeError-throwing.
     10        Note that the Number and String cases already threw before this patch.
     11
    1122013-03-11  Julien Chaffraix  <jchaffraix@webkit.org>
    213

  • trunk/LayoutTests/fast/dom/MutationObserver/mutation-observer-constructor-expected.txt

    r137662 r145379  
    99PASS typeof observer.observe is "function"
    1010PASS typeof observer.disconnect is "function"
     11PASS new MutationObserver({ handleEvent: function() {} }) threw exception TypeError: Callback argument must be a function.
     12PASS new MutationObserver({}) threw exception TypeError: Callback argument must be a function.
     13PASS new MutationObserver(42) threw exception TypeError: Callback argument must be a function.
     14PASS new MutationObserver("foo") threw exception TypeError: Callback argument must be a function.
    1115PASS successfullyParsed is true
    1216

  • trunk/LayoutTests/fast/dom/MutationObserver/mutation-observer-constructor.html

    r143386 r145379  
    11<!DOCTYPE html>
    2 <html>
    3 <head>
    4 <meta charset="utf-8">
     2<body>
    53<script src="../../js/resources/js-test-pre.js"></script>
    6 <title></title>
    7 </head>
    8 <body>
    9 <p id=description></p>
    10 <div id="console"></div>
    114<script>
    12 
    13 function runTest() {
    14     shouldBeNonNull('window.WebKitMutationObserver');
    15     shouldBeEqualToString('typeof WebKitMutationObserver.prototype.observe', 'function');
    16     shouldBeEqualToString('typeof WebKitMutationObserver.prototype.disconnect', 'function');
    17     window.observer = new MutationObserver(function(mutations) { });
    18     shouldBeEqualToString('typeof observer.observe', 'function');
    19     shouldBeEqualToString('typeof observer.disconnect', 'function');
    20 }
    21 
    225description('Test the constructor of WebKitMutationObserver');
    236
    24 runTest();
     7shouldBeNonNull('window.WebKitMutationObserver');
     8shouldBeEqualToString('typeof WebKitMutationObserver.prototype.observe', 'function');
     9shouldBeEqualToString('typeof WebKitMutationObserver.prototype.disconnect', 'function');
     10window.observer = new MutationObserver(function(mutations) { });
     11shouldBeEqualToString('typeof observer.observe', 'function');
     12shouldBeEqualToString('typeof observer.disconnect', 'function');
     13
     14shouldThrow('new MutationObserver({ handleEvent: function() {} })');
     15shouldThrow('new MutationObserver({})');
     16shouldThrow('new MutationObserver(42)');
     17shouldThrow('new MutationObserver("foo")');
    2518</script>
    2619<script src="../../js/resources/js-test-post.js"></script>
    2720</body>
    28 </html>

  • trunk/Source/WebCore/ChangeLog

    r145378 r145379  
     12013-03-11  Adam Klein  <adamk@chromium.org>
     2
     3        MutationCallback should be a WebIDL 'callback', not a [Callback] interface
     4        https://bugs.webkit.org/show_bug.cgi?id=91406
     5
     6        Reviewed by Adam Barth.
     7
     8        Spec: http://dom.spec.whatwg.org/#mutationcallback
     9
     10        Besides no longer calling handleEvent methods on passed-in objects,
     11        throw a TypeError if a non-function is passed to the MutationObserver constructor.
     12        This is per WebIDL: http://www.w3.org/TR/WebIDL/#es-callback-function
     13
     14        Updated MutationObserver constructor tests to exercise TypeError-throwing behavior.
     15
     16        * bindings/js/JSMutationCallback.cpp:
     17        (WebCore::JSMutationCallback::call): Call the callback directly instead of handing off to JSCallbackData; make return value void.
     18        Use jsArray() to convert from WTF::Vector -> JSArray.
     19        * bindings/js/JSMutationCallback.h:
     20        (JSMutationCallback): Rename handleEvent() to call(), make it void.
     21        * bindings/js/JSMutationObserverCustom.cpp:
     22        (WebCore::JSMutationObserverConstructor::constructJSMutationObserver): Throw if passed a non-function.
     23        * bindings/v8/V8MutationCallback.cpp:
     24        (WebCore::V8MutationCallback::V8MutationCallback): Take a v8::Function instead of a v8::Object.
     25        (WebCore::V8MutationCallback::call): Call the callback directly instead of handing off to invokeCallback(); make return value void.
     26        Use v8Array() to convert form WTF::Vector -> JSArray.
     27        * bindings/v8/V8MutationCallback.h:
     28        (WebCore::V8MutationCallback::create): Take a v8::Function instead of a v8::Object.
     29        (V8MutationCallback): ditto
     30        * bindings/v8/custom/V8MutationObserverCustom.cpp:
     31        (WebCore::V8MutationObserver::constructorCustom): Throw if passed a non-function, cast to a v8::Function when constructing callback.
     32        * dom/MutationCallback.h:
     33        (WebCore): Remove unnecessary typedef.
     34        (MutationCallback): Rename handleEvent() to call(), make it void.
     35        * dom/MutationObserver.cpp:
     36        (WebCore::MutationObserver::deliver): Update MutationCallback method name.
     37
    1382013-03-11  Julien Chaffraix  <jchaffraix@webkit.org>
    239

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

    r141296 r145379  
    2828#include "JSMutationCallback.h"
    2929
     30#include "JSDOMGlobalObject.h"
     31#include "JSMainThreadExecState.h"
    3032#include "JSMutationObserver.h"
    3133#include "JSMutationRecord.h"
    … …  
    4850}
    4951
    50 bool JSMutationCallback::handleEvent(MutationRecordArray* mutations, MutationObserver* observer)
     52void JSMutationCallback::call(const Vector<RefPtr<MutationRecord> >& mutations, MutationObserver* observer)
    5153{
    5254    if (!canInvokeCallback())
    53         return true;
     55        return;
    5456
    5557    RefPtr<JSMutationCallback> protect(this);
    … …  
    5860
    5961    if (!m_callback)
    60         return false;
     62        return;
     63
     64    JSValue callback = m_callback.get();
     65    CallData callData;
     66    CallType callType = getCallData(callback, callData);
     67    if (callType == CallTypeNone) {
     68        ASSERT_NOT_REACHED();
     69        return;
     70    }
    6171
    6272    ScriptExecutionContext* context = scriptExecutionContext();
    6373    if (!context)
    64         return false;
     74        return;
     75    ASSERT(context->isDocument());
    6576
    6677    JSDOMGlobalObject* globalObject = toJSDOMGlobalObject(context, m_isolatedWorld.get());
    6778    ExecState* exec = globalObject->globalExec();
    6879
    69     MarkedArgumentBuffer mutationList;
    70     for (size_t i = 0; i < mutations->size(); ++i)
    71         mutationList.append(toJS(exec, globalObject, mutations->at(i).get()));
    72 
    7380    JSValue jsObserver = toJS(exec, globalObject, observer);
    7481
    7582    MarkedArgumentBuffer args;
    76     args.append(constructArray(exec, 0, globalObject, mutationList));
     83    args.append(jsArray(exec, globalObject, mutations));
    7784    args.append(jsObserver);
    7885
    79     bool raisedException = false;
    80     // FIXME: Extract invokeCallback() method.
    81     JSCallbackData(m_callback.get(), globalObject).invokeCallback(jsObserver, args, &raisedException);
    82     return !raisedException;
     86    globalObject->globalData().timeoutChecker.start();
     87    InspectorInstrumentationCookie cookie = JSMainThreadExecState::instrumentFunctionCall(context, callType, callData);
     88
     89    JSMainThreadExecState::call(exec, callback, callType, callData, jsObserver, args);
     90
     91    InspectorInstrumentation::didCallFunction(cookie);
     92    globalObject->globalData().timeoutChecker.stop();
     93
     94    if (exec->hadException())
     95        reportCurrentException(exec);
    8396}
    8497

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

    r141296 r145379  
    2828
    2929#include "ActiveDOMCallback.h"
    30 #include "JSCallbackData.h"
     30#include "DOMWrapperWorld.h"
    3131#include "MutationCallback.h"
     32#include <heap/Weak.h>
     33#include <runtime/JSObject.h>
    3234#include <wtf/Forward.h>
    3335
    3436namespace WebCore {
     37
     38class JSDOMGlobalObject;
    3539
    3640class JSMutationCallback : public MutationCallback, public ActiveDOMCallback {
    … …  
    4347    virtual ~JSMutationCallback();
    4448
    45     virtual bool handleEvent(MutationRecordArray* mutations, MutationObserver*) OVERRIDE;
     49    virtual void call(const Vector<RefPtr<MutationRecord> >&, MutationObserver*) OVERRIDE;
    4650
    4751    virtual ScriptExecutionContext* scriptExecutionContext() const OVERRIDE { return ContextDestructionObserver::scriptExecutionContext(); }

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

    r143863 r145379  
    5050
    5151    JSObject* object = exec->argument(0).getObject();
    52     if (!object) {
    53         setDOMException(exec, TYPE_MISMATCH_ERR);
    54         return JSValue::encode(jsUndefined());
    55     }
     52    CallData callData;
     53    if (!object || object->methodTable()->getCallData(object, callData) == CallTypeNone)
     54        return throwVMError(exec, createTypeError(exec, "Callback argument must be a function"));
    5655
    5756    JSMutationObserverConstructor* jsConstructor = jsCast<JSMutationObserverConstructor*>(exec->callee());

  • trunk/Source/WebCore/bindings/v8/V8MutationCallback.cpp

    r144592 r145379  
    2727#include "V8MutationCallback.h"
    2828
     29#include "ScriptController.h"
    2930#include "ScriptExecutionContext.h"
    3031#include "V8Binding.h"
    31 #include "V8Callback.h"
    3232#include "V8MutationObserver.h"
    3333#include "V8MutationRecord.h"
    … …  
    3636namespace WebCore {
    3737
    38 V8MutationCallback::V8MutationCallback(v8::Handle<v8::Object> callback, ScriptExecutionContext* context, v8::Handle<v8::Object> owner, v8::Isolate* isolate)
     38V8MutationCallback::V8MutationCallback(v8::Handle<v8::Function> callback, ScriptExecutionContext* context, v8::Handle<v8::Object> owner, v8::Isolate* isolate)
    3939    : ActiveDOMCallback(context)
    4040    , m_callback(callback)
    … …  
    4545}
    4646
    47 bool V8MutationCallback::handleEvent(MutationRecordArray* mutations, MutationObserver* observer)
     47void V8MutationCallback::call(const Vector<RefPtr<MutationRecord> >& mutations, MutationObserver* observer)
    4848{
    49     ASSERT(mutations);
    50     if (!mutations)
    51         return true;
    52 
    5349    if (!canInvokeCallback())
    54         return true;
     50        return;
    5551
    5652    v8::HandleScope handleScope;
    … …  
    5854    v8::Handle<v8::Context> v8Context = toV8Context(scriptExecutionContext(), m_world.get());
    5955    if (v8Context.IsEmpty())
    60         return true;
     56        return;
    6157
    6258    v8::Context::Scope scope(v8Context);
     59    v8::Isolate* isolate = v8Context->GetIsolate();
    6360
    64     v8::Local<v8::Array> mutationsArray = v8::Array::New(mutations->size());
    65     for (size_t i = 0; i < mutations->size(); ++i)
    66         mutationsArray->Set(v8Integer(i, v8Context->GetIsolate()), toV8(mutations->at(i).get(), v8::Handle<v8::Object>(), v8Context->GetIsolate()));
     61    v8::Handle<v8::Function> callback = v8::Local<v8::Function>::New(isolate, m_callback.get());
    6762
    68     v8::Handle<v8::Value> observerHandle = toV8(observer, v8::Handle<v8::Object>(), v8Context->GetIsolate());
     63    v8::Handle<v8::Value> observerHandle = toV8(observer, v8::Handle<v8::Object>(), isolate);
    6964    if (observerHandle.IsEmpty()) {
    7065        if (!isScriptControllerTerminating())
    7166            CRASH();
    72         return true;
     67        return;
    7368    }
    7469
    7570    if (!observerHandle->IsObject())
    76         return true;
     71        return;
    7772
    78     v8::Handle<v8::Value> argv[] = {
    79         mutationsArray,
    80         observerHandle
    81     };
     73    v8::Handle<v8::Object> thisObject = v8::Handle<v8::Object>::Cast(observerHandle);
     74    v8::Handle<v8::Value> argv[] = { v8Array(mutations, isolate), observerHandle };
    8275
    83     bool callbackReturnValue = false;
    84     return !invokeCallback(m_callback.get(), v8::Handle<v8::Object>::Cast(observerHandle), 2, argv, callbackReturnValue, scriptExecutionContext());
     76    v8::TryCatch exceptionCatcher;
     77    exceptionCatcher.SetVerbose(true);
     78    ScriptController::callFunctionWithInstrumentation(scriptExecutionContext(), callback, thisObject, 2, argv);
    8579}
    8680

  • trunk/Source/WebCore/bindings/v8/V8MutationCallback.h

    r144522 r145379  
    4040class V8MutationCallback : public MutationCallback, public ActiveDOMCallback {
    4141public:
    42     static PassRefPtr<V8MutationCallback> create(v8::Handle<v8::Value> value, ScriptExecutionContext* context, v8::Handle<v8::Object> owner, v8::Isolate* isolate)
     42    static PassRefPtr<V8MutationCallback> create(v8::Handle<v8::Function> callback, ScriptExecutionContext* context, v8::Handle<v8::Object> owner, v8::Isolate* isolate)
    4343    {
    44         ASSERT(value->IsObject());
    4544        ASSERT(context);
    46         return adoptRef(new V8MutationCallback(v8::Handle<v8::Object>::Cast(value), context, owner, isolate));
     45        return adoptRef(new V8MutationCallback(callback, context, owner, isolate));
    4746    }
    4847
    49     virtual bool handleEvent(MutationRecordArray*, MutationObserver*) OVERRIDE;
     48    virtual void call(const Vector<RefPtr<MutationRecord> >&, MutationObserver*) OVERRIDE;
    5049    virtual ScriptExecutionContext* scriptExecutionContext() const OVERRIDE { return ContextDestructionObserver::scriptExecutionContext(); }
    5150
    5251private:
    53     V8MutationCallback(v8::Handle<v8::Object>, ScriptExecutionContext*, v8::Handle<v8::Object>, v8::Isolate*);
     52    V8MutationCallback(v8::Handle<v8::Function>, ScriptExecutionContext*, v8::Handle<v8::Object>, v8::Isolate*);
    5453
    5554    static void weakCallback(v8::Isolate* isolate, v8::Persistent<v8::Value> wrapper, void* parameter)
    … …  
    5958    }
    6059
    61     ScopedPersistent<v8::Object> m_callback;
     60    ScopedPersistent<v8::Function> m_callback;
    6261    RefPtr<DOMWrapperWorld> m_world;
    6362};

  • trunk/Source/WebCore/bindings/v8/custom/V8MutationObserverCustom.cpp

    r143851 r145379  
    4848
    4949    v8::Local<v8::Value> arg = args[0];
    50     if (!arg->IsObject())
    51         return setDOMException(TYPE_MISMATCH_ERR, args.GetIsolate());
     50    if (!arg->IsFunction())
     51        return throwTypeError("Callback argument must be a function", args.GetIsolate());
    5252
    5353    ScriptExecutionContext* context = getScriptExecutionContext();
    5454    v8::Handle<v8::Object> wrapper = args.Holder();
    5555
    56     RefPtr<MutationCallback> callback = V8MutationCallback::create(arg, context, wrapper, args.GetIsolate());
     56    RefPtr<MutationCallback> callback = V8MutationCallback::create(v8::Handle<v8::Function>::Cast(arg), context, wrapper, args.GetIsolate());
    5757    RefPtr<MutationObserver> observer = MutationObserver::create(callback.release());
    5858

  • trunk/Source/WebCore/dom/MutationCallback.h

    r138811 r145379  
    4141class MutationObserver;
    4242
    43 typedef Vector<RefPtr<MutationRecord> > MutationRecordArray;
    44 
    4543class MutationCallback : public RefCounted<MutationCallback> {
    4644public:
    4745    virtual ~MutationCallback() { }
    4846
    49     virtual bool handleEvent(MutationRecordArray*, MutationObserver*) = 0;
     47    virtual void call(const Vector<RefPtr<MutationRecord> >&, MutationObserver*) = 0;
    5048    virtual ScriptExecutionContext* scriptExecutionContext() const = 0;
    5149};

  • trunk/Source/WebCore/dom/MutationObserver.cpp

    r138811 r145379  
    205205    records.swap(m_records);
    206206
    207     m_callback->handleEvent(&records, this);
     207    m_callback->call(records, this);
    208208}
    209209
Note: See TracChangeset for help on using the changeset viewer.