Bug 67506 – Web Inspector: [Extensions API] expose console API

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Andrey Kosyakov]

This adds an API module to fetch and add console messages.

[Andrey Kosyakov]

Created attachment 106157 [details]
patch

[Pavel Feldman]

Comment on attachment 106157 [details]
patch

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

Lets first agree on what is the console message for formatted console records and evaluation results. Otherwise looks good.

> LayoutTests/inspector/extensions/extensions-api-expected.txt:86
> +        MessageLevel : {

Severity ?

> Source/WebCore/inspector/front-end/ConsoleMessage.js:449
> +        return this._messageText;

Real text is formatted asynchronously, what text do you expect to return for console.log("Foo %s", document) ?

> Source/WebCore/inspector/front-end/ConsoleMessage.js:454
> +        return this._parameters;

Parameters are object wrappers that are not exposed to the user.

[Andrey Kosyakov]

Created attachment 106326 [details]
patch

[Andrey Kosyakov]

(In reply to comment #2)
> (From update of attachment 106157 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=106157&action=review
> 
> Lets first agree on what is the console message for formatted console records and evaluation results. Otherwise looks good.
> 
> > LayoutTests/inspector/extensions/extensions-api-expected.txt:86
> > +        MessageLevel : {
> 
> Severity ?

Renamed to MessageSeverity (although we use level internally)

> 
> > Source/WebCore/inspector/front-end/ConsoleMessage.js:449
> > +        return this._messageText;
> 
> Real text is formatted asynchronously, what text do you expect to return for console.log("Foo %s", document) ?
> 
> > Source/WebCore/inspector/front-end/ConsoleMessage.js:454
> > +        return this._parameters;
> 
> Parameters are object wrappers that are not exposed to the user.

As discussed offline, dropped parameters altogether for the first version.

[Pavel Feldman]

Comment on attachment 106326 [details]
patch

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

> LayoutTests/inspector/extensions/extensions-api-expected.txt:86
> +        MessageSeverity : {

I'd do simply Severity. It is in either case under console.

> Source/WebCore/inspector/front-end/ExtensionServer.js:-429
> -        // The networkManager is normally created after the ExtensionServer is constructed, but before initExtensions() is called.

This is going against the performance effort we are currently focusing on. Specifically, this will do something upon every console message no matter whether there are extensions interested in this type of event. Probably need to resolve it for all the notifications below. r- for that.

[Andrey Kosyakov]

Created attachment 106572 [details]
patch

- subscribe to front-end events only when an extension subscribes to a related event
- MessageSeverity -> Severity

[Pavel Feldman]

Comment on attachment 106572 [details]
patch

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

> LayoutTests/http/tests/inspector/resources/extension-main.js:24
>          else if (typeof propValue === "object")

typeof === "object" && propValue !== null will make undefined and null fall to under latest else.

[Balazs Kelemen]

The new test fails on Qt:
-extensions-console.html:50console.trace()logextensions-console.html:50 console-message console-log-level
+extensions-console.html:50console.trace()logextensions-console.html:50onloadextensions-console.html:57 console-message console-log-level

Do you think it's normal to have platform specific results for the test? If yes, we can simply update the qt expectation.

[Andrey Kosyakov]

(In reply to comment #8)
> The new test fails on Qt:
> -extensions-console.html:50console.trace()logextensions-console.html:50 console-message console-log-level
> +extensions-console.html:50console.trace()logextensions-console.html:50onloadextensions-console.html:57 console-message console-log-level
> 
> Do you think it's normal to have platform specific results for the test? If yes, we can simply update the qt expectation.

I think I'd rather drop trace() from the test -- it doesn't add that much value, so we can sacrifice it to avoid custom expectations. I'll take care of this, just waiting few more minutes for runs on other platforms to complete, to see if there may be other surprises.

[Andrey Kosyakov]

Manually committed r94666: http://trac.webkit.org/changeset/94666