To kill V8Proxy, we can move instrumentedCallFunction() from V8Proxy to ScriptController. Also we can rename instrumentedCallFunction() to callFunctionWithInstrumentation(), for consistency with callFunction().
Created attachment 159372 [details] Patch
Comment on attachment 159372 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159372&action=review > Source/WebCore/bindings/v8/ScriptController.cpp:226 > +#if PLATFORM(CHROMIUM) Are these ifdefs needed? I would have expected them to be inside TRACE_EVENT1, but I haven't actually checked. > Source/WebCore/bindings/v8/ScriptController.h:81 > + static v8::Local<v8::Value> callFunctionWithInstrumentation(Frame*, v8::Handle<v8::Function>, v8::Handle<v8::Object> receiver, int argc, v8::Handle<v8::Value> args[]); Why make this a static function that takes a Frame* as an argument? I guess so it can be null? This function might make more sense as a member function. > Source/WebCore/bindings/v8/ScriptFunctionCall.cpp:205 > - v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(0 /* frame */, function, object, m_arguments.size(), args.get()); > + v8::Handle<v8::Value> result = ScriptController::callFunctionWithInstrumentation(0 /* frame */, function, object, m_arguments.size(), args.get()); I see. Frame can be 0... > Source/WebCore/bindings/v8/V8Callback.cpp:63 > Frame* frame = scriptExecutionContext && scriptExecutionContext->isDocument() ? static_cast<Document*>(scriptExecutionContext)->frame() : 0; > - v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(frame, callbackFunction, thisObject, argc, argv); > + v8::Handle<v8::Value> result = ScriptController::callFunctionWithInstrumentation(frame, callbackFunction, thisObject, argc, argv); Maybe we should try passing a ScriptExecutionContext directly? This code that goes from ScriptExecutionContext to Frame just so that callFunctionWithInstrumentation can go back to ScriptExecutionContext seems less than ideal.
Comment on attachment 159372 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159372&action=review >> Source/WebCore/bindings/v8/ScriptController.cpp:226 >> +#if PLATFORM(CHROMIUM) > > Are these ifdefs needed? I would have expected them to be inside TRACE_EVENT1, but I haven't actually checked. Right. Removed. >> Source/WebCore/bindings/v8/V8Callback.cpp:63 >> + v8::Handle<v8::Value> result = ScriptController::callFunctionWithInstrumentation(frame, callbackFunction, thisObject, argc, argv); > > Maybe we should try passing a ScriptExecutionContext directly? This code that goes from ScriptExecutionContext to Frame just so that callFunctionWithInstrumentation can go back to ScriptExecutionContext seems less than ideal. Done.
Committed r126109: <http://trac.webkit.org/changeset/126109>