Summary: | REGRESSION (r249078): Flaky crash in com.apple.JavaScriptCore: Inspector::InjectedScriptModule::ensureInjected | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryan Haddad <ryanhaddad> | ||||||||
Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, ews-watchlist, hi, inspector-bugzilla-changes, joepeck, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bot-watchers-bugzilla, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Other | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=200971 https://bugs.webkit.org/show_bug.cgi?id=201366 |
||||||||||
Attachments: |
|
Description
Ryan Haddad
2019-08-27 13:56:51 PDT
stderr: Failed to parse/execute CommandLineAPI! //# sourceURL=__InjectedScript_CommandLineAPIModuleSource.js?(function(InjectedScriptHost,inspectedGlobalObject,injectedScriptId,injectedScript,RemoteObject,CommandLineAPIHost){injectedScript._inspectObject=function(object){if(arguments.length===0)?return;let objectId=RemoteObject.create(object,"");let hints={};switch(RemoteObject.describe(object)){case"Database":let databaseId=CommandLineAPIHost.databaseId(object)?if(databaseId)?hints.databaseId=databaseId;break;case"Storage":let storageId=CommandLineAPIHost.storageId(object)?if(storageId)?hints.domStorageId=InjectedScriptHost.evaluate("("+storageId+")");break;}?CommandLineAPIHost.inspect(objectId,hints);return object;};injectedScript.addCommandLineAPIGetter("0",function(){return CommandLineAPIHost.inspectedObject();});injectedScript.addCommandLineAPIMethod("copy",function(object){let string=null;let subtype=RemoteObject.subtype(object);if(subtype==="node")?string=object.outerHTML;else if(subtype==="regexp")?string=""+object;else if(injectedScript.isPrimitiveValue(object))?string=""+object;else if(typeof object==="symbol")?string=String(object);else if(typeof object==="function")?string=""+object;else{try{string=JSON.stringify(object,null," ");}catch{string=""+object;}}?CommandLineAPIHost.copyText(string);});injectedScript.addCommandLineAPIMethod("getEventListeners",function(target){return CommandLineAPIHost.getEventListeners(target);});function normalizeEventTypes(types){if(types===undefined)?types=["mouse","key","touch","control","abort","blur","change","devicemotion","deviceorientation","error","focus","load","reset","resize","scroll","search","select","submit","unload"];else if(typeof types==="string")?types=[types];let result=[];for(let i=0;i<types.length;i++){if(types[i]==="mouse")?result.push("click","dblclick","mousedown","mousemove","mouseout","mouseover","mouseup","mousewheel");else if(types[i]==="key")?result.push("keydown","keypress","keyup","textInput");else if(types[i]==="touch")?result.push("touchcancel","touchend","touchmove","touchstart");else if(types[i]==="control")?result.push("blur","change","focus","reset","resize","scroll","select","submit","zoom");else?result.push(types[i]);}?return result;}?function logEvent(event)?{inspectedGlobalObject.console.log(event.type,event);}?injectedScript.addCommandLineAPIMethod("monitorEvents",function(object,types){if(!object||!object.addEventListener||!object.removeEventListener)?return;types=normalizeEventTypes(types);for(let i=0;i<types.length;++i){object.removeEventListener(types[i],logEvent,false);object.addEventListener(types[i],logEvent,false);}});injectedScript.addCommandLineAPIMethod("unmonitorEvents",function(object,types){if(!object||!object.addEventListener||!object.removeEventListener)?return;types=normalizeEventTypes(types);for(let i=0;i<types.length;++i)?object.removeEventListener(types[i],logEvent,false);});if(inspectedGlobalObject.document&&inspectedGlobalObject.Node){function canQuerySelectorOnNode(node){return node&&InjectedScriptHost.subtype(node)==="node"&&(node.nodeType===inspectedGlobalObject.Node.ELEMENT_NODE||node.nodeType===inspectedGlobalObject.Node.DOCUMENT_NODE||node.nodeType===inspectedGlobalObject.Node.DOCUMENT_FRAGMENT_NODE);}?injectedScript.addCommandLineAPIMethod("$",function(selector,start){if(canQuerySelectorOnNode(start))?return start.querySelector(selector);let result=inspectedGlobalObject.document.querySelector(selector);if(result)?return result;if(selector&&selector[0]!=="#"){result=inspectedGlobalObject.document.getElementById(selector);if(result){inspectedGlobalObject.console.warn("The console function $() has changed from $=getElementById(id) to $=querySelector(selector). You might try $(\"#%s\")",selector);return null;}}?return result;});injectedScript.addCommandLineAPIMethod("$$",function(selector,start){if(canQuerySelectorOnNode(start))?return Array.from(start.querySelectorAll(selector));return Array.from(inspectedGlobalObject.document.querySelectorAll(selector));});injectedScript.addCommandLineAPIMethod("$x",function(xpath,context){let doc=(context&&context.ownerDocument)||inspectedGlobalObject.document;var result=doc.evaluate(xpath,context||doc,null,XPathResult.ANY_TYPE,null);switch(result.resultType){case XPathResult.NUMBER_TYPE:return result.numberValue;case XPathResult.STRING_TYPE:return result.stringValue;case XPathResult.BOOLEAN_TYPE:return result.booleanValue;default:var nodes=[];var node;while(node=result.iterateNext())?nodes.push(node);return nodes;}});}})? I think this is related to https://trac.webkit.org/changeset/249078 Created attachment 377758 [details]
Patch
Comment on attachment 377758 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377758&action=review Seems fine. I actually liked the old methods better, but I don't have a strong preference. > Source/JavaScriptCore/inspector/InjectedScriptModule.cpp:75 > + hadException = false; This shouldn't be necessary. > Source/JavaScriptCore/inspector/InjectedScriptSource.js:-436 > - setInspectObject(callback) > - { > - this._inspectObject = callback; > - } > - > - addCommandLineAPIGetter(name, func) > - { > - InjectedScript.CommandLineAPI._getters.push({name, func}); > - } > - > - addCommandLineAPIMethod(name, func) > - { > - func.toString = function() { return "function " + name + "() { [Command Line API] }" }; > - InjectedScript.CommandLineAPI._methods.push({name, func}); > - } I found this approach really clean! Comment on attachment 377758 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377758&action=review >> Source/JavaScriptCore/inspector/InjectedScriptModule.cpp:75 >> + hadException = false; > > This shouldn't be necessary. There's nothing that resets `hadException` back to `false` in the case that it was `true` for the first `callFunctionWithEvalEnabled`. If `hasInjectedModule` threw an exception (`hadException = true`) and the resulting `injectModule` did NOT throw an exception, `hadException` would still be `true`. >> Source/JavaScriptCore/inspector/InjectedScriptSource.js:-436 >> - } > > I found this approach really clean! I'm not as much of a fan of this approach as it's somewhat of a weird paradigm (calling a function on an instance of a class that adds values to a static list). I'd have preferred to have exposed the `InjectedScript` class itself and have these be static, but at that point we should just expose the `CommandLineAPI` class and have additional callers add to it directly. Created attachment 377921 [details]
Patch
Update expected output of additional tests.
Comment on attachment 377921 [details] Patch Clearing flags on attachment: 377921 Committed r249445: <https://trac.webkit.org/changeset/249445> All reviewed patches have been landed. Closing bug. |