Bug 57957 – Web Inspector: migrate Inspector protocol messages format to JSON-RPC

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

[Ilya Tikhonovsky]

Created attachment 88449 [details]
errors sample

There is not a significant difference between  inspector messages spec and and JSON-RPC 2.0 messages spec.
Also JSON-RPC has a pretty clear specification for error descriptions which we haven't.
It was decided that we will use it.

the list of renames:

1) type-> /dev/null
2) domain + '.' + event => method // for events
3) domain + '.' + command => method // for requests
4) requestId => id // for responses

5) arguments => params // for requests
6) data => params // for events 
7) body => result // for responses

protocolErrors and error properties will be converted to JSON-RPC error format.
The order of properties in messages also will be adjusted.
The only thing that looks unnecessary is jsonrpc property.

[Ilya Tikhonovsky]

Created attachment 88450 [details]
protocol messages sample

[Ilya Tikhonovsky]

Created attachment 88452 [details]
InspectorBackendDispatcher.cpp sample

[Ilya Tikhonovsky]

Created attachment 88453 [details]
InspectorBackendStub.js

[Ilya Tikhonovsky]

Created attachment 88455 [details]
[patch] initial version

[WebKit Review Bot]

Attachment 88455 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style']" exit_code: 1

LayoutTests/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/WebCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ilya Tikhonovsky]

Created attachment 88457 [details]
[patch] initial version with fixed ChangeLogs

[Yury Semikhatsky]

Comment on attachment 88457 [details]
[patch] initial version with fixed ChangeLogs

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

> LayoutTests/inspector/report-protocol-errors-expected.txt:37
> +        data : {

Shouldn't the data be an array instead of object with properties 0, 1, 2,... ?

> Source/WebCore/inspector/CodeGeneratorInspector.pm:508
> +        commonErrors.insert(ParseError, "{\\\"code\\\":-32700,\\\"message\\\":\\\"Parse error.\\\"}");

Would be nice to have a link to the spec defining all these constants.

[Pavel Feldman]

Comment on attachment 88457 [details]
[patch] initial version with fixed ChangeLogs

Note that this needs coordinated landing.

[Ilya Tikhonovsky]

(In reply to comment #7)
> (From update of attachment 88457 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=88457&action=review
> 
> > LayoutTests/inspector/report-protocol-errors-expected.txt:37
> > +        data : {
> 
> Shouldn't the data be an array instead of object with properties 0, 1, 2,... ?

Really it is the array but there is a bug in InspectorTest.addObject. 
As far as such fix will affect a number of expectations it'd be better do do that in another patch.

> 
> > Source/WebCore/inspector/CodeGeneratorInspector.pm:508
> > +        commonErrors.insert(ParseError, "{\\\"code\\\":-32700,\\\"message\\\":\\\"Parse error.\\\"}");
> 
> Would be nice to have a link to the spec defining all these constants.

all these codes are described in the the spec. 
You can see the url to it in URL property of the bug. 

http://groups.google.com/group/json-rpc/web/json-rpc-2-0

[WebKit Review Bot]

Attachment 88457 [details] did not build on mac:
Build output: http://queues.webkit.org/results/8341528

[Yury Semikhatsky]

Comment on attachment 88457 [details]
[patch] initial version with fixed ChangeLogs

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

> Source/WebCore/inspector/CodeGeneratorInspector.pm:966
> +    push(@backendHead, "        ParseError = 0,");

Why not use same constant values as defined in the spec?

[Yury Semikhatsky]

(In reply to comment #9)
> (In reply to comment #7)
> > (From update of attachment 88457 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=88457&action=review
> > 
> > > LayoutTests/inspector/report-protocol-errors-expected.txt:37
> > > +        data : {
> > 
> > Shouldn't the data be an array instead of object with properties 0, 1, 2,... ?
> 
> Really it is the array but there is a bug in InspectorTest.addObject. 
> As far as such fix will affect a number of expectations it'd be better do do that in another patch.
> 
Please file a bug on this.

> > 
> > > Source/WebCore/inspector/CodeGeneratorInspector.pm:508
> > > +        commonErrors.insert(ParseError, "{\\\"code\\\":-32700,\\\"message\\\":\\\"Parse error.\\\"}");
> > 
> > Would be nice to have a link to the spec defining all these constants.
> 
> all these codes are described in the the spec. 
> You can see the url to it in URL property of the bug. 
> 
> http://groups.google.com/group/json-rpc/web/json-rpc-2-0
I meant putting the URL into code so that it's easier to find the spec when you are reading the code.

[Ilya Tikhonovsky]

Committed r83171: <http://trac.webkit.org/changeset/83171>