Web Inspector: control whether to collect and dump protocol messages using a WebInspector.Setting

[BJ Burg]

The first step towards inspector^2 tooling for analyzing protocol messages. See the meta bug for some big picture ideas.

[Radar WebKit Bug Importer]

<rdar://problem/23679143>

[BJ Burg]

Created attachment 266212 [details]
Proposed Fix

May not apply yet, due to being on top of 5 patches. I don't think it depends on them outright, though.

[BJ Burg]

Created attachment 266213 [details]
Proposed Fix

[Timothy Hatcher]

Comment on attachment 266213 [details]
Proposed Fix

Wrong patch.

[BJ Burg]

Created attachment 266228 [details]
Proposed Fix

[Timothy Hatcher]

Comment on attachment 266228 [details]
Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:117
> +        if (this.activeTracer)
> +            this.activeTracer.logStarted();
> +        // If the custom tracer was removed and automatic tracing is enabled,
> +        // then create a new automatic tracer and install it in its place.
> +        else

This style is confusing. The comment should move under the else and add {}.

[BJ Burg]

Created attachment 267043 [details]
For Landing

[WebKit Commit Bot]

Comment on attachment 267043 [details]
For Landing

Clearing flags on attachment: 267043

Committed r193870: <http://trac.webkit.org/changeset/193870>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Comment on attachment 267043 [details]
For Landing

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

> Source/WebInspectorUI/UserInterface/Protocol/LoggingProtocolTracer.js:112
> +    logWillHandleEvent(message)
> +    {
> +        console.assert(typeof message === "string", "Must stringify messages to avoid leaking all JSON protocol messages.")
> +
> +        let entry = {type: "event", message};
> +        this._processEntry(entry);
> +    }
> +
> +    logDidHandleEvent(message, timings = null)
> +    {
> +        console.assert(typeof message === "string", "Must stringify messages to avoid leaking all JSON protocol messages.")
> +
> +        let entry = {type: "event", message};
> +        if (timings)
> +            entry.timings = Object.shallowCopy(timings);
> +
> +        this._processEntry(entry);
> +    }

This is processing the entry (logging to console) on both willHandle and didHandle. This means when dumping to the console its happening twice for each event (and response). What is the intended behavior here?

[BJ Burg]

Comment on attachment 267043 [details]
For Landing

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

>> Source/WebInspectorUI/UserInterface/Protocol/LoggingProtocolTracer.js:112
>> +    }
> 
> This is processing the entry (logging to console) on both willHandle and didHandle. This means when dumping to the console its happening twice for each event (and response). What is the intended behavior here?

The prior behavior was to log twice only if time-stats are enabled. Now it always logs twice. We could change the outer else if to be (this._dumpMessagesToConsole && !entry.timings).

[BJ Burg]

Reopening to attach new patch.

[BJ Burg]

Created attachment 267585 [details]
Follow-up fix

[Joseph Pecoraro]

Comment on attachment 267585 [details]
Follow-up fix

r=me

[WebKit Commit Bot]

Comment on attachment 267585 [details]
Follow-up fix

Clearing flags on attachment: 267585

Committed r194244: <http://trac.webkit.org/changeset/194244>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.