[webkit-reviews] review granted: [Bug 91406] MutationCallback should be a WebIDL 'callback', not a [Callback] interface : [Attachment 192081] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Adam Klein <adamk at chromium.org>'s
request for review:
Bug 91406: MutationCallback should be a WebIDL 'callback', not a [Callback]
interface
https://bugs.webkit.org/show_bug.cgi?id=91406

Attachment 192081: Patch
https://bugs.webkit.org/attachment.cgi?id=192081&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
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.  :)