Bug 142599 – Web Inspector: Split ConsoleMessageImpl into the View and the Model

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

This issue is for refactoring only, it has no UI changes.


1. Merge ConsoleMessage and ConsoleMessageImpl.

WebInspector.ConsoleMessageImpl inherits from WebInspector.ConsoleMessage. However, WebInspector.ConsoleMessageImpl gets instantiated only by WebInspector.ConsoleMessage.create, which makes the two tightly coupled and defeats the purpose of inheritance.

ConsoleMessage isn’t used by anything other than ConsoleMessageImpl, so there is no point of keeping them both – they should be merged.


2. Split ConsoleMessage(Impl) into the View and the Model.

Model will hold the data and some logic but no DOM operations, all DOM operations will go to the view.

I believe, it should be possible to have more than one view for a single model, although, I don’t yet know a use case for that.

[Radar WebKit Bug Importer]

<rdar://problem/20128435>

[Radar WebKit Bug Importer]

<rdar://problem/20128453>

[Nikita Vasilyev]

Created attachment 248596 [details]
WIP

Not quite finished, but it works and has no bugs what I know of.

[Joseph Pecoraro]

Comment on attachment 248596 [details]
WIP

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

> Source/WebInspectorUI/UserInterface/Main.html:230
> +    <script src="Models/ConsoleMessage.js"></script>

This file is missing... I can't really review the patch.

[Joseph Pecoraro]

Comment on attachment 248596 [details]
WIP

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

> Source/WebInspectorUI/UserInterface/Controllers/LogManager.js:51
> +        var consoleMessage = new WebInspector.ConsoleMessage(source, level, text, null, type, url, line, column, repeatCount, parameters, stackTrace, null);

There is this mysterious "null" property between "text" and "type". What is it? Can it just be removed? Otherwise, it should move to the end.

> Source/WebInspectorUI/UserInterface/Views/ConsoleCommandResult.js:39
> +    var model = new WebInspector.ConsoleMessage(

Style: Spread out over multiple lines like this makes it hard to read.

> Source/WebInspectorUI/UserInterface/Views/ConsoleCommandResult.js:43
> +        null,

Here it is again.

> Source/WebInspectorUI/UserInterface/Views/ConsoleCommandResult.js:84
> +WebInspector.ConsoleCommandResult.prototype.__proto__ = WebInspector.ConsoleMessageView.prototype;

We should switch to new class style:

    Foo.prototype = {
        constructor: Foo,
        __proto__ = WebInspector.ConsoleMessageView.prototype,
        ...
    };

> Source/WebInspectorUI/UserInterface/Views/LegacyConsoleMessageImpl.js:734
> +    clone: function()

Same here, these should be the new method syntax.

[Nikita Vasilyev]

Created attachment 248603 [details]
WIP

Added missing Models/ConsoleMessage.js.

[Nikita Vasilyev]

(In reply to comment #5)
> Comment on attachment 248596 [details]
> WIP
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248596&action=review
> 
> > Source/WebInspectorUI/UserInterface/Controllers/LogManager.js:51
> > +        var consoleMessage = new WebInspector.ConsoleMessage(source, level, text, null, type, url, line, column, repeatCount, parameters, stackTrace, null);
> 
> There is this mysterious "null" property between "text" and "type". What is
> it? Can it just be removed? Otherwise, it should move to the end.

It’s linkifier copied over from http://trac.webkit.org/browser/trunk/Source/WebInspectorUI/UserInterface/Views/ConsoleMessage.js#L64. I agree that it should be moved to the end.

[Nikita Vasilyev]

Overall, does this address the initial goal of the rewrite, e.g. does it make testing easier (possible)? I’m not familiar with the Console tests.

[Joseph Pecoraro]

Comment on attachment 248603 [details]
WIP

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

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:31
> +
> +

Style: Extra newline here, should just be 1.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:32
> +WebInspector.ConsoleMessage = function(source, level, message, linkifier, type, url, line, column, repeatCount, parameters, stackTrace, request) {

Style: Brace should be on its own line.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:36
> +    this._linkifier = linkifier || null; // Unused.

Unused? Lets get rid of it.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:49
> +};

Style: There are a lot of style issues here. This should follow the new class guidelines.
http://trac.webkit.org/wiki/WebInspectorCodingStyleGuide#Newclassskeleton

Affecting this file:

1. This should subclass WebInspector.Object, and so should call WebInspector.Object in the constructor.
2. Member variables should be private with "_"s.
3. Prototype should have "constructor" and "__proto__" properties.
4. Prototype should have "Public" and "Private" section.
5. We should start adopting the new method syntax for object literals ("foo()" instead of "foo: function()").

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:148
> +WebInspector.ConsoleMessage.create = function(source, level, message, type, url, line, column, repeatCount, parameters, stackTrace, request)
> +{
> +    console.warn("ConsoleMessage#create is deprecated, please don't use it.");
> +    return new WebInspector.ConsoleMessageImpl(source, level, message, null, type, url, line, column, repeatCount, parameters, stackTrace, request);
> +};

Instead of issuing a warning, this patch should go through and actually replace callers of "create" with constructor calls.

I'm going to stop the review here. I'm confused by what this patch is supposed to do.

Since this still has "LegacyConsoleMessage" and "LegacyConsoleMessageImpl" it sounds like this should be multiple patches that can be clear distinct changes. Easy to review, and test on your end.

  1. One patch that renames "ConsoleMessage" and "ConsoleMessageImpl" to "LegacyConsoleMessage" and "LegacyConsoleMessage". Both the files and the JavaScript classes and call sites.
    => clean code, clearly using the old classes
  2. One patch that adds the new "Models/ConsoleMessage" and "Views/ConsoleMessageView" classes. New, clean, versions.
    => clean code, clearly introducing new clean classes following our existing code guidelines and patterns for Models and Views
  3. One patch that moves existing code over from LegacyConsole* to the new non-Legacy Console* classes. This can remove the Legacy* classes at the same time.
    => that will mean removing ALL the old code, and moving everything to the new code, and since you renamed all the Old code to "Legacy" this should be pretty easy!

Also, please include ChangeLogs for the patch with at least a brief description of the changes happening.

[Nikita Vasilyev]

(In reply to comment #9)
> Style: There are a lot of style issues here. This should follow the new
> class guidelines.
> http://trac.webkit.org/wiki/WebInspectorCodingStyleGuide#Newclassskeleton
> 
> Affecting this file:
> 
> 1. This should subclass WebInspector.Object, and so should call
> WebInspector.Object in the constructor.
> 2. Member variables should be private with "_"s.

Should I add a getter and a setter for each one of them like so:

    get source()
    {
        return this._source;
    },

    set source(value)
    {
        this._source = value;
    }

?

[Timothy Hatcher]

(In reply to comment #10)
> (In reply to comment #9)
> > Style: There are a lot of style issues here. This should follow the new
> > class guidelines.
> > http://trac.webkit.org/wiki/WebInspectorCodingStyleGuide#Newclassskeleton
> > 
> > Affecting this file:
> > 
> > 1. This should subclass WebInspector.Object, and so should call
> > WebInspector.Object in the constructor.
> > 2. Member variables should be private with "_"s.
> 
> Should I add a getter and a setter for each one of them like so:
> 
>     get source()
>     {
>         return this._source;
>     },
> 
>     set source(value)
>     {
>         this._source = value;
>     }
> 
> ?

Anything that needs to be public and used by another file.

[Nikita Vasilyev]

Created attachment 249071 [details]
Patch

Note that this patch doesn't:
 - touch WebInspector.IssueMessage yet
 - use ECMAScript class syntax

[Joseph Pecoraro]

Comment on attachment 249071 [details]
Patch

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

This is much better! I stopped the review just shy of the bulk of DOM creation because I know that is just copied code. I'd like to see another version cleaning up things some more.

> Source/WebInspectorUI/UserInterface/Controllers/LogManager.js:-51
> -        var consoleMessage = WebInspector.LegacyConsoleMessage.create(source, level, text, type, url, line, column, repeatCount, parameters, stackTrace, null);

Sounds like you remove all uses of LegacyConsole*. So we can (and should) remove the files with this change.

> Source/WebInspectorUI/UserInterface/Controllers/LogManager.js:51
> +        var consoleMessage = new WebInspector.ConsoleMessage(source, level, text, type, url, line, column, repeatCount, parameters, stackTrace, null);

It worries me a bit that we are passing the raw "stackTrace" object into ConsoleMessage. We should probably create a Model class for a stack trace. It is used in multiple places. But for now, this seems okay.

> Source/WebInspectorUI/UserInterface/Controllers/LogManager.js:52
> +        var consoleMessageView = new WebInspector.ConsoleMessageView(consoleMessage);

Lets drop this View creation inside the Controller. Ideally Controllers will not know about views at all. Just model objects.

> Source/WebInspectorUI/UserInterface/Controllers/LogManager.js:54
> +        this.dispatchEventToListeners(WebInspector.LogManager.Event.MessageAdded, {message: consoleMessageView});

This should send the event with the Model object. Listeners can create views for it if they want.

> Source/WebInspectorUI/UserInterface/Controllers/LogManager.js:56
> +        console.assert(!consoleMessageView._element || !consoleMessageView._element.parentNode, "This console message shouldn't be added to a view. To add it you need to use clone().");

This shouldn't be needed anymore. Instead of cloning ConsoleMessage (the old style) all we should need to do now is create a new ConsoleMessageView(model). So this assert no longer makes sense.

> Source/WebInspectorUI/UserInterface/Main.html:298
>      <script src="Views/LegacyConsoleMessage.js"></script>

Remove these old classes.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:3
> + * Copyright (C) 2007, 2008, 2013 Apple Inc.  All rights reserved.

Incude 2015.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:31
> +WebInspector.ConsoleMessage = function(source, level, message, type, url, line, column, repeatCount, parameters, stackTrace, request)

We should convert to classes. I realize that happened after this patch went up.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:43
> +    this._repeatCount = repeatCount;

I think this should be || 0. It is optional in the protocol.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:44
> +    this._totalRepeatCount = repeatCount;

Doesn't look like this ever changes. So we should just remove it.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:95
> +    get repeatCount()
> +    {
> +        return this._repeatCount;
> +    },

It looks like there is code that tries to update this:

            this._previousMessage.repeatCount = count - (this._previousMessage.ignoredCount || 0);
            this._previousMessage.updateRepeatCount();

So it sounds like there should be a setter somewhere. And if so, probably an Event to get dispatched from the model to update a view.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:100
> +    get totalRepeatCount()
> +    {
> +        return this._totalRepeatCount;
> +    },

Unused.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:117
> +    isErrorOrWarning: function()

Style: use new method syntax: "isErrorOrWarning()" here and in all methods below.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:119
> +        return (this.level === WebInspector.ConsoleMessage.MessageLevel.Warning || this.level === WebInspector.ConsoleMessage.MessageLevel.Error);

Style: Parenthesis not needed.

We may want to call this "isIssue".

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:137
> +    isEqual: function(msg)

Style: we try not to abbreviate, lets name this "other".

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:155
> +            var l = this._stackTrace;
> +            var r = msg._stackTrace;
> +            for (var i = 0; i < l.length; i++) {
> +                if (l[i].url !== r[i].url ||
> +                l[i].functionName !== r[i].functionName ||
> +                l[i].lineNumber !== r[i].lineNumber ||
> +                l[i].columnNumber !== r[i].columnNumber)
> +                    return false;

Style: Indentation looks wrong.

If we create a StackTrace model object, this would be easier. (StackTrace.prototype.isEqual)

We may want to just add a deepEqual function, that assumes non-cyclic objects passed into it.

That said, this doesn't check if "l.length" differs from "r.length" so this could be wrong (in an extremely unlikely case).

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:165
> +        return (this.source === msg.source)
> +            && (this.type === msg.type)
> +            && (this.level === msg.level)
> +            && (this.line === msg.line)
> +            && (this.url === msg.url)
> +            && (this.message === msg.message)
> +            && (this._request === msg._request);

Style: these parenthesis are unnecessary.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:171
> +    clone: function()
> +    {
> +        return new WebInspector.ConsoleMessage(this.source, this.level, this._messageText, this.type, this.url, this.line, this.column, this.repeatCount, this._parameters, this._stackTrace, this._request);
> +    },

We should remove this, in favor of "new WebInspector.ConsoleMessageView" in JavaScriptLogViewController.js.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:175
> +        switch (this.model.level) {

This should be "this._level".

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:176
> +            case WebInspector.ConsoleMessage.MessageLevel.Tip:

Style: "case" should line up with "switch".

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:189
> +    shouldDumpStackTrace: function()

This sounds like it should be a ConsoleMessageView decision. The model just is. The View determines how to show it.

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:191
> +        return !!this._stackTrace && this._stackTrace.length && (this.source === WebInspector.ConsoleMessage.MessageSource.Network || this.level === WebInspector.ConsoleMessage.MessageLevel.Error || this.type === WebInspector.ConsoleMessage.MessageType.Trace);

Style: This would be much better written with early returns:

    if (!this.stackTrace || !this.stackTrace.length)
        return false;

    if (this.source === ....)
        return true;

    return false;

> Source/WebInspectorUI/UserInterface/Views/ConsoleCommandResult.js:30
>  WebInspector.ConsoleCommandResult = function(result, wasThrown, originatingCommand, savedResultIndex)

Seems this should be a Model object that is a ConsoleMessage subclass.

All View logic can just check "message instanceof ConsoleCommandResult" to know whether to treat it like a command result or not. Much like how we deal with WebInspector.SourceCode <-> WebInspector.Resource.

> Source/WebInspectorUI/UserInterface/Views/ConsoleCommandResult.js:33
>      this.originatingCommand = originatingCommand;

Originating command doesn't appear to be used anywhere. Lets remove it.

> Source/WebInspectorUI/UserInterface/Views/ConsoleCommandResult.js:34
>      this.savedResultIndex = savedResultIndex;

This should be "_savedResultIndex" with a getter below.

> Source/WebInspectorUI/UserInterface/Views/ConsoleCommandResult.js:39
> +    var model = new WebInspector.ConsoleMessage(WebInspector.ConsoleMessage.MessageSource.JS, level, "", WebInspector.ConsoleMessage.MessageType.Result, undefined, undefined, undefined, undefined, [result]);

These undefined look gross. Maybe we should re-order the parameters of the constructor to put the more likely to be used ones earlier.

> Source/WebInspectorUI/UserInterface/Views/ConsoleCommandResult.js:41
> +    WebInspector.ConsoleMessageView.call(this, model);

Then this could just be removed.

> Source/WebInspectorUI/UserInterface/Views/ConsoleCommandResult.js:63
>      toMessageElement: function()
>      {
> -        var element = WebInspector.LegacyConsoleMessageImpl.prototype.toMessageElement.call(this);
> +        var element = WebInspector.ConsoleMessageView.prototype.toMessageElement.call(this);
>          element.classList.add("console-user-command-result");
>          if (!element.getAttribute("data-labelprefix"))
>              element.setAttribute("data-labelprefix", WebInspector.UIString("Output: "));

So this could should move inside ConsoleMessageView.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:3
> + * Copyright (C) 2007, 2008, 2013 Apple Inc.  All rights reserved.

2015.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:35
> +    this.model = model;

this._model. And of course update uses within the class.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:55
> +    enforcesClipboardPrefixString: true,

When moving to classes, this will need to be made a getter, since properties are not allowed on classes.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:62
> +    // Some of these getters match the ones on WebInspector.IssueMessage.
> +    // Remove them once we remove WebInspector.IssueMessage.
> +    get source()
> +    {
> +        return this.model.source;
> +    },

Ideally we should be able to remove these. Anyone who uses the view to access model state (ConsoleMessageView.source) should instead have been using (ConsoleMessage.source). This cleanup can be done later.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:97
> +    get totalRepeatCount()
> +    {
> +        return this.model.totalRepeatCount;
> +    },

Unused.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:109
> +    decorateMessageElement: function(element)
> +    {
> +        if (this._element)
> +            return this._element;

Does this early bail make sense? Given an element it returns a completely different element?

Seems this is only used in one place. Just inline it into toMessageElement.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:162
> +        if (!this.repeatCountElement) {

this._repeatCountElement, it is private.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:164
> +            this.repeatCountElement.className = "bubble";

A better class name would be "repeat-count".

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:183
> +            clipboardString = this._formattedMessage.querySelector("span").innerText;

We shouldn't need to resort to querySelector. We are the view, we created the elements. This should just be something like, "this._fooElement.innerText".

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:223
> +    clone: function(isDeep)
> +    {
> +        var newModel = isDeep ? this.model.clone() : this.model;
> +        return new WebInspector.ConsoleMessageView(newModel);
> +    },

Remove clone.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:243
> +    get _messageText()
> +    {
> +        return this.model.messageText;
> +    },
> +
> +    set _messageText(value)
> +    {
> +        this.model.messageText = value;
> +    },

I don't think you need any of these _ getters / setters... If anyone is using them, something is wrong.

[Joseph Pecoraro]

I'm taking this.

[Joseph Pecoraro]

Created attachment 250391 [details]
[PATCH] Proposed Fix

[Timothy Hatcher]

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

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

Sweet!

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:35
> +        this._source = source;
> +        this._level = level;

|| WebInspector.Foo ?

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:36
> +        this._messageText = message;

|| "" ?

> Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:45
> +        this._parameters = parameters;
> +        this._stackTrace = stackTrace;
> +        this._request = request;

|| []?
|| []?
|| null ?

> Source/WebInspectorUI/UserInterface/Views/ConsoleCommandView.jsSource/WebInspectorUI/UserInterface/Views/ConsoleCommand.js:47
> +        // FIXME: LogContentView should use higher level objects.
> +        this._element.__commandView = this;

Should use Symbol here.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:46
> +        this._element.__message = this._message;
> +        this._element.__messageView = this;

Symbol?

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:349
> +        };
> +        var formatter = formatters[type] || this._formatParameterAsValue;

Nit: Newline.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:470
> +                }
> +                var span = document.createElement("span");

Nit: Newline.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:526
> +    }
> +
> +
> +    _userProvidedColumnNames(columnNamesArgument)

Nit: Double newline.

> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:296
> +            var messageView = messageElement.__messageView || messageElement.__commandView;

Symbol would be used here.

> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:675
> +            var visible = showsAll || messageElement.__commandView instanceof WebInspector.ConsoleCommandView || messageElement.__message instanceof WebInspector.ConsoleCommandResultMessage;

And here.

> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:677
> +                switch(messageElement.__message.level) {

And here.

[Joseph Pecoraro]

(In reply to comment #16)
> Comment on attachment 250391 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=250391&action=review
> 
> Sweet!
> 
> > Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:35
> > +        this._source = source;
> > +        this._level = level;
> 
> || WebInspector.Foo ?

These should actually be required and be asserted.

> > Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:36
> > +        this._messageText = message;
> 
> || "" ?
> 
> > Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js:45
> > +        this._parameters = parameters;
> > +        this._stackTrace = stackTrace;
> > +        this._request = request;
> 
> || []?
> || []?
> || null ?

I prefer undefined over an empty array. I don't know if there is any benefit to undefined/null. I know we have been doing null in the past though. The one benefit I see to undefined is in JSON printing it'll just not show up in JSON.stringify which I've been using in tests.

Source/WebInspectorUI/UserInterface/Views/ConsoleCommandView.jsSource/WebInspectorUI/UserInterface/Views/ConsoleCommand.js:47
> > +        // FIXME: LogContentView should use higher level objects.
> > +        this._element.__commandView = this;
> 
> Should use Symbol here.
> 
> > Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:46
> > +        this._element.__message = this._message;
> > +        this._element.__messageView = this;
> 
> Symbol?

I think ultimately these should just be eliminated, so I'm avoiding cleaning up using Symbols.

For example I made the "IgnoredRepeatCount" a symbol since that was a legit case being cleaned up. I didn't want to clean this up in this patch if we will just remove them. I made these double underscore so they can be easily searchable when this gets fixed up later and when we move to a clean syntax / new approach it'll be clear.

[Joseph Pecoraro]

http://trac.webkit.org/changeset/182579

[Brent Fulgham]

The new 'LayoutTests/inspector/console/console-api.html' test is failing (timing out) on Windows. :-(

[Joseph Pecoraro]

Test fix in: http://trac.webkit.org/changeset/182583

[Joseph Pecoraro]

(In reply to comment #19)
> The new 'LayoutTests/inspector/console/console-api.html' test is failing
> (timing out) on Windows. :-(

Err my test fix was for something else, but... if my theory about inter-locked inspector test failures turns out to be true, it might have been the cause of this.

My theory right now is that when another inspector test fails, it causes a lock on something that causes other inspector tests to time-out. If true, that would explain why random tests timeout.

[Alexey Proskuryakov]

This test is very slow on Mac too, timing out in debug build half of the time:

https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Fconsole%2Fconsole-api.html