Not sure exactly how to reproduce yet. Was typing: 1. js> entry.contentView.| When Uncaught Exception: ---------------------- null is not an object (evaluating 'Object.keys(propertyNames)') (at JavaScriptRuntimeCompletionProvider.js:244:57) keys @ [native code] receivedObjectPropertyNames @ JavaScriptRuntimeCompletionProvider.js:244:57 receivedObjectPropertyNames @ [native code] mycallback @ RemoteObject.js:516:21 _dispatchResponseToCallback @ Connection.js:148:27 _dispatchResponse @ Connection.js:118:45 dispatch @ Connection.js:70:35 dispatchMessageFromTarget @ TargetManager.js:143:35 dispatchMessageFromTarget @ TargetObserver.js:42:51 dispatchEvent @ InspectorBackend.js:340:42 _dispatchEvent @ Connection.js:195:32 dispatch @ Connection.js:72:32 dispatch @ InspectorBackend.js:178:52 ? @ MessageDispatcher.js:42:34 ----------------------
This seems like a race when typing very fast. Calls to: RuntimeAgent.releaseObjectGroup("completion") From a prior evaluation will wipe out an ongoing request. --- Typing: `entry.contentView` quickly in the quick console when paused: [Log] request: { "id": 675, "method": "Debugger.evaluateOnCallFrame", "params": { "callFrameId": "{\"ordinal\":2,\"injectedScriptId\":1}", "expression": "\n//# sourceURL=__WebInspectorInternal__\nentry", "objectGroup": "completion", "includeCommandLineAPI": true, "doNotPauseOnExceptionsAndMuteConsole": true, "returnByValue": false, "generatePreview": false, "saveResult": false, "emulateUserGesture": false } } [Log] response: { "result": { "result": { "type": "object", "objectId": "{\"injectedScriptId\":1,\"id\":4003}", "className": "BackForwardEntry", "description": "BackForwardEntry" } }, "id": 675 [Log] request: { "id": 701, "method": "Runtime.callFunctionOn", "params": { "objectId": "{\"injectedScriptId\":1,\"id\":4003}", "functionDeclaration": "\n//# sourceURL=__WebInspectorInternal__\nfunction inspectedPage_evalResult_getCompletions(primitiveType)\n {\n var object;\n if (primitiveType === \"string\")\n object = new String(\"\");\n else if (primitiveType === \"number\")\n object = new Number(0);\n else if (primitiveType === \"boolean\")\n object = new Boolean(false);\n else if (primitiveType === \"symbol\")\n object = Symbol();\n else\n object = this;\n\n var resultSet = {};\n for (var o = object; o; o = o.__proto__) {\n try {\n var names = Object.getOwnPropertyNames(o);\n for (var i = 0; i < names.length; ++i)\n resultSet[names[i]] = true;\n } catch (e) { }\n }\n\n return resultSet;\n }", "doNotPauseOnExceptionsAndMuteConsole": true, "returnByValue": true } } ... [Log] request: { "id": 703, "method": "Runtime.releaseObjectGroup", "params": { "objectGroup": "completion" } } ... [Log] response: { "error": { "code": -32000, "message": "Could not find object with given id", "data": [{ "code": -32000, "message": "Could not find object with given id" }] }, "id": 701 }
Created attachment 378667 [details] [PATCH] Proposed Fix
Comment on attachment 378667 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=378667&action=review r=me, nice! I've encountered this a handful of times but could never reproduce. Great catch! =D > Source/WebInspectorUI/UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js:378 > + this._ongoingCompletionRequests++; Style: `++this._ongoingCompletionRequests;` > Source/WebInspectorUI/UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js:381 > + _decrementOngoingCompletionRequests() Style: `--this._ongoingCompletionRequests;` > Source/WebInspectorUI/UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js:385 > + if (!this._ongoingCompletionRequests) We should assert that we haven't gone below `0` (and maybe even change the `if` condition to reflect that as well, given how many different branches this code has).
Comment on attachment 378667 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=378667&action=review >> Source/WebInspectorUI/UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js:378 >> + this._ongoingCompletionRequests++; > > Style: `++this._ongoingCompletionRequests;` There is no reason to prefer pre or post in JavaScript. We have hundreds of examples of each. I think readability should matter more and post is more readable in `this._member` cases, and our existing code seems to agree. >> Source/WebInspectorUI/UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js:385 >> + if (!this._ongoingCompletionRequests) > > We should assert that we haven't gone below `0` (and maybe even change the `if` condition to reflect that as well, given how many different branches this code has). Okay. It seems more likely that a decrement will be missed, in which case the counter ever increases, which is not easy to catch with assertions.
Source/WebInspectorUI/UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js:385 > >> + if (!this._ongoingCompletionRequests) > > > > We should assert that we haven't gone below `0` (and maybe even change the `if` condition to reflect that as well, given how many different branches this code has). > > Okay. It seems more likely that a decrement will be missed, in which case > the counter ever increases, which is not easy to catch with assertions. I think I'll add an assertion that we never get above 50. If so something has probably gone wrong and it would be useful to alert us of that in development!
https://trac.webkit.org/changeset/249846/webkit
<rdar://problem/55347422>