Summary: | Web Inspector: migrate Inspector protocol messages format to JSON-RPC | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ilya Tikhonovsky <loislo> | ||||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Ilya Tikhonovsky <loislo> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
URL: | http://groups.google.com/group/json-rpc/web/json-rpc-2-0 | ||||||||||||||||
Attachments: |
|
Created attachment 88450 [details]
protocol messages sample
Created attachment 88452 [details]
InspectorBackendDispatcher.cpp sample
Created attachment 88453 [details]
InspectorBackendStub.js
Created attachment 88455 [details]
[patch] initial version
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.
Created attachment 88457 [details]
[patch] initial version with fixed ChangeLogs
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. Comment on attachment 88457 [details]
[patch] initial version with fixed ChangeLogs
Note that this needs coordinated landing.
(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 Attachment 88457 [details] did not build on mac: Build output: http://queues.webkit.org/results/8341528 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? (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. Committed r83171: <http://trac.webkit.org/changeset/83171> |
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.