Bug 194719 – Add MSE logging configuration

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

[Eric Carlson]

Add UI to configure the MSE runtime logging added in r241148.

[Radar WebKit Bug Importer]

<rdar://problem/48122151>

[Eric Carlson]

Created attachment 362154 [details]
Patch

[EWS Watchlist]

This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)

This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.

[Joseph Pecoraro]

Comment on attachment 362154 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:93
>                  new WI.ScopeBarItem(WI.LogContentView.Scopes.Media, WI.UIString("Media"), {className: "media"}),
> +                new WI.ScopeBarItem(WI.LogContentView.Scopes.Media, WI.UIString("MediaSource"), {className: "mediasource"}),

So we are re-using the "Media" Scope? I think that is fine.

[EWS Watchlist]

Comment on attachment 362154 [details]
Patch

Attachment 362154 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11164365

New failing tests:
inspector/console/webcore-logging.html

[EWS Watchlist]

Created attachment 362163 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 362154 [details]
Patch

Attachment 362154 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11164329

New failing tests:
inspector/console/webcore-logging.html

[EWS Watchlist]

Created attachment 362164 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Eric Carlson]

Created attachment 362173 [details]
Patch for landing

[Devin Rousso]

Comment on attachment 362173 [details]
Patch for landing

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

> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:93
> +                new WI.ScopeBarItem(WI.LogContentView.Scopes.Media, WI.UIString("MediaSource"), {className: "mediasource"}),

Doesn't this mean that if someone wants to search by MediaSource messages, they'd only get Media messages?  Why aren't we using a `WI.LogConentView.Scopes.MediaSource`?

[Eric Carlson]

(In reply to Devin Rousso from comment #10)
> Comment on attachment 362173 [details]
> Patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=362173&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/LogContentView.js:93
> > +                new WI.ScopeBarItem(WI.LogContentView.Scopes.Media, WI.UIString("MediaSource"), {className: "mediasource"}),
> 
> Doesn't this mean that if someone wants to search by MediaSource messages,
> they'd only get Media messages?  Why aren't we using a
> `WI.LogConentView.Scopes.MediaSource`?

Adding a new scope will add another button to the already crowded console, and I don't think people will typically want MSE without Media. 

We should revisit the logging and console UI soon, we can consider adding something specifically for MSE then.

[Devin Rousso]

Comment on attachment 362173 [details]
Patch for landing

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

>>> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:93
>>> +                new WI.ScopeBarItem(WI.LogContentView.Scopes.Media, WI.UIString("MediaSource"), {className: "mediasource"}),
>> 
>> Doesn't this mean that if someone wants to search by MediaSource messages, they'd only get Media messages?  Why aren't we using a `WI.LogConentView.Scopes.MediaSource`?
> 
> Adding a new scope will add another button to the already crowded console, and I don't think people will typically want MSE without Media. 
> 
> We should revisit the logging and console UI soon, we can consider adding something specifically for MSE then.

This code is adding another scope bar item though.  I'm questioning whether the id for the item should be `WI.LogContentView.Scopes.MediaSource`, as otherwise the filtering won't work.  I think we should be adding a `WI.LogContentView.Scopes.MediaSource` type so that someone can filter MediaSource messages.  As you have it now, the Media and MediaSource buttons will have the _same_ effect, which I don't think is what you want (if it is, then please add a comment as to why).

    new WI.ScopeBarItem(WI.LogContentView.Scopes.MediaSource, WI.UIString("MediaSource"), {className: "mediasource"}),

Media-related console messages are filtered by the source of the message.  MediaSource messages will have the source of `WI.ConsoleMessage.MessageSource.MediaSource`, which is exactly what you'd want to filter on (you'd need to add a case for it inside `_scopeFromMessageSource`).

[Eric Carlson]

Comment on attachment 362173 [details]
Patch for landing

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

>>>> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:93
>>>> +                new WI.ScopeBarItem(WI.LogContentView.Scopes.Media, WI.UIString("MediaSource"), {className: "mediasource"}),
>>> 
>>> Doesn't this mean that if someone wants to search by MediaSource messages, they'd only get Media messages?  Why aren't we using a `WI.LogConentView.Scopes.MediaSource`?
>> 
>> Adding a new scope will add another button to the already crowded console, and I don't think people will typically want MSE without Media. 
>> 
>> We should revisit the logging and console UI soon, we can consider adding something specifically for MSE then.
> 
> This code is adding another scope bar item though.  I'm questioning whether the id for the item should be `WI.LogContentView.Scopes.MediaSource`, as otherwise the filtering won't work.  I think we should be adding a `WI.LogContentView.Scopes.MediaSource` type so that someone can filter MediaSource messages.  As you have it now, the Media and MediaSource buttons will have the _same_ effect, which I don't think is what you want (if it is, then please add a comment as to why).
> 
>     new WI.ScopeBarItem(WI.LogContentView.Scopes.MediaSource, WI.UIString("MediaSource"), {className: "mediasource"}),
> 
> Media-related console messages are filtered by the source of the message.  MediaSource messages will have the source of `WI.ConsoleMessage.MessageSource.MediaSource`, which is exactly what you'd want to filter on (you'd need to add a case for it inside `_scopeFromMessageSource`).

Fair enough.

[Eric Carlson]

Created attachment 362299 [details]
Updated pPatch for landing.

[WebKit Commit Bot]

Comment on attachment 362299 [details]
Updated pPatch for landing.

Clearing flags on attachment: 362299

Committed r241729: <https://trac.webkit.org/changeset/241729>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.