Bug 191478 – Web Inspector: Include target identifier in protocol logging

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

Include styled target identifier in protocol logging

With multi-target support, the number of messages doubles as everything gets routed again through a Target agent message. I'll have a few tweaks to the logging tracer to make it behave as expected again but this is a reasonable change to make regardless. It includes the target identifier in the tracing, so its much easier to follow (and filter).

[Joseph Pecoraro]

Created attachment 354363 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 354364 [details]
[IMAGE] Colored (not what the patch has)

[Joseph Pecoraro]

Created attachment 354365 [details]
[IMAGE] After Patch

[Devin Rousso]

Comment on attachment 354363 [details]
[PATCH] Proposed Fix

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

r-, as discussed offline, you didn't include any of the changes to the call-sites of these functions

> Source/WebInspectorUI/ChangeLog:13
> +        (WI.LoggingProtocolTracer.prototype.set filterMultiplexingBackend):
> +        (WI.LoggingProtocolTracer.prototype.get filterMultiplexingBackend):

These aren't part of this patch.

> Source/WebInspectorUI/UserInterface/Protocol/LoggingProtocolTracer.js:112
> +            let connection = entry.connection;

NIT: how about destructuring these variables inside the parameters list?

> Source/WebInspectorUI/UserInterface/Protocol/LoggingProtocolTracer.js:114
> +            this._logToConsole(`${entry.type} (${targetId}): ${JSON.stringify(entry.message)}`);

How about adding colorization when we are showing all targets?  We'd do the default behavior of no color when only showing a single connection.

[Joseph Pecoraro]

> > Source/WebInspectorUI/UserInterface/Protocol/LoggingProtocolTracer.js:114
> > +            this._logToConsole(`${entry.type} (${targetId}): ${JSON.stringify(entry.message)}`);
> 
> How about adding colorization when we are showing all targets?  We'd do the
> default behavior of no color when only showing a single connection.

Yeah that is when it becomes more useful. I'll look into it then.

[Joseph Pecoraro]

Created attachment 354368 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Removing the word "styled" from the title, since I dropped that for now.

[Joseph Pecoraro]

Created attachment 354370 [details]
[PATCH] Proposed Fix

[Devin Rousso]

Comment on attachment 354370 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Debug/CapturingProtocolTracer.js:42
> +    logFrontendException(connection, message, exception)

Do we not want to save the connection with this as well?

[Joseph Pecoraro]

Comment on attachment 354370 [details]
[PATCH] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Debug/CapturingProtocolTracer.js:42
>> +    logFrontendException(connection, message, exception)
> 
> Do we not want to save the connection with this as well?

The `logWillHandleEvent` that happened before this will have had it.

[Devin Rousso]

Comment on attachment 354370 [details]
[PATCH] Proposed Fix

r=me

[Joseph Pecoraro]

https://trac.webkit.org/r238197

[Radar WebKit Bug Importer]

<rdar://problem/46074459>