Bug 42345 – Web Inspector: pass all parameters to WebInspector.addConsoleMessage as a single payload object

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

[Yury Semikhatsky]

Web Inspector: pass all parameters to WebInspector.addConsoleMessage as a single payload object. This would simplify parameter addition/removal.

[Yury Semikhatsky]

Created attachment 61621 [details]
Patch

[Pavel Feldman]

Comment on attachment 61621 [details]
Patch

I'd suggest that we remove ScriptArray as a whole instead. Ilya is almost there.

[Yury Semikhatsky]

(In reply to comment #2)
> (From update of attachment 61621 [details])
> I'd suggest that we remove ScriptArray as a whole instead. Ilya is almost there.

At the present time we cannot serialize console parameters into InspectorObject's, because there is no way to get InspectorObject from JavaScript object. I think it should be addressed in a separate change .

[Pavel Feldman]

Comment on attachment 61621 [details]
Patch

WebCore/inspector/ConsoleMessage.cpp: 
 +      frontend->addConsoleMessage(jsonObj, m_frames, arguments,  m_message);
m_frames, arguments are missing.

[Yury Semikhatsky]

Created attachment 61630 [details]
Patch

[Yury Semikhatsky]

(In reply to comment #4)
> (From update of attachment 61621 [details])
> WebCore/inspector/ConsoleMessage.cpp: 
>  +      frontend->addConsoleMessage(jsonObj, m_frames, arguments,  m_message);
> m_frames, arguments are missing.

Fixed.

[Pavel Feldman]

Comment on attachment 61630 [details]
Patch

WebCore/inspector/front-end/ConsoleView.js:278
 +              msgCopy = new WebInspector.ConsoleMessage(msg.source, msg.type, msg.level, msg.line, msg.url, msg.groupLevel, count, msg.messageText, msg.parameters, msg.stackTrace);
I think you affect the repeatCount and repeatDelta properties.

WebCore/inspector/front-end/ConsoleView.js:658
 +      this.parameters = parameters;
These should be private.



WebCore/inspector/front-end/ConsoleView.js:676
 +                  var stack = this.stackTrace.slice();
No need.

[Yury Semikhatsky]

Created attachment 61646 [details]
Patch

[Yury Semikhatsky]

(In reply to comment #7)
> (From update of attachment 61630 [details])
> WebCore/inspector/front-end/ConsoleView.js:278
>  +              msgCopy = new WebInspector.ConsoleMessage(msg.source, msg.type, msg.level, msg.line, msg.url, msg.groupLevel, count, msg.messageText, msg.parameters, msg.stackTrace);
> I think you affect the repeatCount and repeatDelta properties.
>
Fixed.
 
> WebCore/inspector/front-end/ConsoleView.js:658
>  +      this.parameters = parameters;
> These should be private.
> 
Done.

> 
> 
> WebCore/inspector/front-end/ConsoleView.js:676
>  +                  var stack = this.stackTrace.slice();
> No need.
Done.

[Pavel Feldman]

Comment on attachment 61646 [details]
Patch

I think it is too risky to rename everything. Could you name only new members as private?

[Yury Semikhatsky]

Created attachment 61664 [details]
Patch

[Pavel Feldman]

Comment on attachment 61664 [details]
Patch

WebCore/inspector/front-end/ConsoleView.js:685
 +                  this.formattedMessage = this._format(["%O", this._parameters[0]]);
I am a bit concerned with the potential switch from <non-undefined>[0] to <potentially-undefined>[0].

[Yury Semikhatsky]

(In reply to comment #12)
> (From update of attachment 61664 [details])
> WebCore/inspector/front-end/ConsoleView.js:685
>  +                  this.formattedMessage = this._format(["%O", this._parameters[0]]);
> I am a bit concerned with the potential switch from <non-undefined>[0] to <potentially-undefined>[0].

Added explicit check.

[Yury Semikhatsky]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/bindings/js/ScriptArray.cpp
	M	WebCore/bindings/js/ScriptArray.h
	M	WebCore/bindings/v8/ScriptArray.cpp
	M	WebCore/bindings/v8/ScriptArray.h
	M	WebCore/inspector/ConsoleMessage.cpp
	M	WebCore/inspector/InspectorFrontend.cpp
	M	WebCore/inspector/InspectorFrontend.h
	M	WebCore/inspector/front-end/ConsoleView.js
	M	WebCore/inspector/front-end/ElementsPanel.js
	M	WebCore/inspector/front-end/InjectedScriptAccess.js
	M	WebCore/inspector/front-end/Resource.js
	M	WebCore/inspector/front-end/inspector.js
Committed r63427