Per the DOM4 spec, http://www.w3.org/TR/dom/#mutationcallback, MutationCallback should be a 'callback'. The main differences between a callback and a [Callback] interface: It must be a function; an object with a 'handleEvent' property that's a function is not acceptable (and should throw a TypeError). See http://www.w3.org/TR/WebIDL/#es-callback-function. This form of callback is not currently supported by CodeGenerator.pm, but it seems like it would be a good thing to add.
Created attachment 191872 [details] WIP
Comment on attachment 191872 [details] WIP Attachment 191872 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17059101
Comment on attachment 191872 [details] WIP Attachment 191872 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17086110
Created attachment 191877 [details] WIP
Comment on attachment 191877 [details] WIP Attachment 191877 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17023063
Comment on attachment 191877 [details] WIP Attachment 191877 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17070047
Comment on attachment 191877 [details] WIP Attachment 191877 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17070048
Created attachment 191879 [details] WIP: try to fix JSC build
Comment on attachment 191879 [details] WIP: try to fix JSC build Attachment 191879 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17070053
Comment on attachment 191879 [details] WIP: try to fix JSC build Attachment 191879 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17010044
Comment on attachment 191879 [details] WIP: try to fix JSC build Attachment 191879 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17010049
Comment on attachment 191879 [details] WIP: try to fix JSC build Attachment 191879 [details] did not pass win-ews (win): Output: http://webkit-commit-queue.appspot.com/results/17050270
Comment on attachment 191879 [details] WIP: try to fix JSC build Attachment 191879 [details] did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/17023098
Comment on attachment 191879 [details] WIP: try to fix JSC build Attachment 191879 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17064304
Created attachment 192081 [details] Patch
Now works on JSC as well as V8, ready for a look.
Comment on attachment 192081 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192081&action=review > Source/WebCore/bindings/js/JSMutationCallback.cpp:69 > + ASSERT_NOT_REACHED(); > + return; Do we need to handle this case if it can't be reached? > Source/WebCore/bindings/js/JSMutationCallback.cpp:95 > + globalObject->globalData().timeoutChecker.start(); > + InspectorInstrumentationCookie cookie = JSMainThreadExecState::instrumentFunctionCall(context, callType, callData); > + > + JSMainThreadExecState::call(exec, callback, callType, callData, jsObserver, args); > + > + InspectorInstrumentation::didCallFunction(cookie); > + globalObject->globalData().timeoutChecker.stop(); > + > + if (exec->hadException()) > + reportCurrentException(exec); Should we wrap this in a helper function so that we don't need to copy/paste this code for every callback? > Source/WebCore/bindings/v8/V8MutationCallback.cpp:78 > + ScriptController::callFunctionWithInstrumentation(scriptExecutionContext(), callback, thisObject, 2, argv); ScriptController::callFunctionWithInstrumentation <--- Looks like we have the wrapper function for V8 already. :)
+ggaren for JSC questions
Comment on attachment 192081 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192081&action=review >> Source/WebCore/bindings/js/JSMutationCallback.cpp:69 >> + return; > > Do we need to handle this case if it can't be reached? I don't know enough about JSC API to answer that question well. It seems like I might be able to avoid this by storing a JSC::JSFunction* instead of a JSC::JSObject*, but it doesn't seem like that's at all common in JSC code. Basically, I think it's a good idea to handle this case for now, since it's trying to enforce an invariant from another class (in this case, the constructor of JSMutationObserver in JSMutationObserverCustom.cpp). >> Source/WebCore/bindings/js/JSMutationCallback.cpp:95 >> + reportCurrentException(exec); > > Should we wrap this in a helper function so that we don't need to copy/paste this code for every callback? Seems like that'd be a good question for some JSC folks, I'm not sure they want yet-another method on JSMainThreadExecState.
Comment on attachment 192081 [details] Patch Neither of the outstanding questions affect the correctness of this code, so I'm sending this to the cq. Those questions can be dealt with as a post-submit cleanup as necessary.
Comment on attachment 192081 [details] Patch Clearing flags on attachment: 192081 Committed r145379: <http://trac.webkit.org/changeset/145379>
All reviewed patches have been landed. Closing bug.