As reported by vsevik@ (thank you), the change causes several warnings. Source/WebCore/inspector/InjectedScriptSourceTmp.js:600: WARNING - Bad type annotation. Unknown type integer * @param {integer} scopeNumber ^ Source/WebCore/inspector/InjectedScriptSourceTmp.js:618: WARNING - Property setFunctionVariableValue never defined on InjectedScriptHost setter = InjectedScriptHost.setFunctionVariableValue.bind(InjectedScriptHost, func); ^ Source/WebCore/inspector/front-end/protocol-externs.js:2085: WARNING - optional arguments must be at the end DebuggerAgent.setVariableValue = function(opt_callFrameId, opt_functionObjectId, scopeNumber, variableName, newValue, opt_callback) {}
Created attachment 187676 [details] Patch
(In reply to comment #1) > Created an attachment (id=187676) [details] > Patch vsevik@, could you please take a look at the patch. Note that in order to satisfy requirement that optional parameters go last, I tried to reorder parameters in Inspector.json, those ones that originally were first, but recently became optional. I'm not sure though, that we can afford reordering parameters.
Comment on attachment 187676 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187676&action=review > Source/WebCore/inspector/Inspector.json:2936 > + { "name": "newValue", "$ref": "Runtime.CallArgument", "description": "New variable value." }, Can you provide a test for this command?
Comment on attachment 187676 [details] Patch The patch introduces new warnings: Compiling InjectedScriptSource.js... Source/WebCore/inspector/InjectedScriptSourceTmp.js:609: WARNING - actual parameter 2 of InjectedScript.prototype._callFrameForId does not match formal parameter found : (boolean|string) required: string var callFrame = this._callFrameForId(topCallFrame, callFrameId); ^ Source/WebCore/inspector/InjectedScriptSourceTmp.js:614: WARNING - actual parameter 1 of InjectedScript.prototype._parseObjectId does not match formal parameter found : (boolean|string) required: string var parsedFunctionId = this._parseObjectId(functionObjectId); ^ 0 error(s), 2 warning(s), 89.8% typed
Comment on attachment 187676 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187676&action=review >> Source/WebCore/inspector/Inspector.json:2936 >> + { "name": "newValue", "$ref": "Runtime.CallArgument", "description": "New variable value." }, > > Can you provide a test for this command? I thought LayoutTests/inspector-protocol/debugger-setVariableValue.html was that test.
(In reply to comment #5) > (From update of attachment 187676 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=187676&action=review > > >> Source/WebCore/inspector/Inspector.json:2936 > >> + { "name": "newValue", "$ref": "Runtime.CallArgument", "description": "New variable value." }, > > > > Can you provide a test for this command? > > I thought LayoutTests/inspector-protocol/debugger-setVariableValue.html was that test. Oh, I see. I meant that it would be nice if we had a test that would require changes in accord with your concern https://bugs.webkit.org/show_bug.cgi?id=109488#c2
(In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 187676 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=187676&action=review > > > > >> Source/WebCore/inspector/Inspector.json:2936 > > >> + { "name": "newValue", "$ref": "Runtime.CallArgument", "description": "New variable value." }, > > > > > > Can you provide a test for this command? > > > > I thought LayoutTests/inspector-protocol/debugger-setVariableValue.html was that test. > > Oh, I see. I meant that it would be nice if we had a test that would require changes in accord with your concern https://bugs.webkit.org/show_bug.cgi?id=109488#c2 That's right. But so far we don't any client that depends on parameter order. I don't think we should prepare such client only to prove that parameter order in fact changed. However, I wonder if we should change parameter order. For example Java project bindings depends on order and theoretically such changes might go unnoticed unless caught by parameter type checks.
Created attachment 187882 [details] Patch
Comment on attachment 187882 [details] Patch Clearing flags on attachment: 187882 Committed r142888: <http://trac.webkit.org/changeset/142888>
All reviewed patches have been landed. Closing bug.