<rdar://problem/21892877>

Created attachment 259790 [details] Proposed Fix

Comment on attachment 259790 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=259790&action=review Nice! > LayoutTests/inspector/protocol/backend-dispatcher-argument-errors.html:71 > + name : "RequiredParameterWrongType", > + description: "The backend should return an error if a message has an optional parameter with wrong type.", The name here should be "OptionalParameterWrongType". > LayoutTests/inspector/protocol/backend-dispatcher-malformed-message-errors.html:17 > + let errorCodes = { > + ParseError: -32700, > + InvalidRequest: -32600, > + MethodNotFound: -32601, > + InvalidParams: -32602, > + InternalError: -32603, > + ServerError: -32000, > + }; Should InspectorBackend expose these? Currently it only checks for -32000 but it could expose these constants. Then we wouldn't need to duplicate it in these tests. > LayoutTests/inspector/protocol/backend-dispatcher-malformed-message-errors.html:39 > + }) Style: semicolon? I'm normally very lax about commenting on style of tests, but seeing a they are getting rather clean / uniform now. > LayoutTests/inspector/protocol/backend-dispatcher-malformed-message-errors.html:46 > + name : "UnparseableStringMessage", Style: "name :" => "name:" (for all of these in both tests) > LayoutTests/inspector/protocol/backend-dispatcher-malformed-message-errors.html:141 > + addErrorResponseTestCase({ > + name : "MethodFieldWithBadFormatting5", > + description: "The backend should return an error if a message has a 'method' field not formatted as 'Domain.Methodname'.", > + message: {id: 123, method: ".FooBar."}, > + expectedError: "InvalidRequest" > + }); Another similar one would be Foo.bar.baz. Both covered by the same condition here, but might be good to test separately.

Comment on attachment 259790 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=259790&action=review >> LayoutTests/inspector/protocol/backend-dispatcher-malformed-message-errors.html:17 >> + }; > > Should InspectorBackend expose these? Currently it only checks for -32000 but it could expose these constants. Then we wouldn't need to duplicate it in these tests. Meh, they are part of the JSON-RPC specification. Besides, this test doesn't cover InspetorBackend.js, which should be a separate test (focused on correct resolve/reject/callback args). >> LayoutTests/inspector/protocol/backend-dispatcher-malformed-message-errors.html:39 >> + }) > > Style: semicolon? I'm normally very lax about commenting on style of tests, but seeing a they are getting rather clean / uniform now. It's a promise chain.

Committed r188897: <http://trac.webkit.org/changeset/188897>