<rdar://problem/30033678>

Created attachment 317450 [details]
Patch

This is an early WIP. Two major issues:

1) InspectorInstrumentationObject its talking to the InspectorDebuggerAgent for the global object's environment.
2) The builtin code calls out to C++ land regardless of Inspector enabled state.

Comment on attachment 317450 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=317450&action=review

I'd suggest writing in a comment what you expect the Debugger Sidebar's CallFrames to be for particular Promise scenarios.

For example this test case:

function doWork() {
    var promise = new Promise((resolve, reject) => {
        setTimeout(function createThen() { promise.then((value) => { debugger; }); });
        setTimeout(function resolvePromise() { resolve(999); });
    });
}

doWork();

> Source/JavaScriptCore/builtins/PromiseOperations.js:234
> +    @InspectorInstrumentation.didInitializePromise(this, executor);

I don't think you want to include `executor` here. It is also unused in the C++. `executor` is just the `new Promise(X)` X function required to access the resolve/reject resolving functions.

> Source/JavaScriptCore/runtime/InspectorInstrumentationObject.cpp:110
> +    JSValue promise = exec->argument(0);

At this point, given you know it should be a

> Source/JavaScriptCore/runtime/InspectorInstrumentationObject.cpp:114
> +    Inspector::JSGlobalObjectInspectorController& controller = exec->lexicalGlobalObject()->inspectorController();

I don't think you want to require REMOTE_INSPECTOR and go through the GlobalObject's InspectorController (which is the JSContext Inspector's Debugger Agent and not WebCore's Debugger Agent).

I think you should go through JSGlobalObject's JSC::Debugger like so:

    auto globalObject = exec->lexicalGlobalObject();
    auto debugger = globalObject->debugger();
    if (!debugger || !debugger->isInteractivelyDebugging())
        return;

    debugger->initializedPromise(promise);

The JSC::Debugger can then send messages out to DebuggerAgents through the ScriptDebugListener interface, and the right DebuggerAgent will be able to handle it.

This also early returns if breakpoints are not enabled (which I think is reasonable).

Comment on attachment 317450 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=317450&action=review

>> Source/JavaScriptCore/runtime/InspectorInstrumentationObject.cpp:110
>> +    JSValue promise = exec->argument(0);
> 
> At this point, given you know it should be a

I meant to say since you know it should be a JSPromise I think you should jsCast<JSPromise> at this point and pass on the nicely typed JSPromise and not an opaque JSValue.