Bug 191785 – Web Inspector: Include default filtering of InspectorBackend.dumpInspectorProtocolMessages with multi target backend

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

Include default filtering of InspectorBackend.dumpInspectorProtocolMessages with multi target backend

Dumping all messages will include everything twice:

  1. The agent message per-target
  2. The multi backend message Target agent message containing the above message

Since the multi backend right now really is just a multiplexer, lets just filter its messages by default.

[Joseph Pecoraro]

Created attachment 355146 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 355147 [details]
[IMAGE] All Button States

[Joseph Pecoraro]

Created attachment 355148 [details]
[IMAGE] Before - overwhelming logs

[Joseph Pecoraro]

Created attachment 355149 [details]
[IMAGE] After - typical logs

[Matt Baker]

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

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

r=me

> Source/WebInspectorUI/UserInterface/Debug/Bootstrap.js:45
> +    const dumpMessagesToolTip = WI.unlocalizedString("Enable dump inspector messages to console.\nShift-click to dump inspector all messages with no filtering.");

"inspector all messages" -> "all inspector messages"

> Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:109
> +    }

Style: we usually put the getter above the setter for read/write properties.

> Source/WebInspectorUI/UserInterface/Protocol/LoggingProtocolTracer.js:68
> +    }

Ditto.

[Joseph Pecoraro]

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

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

>> Source/WebInspectorUI/UserInterface/Debug/Bootstrap.js:45
>> +    const dumpMessagesToolTip = WI.unlocalizedString("Enable dump inspector messages to console.\nShift-click to dump inspector all messages with no filtering.");
> 
> "inspector all messages" -> "all inspector messages"

oops!

>> Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:109
>> +    }
> 
> Style: we usually put the getter above the setter for read/write properties.

Yeah, these two files are oddly different and I didn't want a bunch of red in the patch =(

[Joseph Pecoraro]

Created attachment 355160 [details]
[PATCH] For Bots

[Devin Rousso]

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

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

> Source/WebInspectorUI/UserInterface/Debug/Bootstrap.js:44
> +    const DumpMessagesState = {Off: "off", Filtering: "filtering", Everything: "everything"};

Style: this should be on separate lines

> Source/WebInspectorUI/UserInterface/Debug/Bootstrap.js:66
> +        case DumpMessagesState.Filtering:

Rather than "Filtering", what about "Filtered" to keep the same word type as "Off" and "Everything"?

> Source/WebInspectorUI/UserInterface/Debug/Bootstrap.js:76
> +            dumpMessagesToolbarItem.element.style.color = "rgb(164, 41, 154)";

I prefer using the `setProperty` function, as it makes it somewhat easier to search for CSS properties since they aren't mangled (e.g. "background-image" becomes "style.backgroundImage")

> Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:108
> +        return WI.settings.filterMultiplexingBackendInspectorProtocolMessages.value;

Should we bother asserting that `this._defaultTracer.filterMultiplexingBackend` has the same value as the setting?

[Joseph Pecoraro]

https://trac.webkit.org/r238331

[Radar WebKit Bug Importer]

<rdar://problem/46144912>