From 5c9a9143a9a11e26192d889c0a9aaddffad170d8 Mon Sep 17 00:00:00 2001 From: Mihai Sucan Date: Tue, 3 Dec 2013 15:32:41 +0200 Subject: [PATCH] Bug 943496 - Autocomplete should execute native getters; r=past --- ...ole_autocomplete_in_debugger_stackframe.js | 20 +- ...e_bug_651501_document_body_autocomplete.js | 13 +- toolkit/devtools/DevToolsUtils.js | 45 ++ toolkit/devtools/DevToolsUtils.jsm | 4 +- toolkit/devtools/server/actors/script.js | 59 +-- toolkit/devtools/webconsole/utils.js | 435 ++++++++---------- 6 files changed, 281 insertions(+), 295 deletions(-) diff --git a/browser/devtools/webconsole/test/browser_webconsole_autocomplete_in_debugger_stackframe.js b/browser/devtools/webconsole/test/browser_webconsole_autocomplete_in_debugger_stackframe.js index 9acefa4c9f6d..4ca19ffc5c12 100644 --- a/browser/devtools/webconsole/test/browser_webconsole_autocomplete_in_debugger_stackframe.js +++ b/browser/devtools/webconsole/test/browser_webconsole_autocomplete_in_debugger_stackframe.js @@ -34,13 +34,31 @@ function testCompletion(hud) { let input = jsterm.inputNode; let popup = jsterm.autocompletePopup; + // Test that document.title gives string methods. Native getters must execute. + input.value = "document.title."; + input.setSelectionRange(input.value.length, input.value.length); + jsterm.complete(jsterm.COMPLETE_HINT_ONLY, testNext); + yield undefined; + + let newItems = popup.getItems(); + ok(newItems.length > 0, "'document.title.' gave a list of suggestions"); + ok(newItems.some(function(item) { + return item.label == "substr"; + }), "autocomplete results do contain substr"); + ok(newItems.some(function(item) { + return item.label == "toLowerCase"; + }), "autocomplete results do contain toLowerCase"); + ok(newItems.some(function(item) { + return item.label == "strike"; + }), "autocomplete results do contain strike"); + // Test if 'f' gives 'foo1' but not 'foo2' or 'foo3' input.value = "f"; input.setSelectionRange(1, 1); jsterm.complete(jsterm.COMPLETE_HINT_ONLY, testNext); yield undefined; - let newItems = popup.getItems(); + newItems = popup.getItems(); ok(newItems.length > 0, "'f' gave a list of suggestions"); ok(!newItems.every(function(item) { return item.label != "foo1"; diff --git a/browser/devtools/webconsole/test/browser_webconsole_bug_651501_document_body_autocomplete.js b/browser/devtools/webconsole/test/browser_webconsole_bug_651501_document_body_autocomplete.js index e136b53ede89..10df84401ab9 100644 --- a/browser/devtools/webconsole/test/browser_webconsole_bug_651501_document_body_autocomplete.js +++ b/browser/devtools/webconsole/test/browser_webconsole_bug_651501_document_body_autocomplete.js @@ -29,11 +29,14 @@ function consoleOpened(aHud) { ok(popup.isOpen, "popup is open"); - // expected properties: - // __defineGetter__ __defineSetter__ __lookupGetter__ __lookupSetter__ - // constructor hasOwnProperty isPrototypeOf propertyIsEnumerable - // toLocaleString toSource toString unwatch valueOf watch. - ok(popup.itemCount >= 14, "popup.itemCount is correct"); + is(popup.itemCount, jsterm._autocompleteCache.length, + "popup.itemCount is correct"); + isnot(jsterm._autocompleteCache.indexOf("addEventListener"), -1, + "addEventListener is in the list of suggestions"); + isnot(jsterm._autocompleteCache.indexOf("bgColor"), -1, + "bgColor is in the list of suggestions"); + isnot(jsterm._autocompleteCache.indexOf("ATTRIBUTE_NODE"), -1, + "ATTRIBUTE_NODE is in the list of suggestions"); popup._panel.addEventListener("popuphidden", autocompletePopupHidden, false); diff --git a/toolkit/devtools/DevToolsUtils.js b/toolkit/devtools/DevToolsUtils.js index f373a585b3f0..6fbe6dde0dcf 100644 --- a/toolkit/devtools/DevToolsUtils.js +++ b/toolkit/devtools/DevToolsUtils.js @@ -166,3 +166,48 @@ function defineLazyPrototypeGetter(aObject, aKey, aCallback) { } }); } + +/** + * Safely get the property value from a Debugger.Object for a given key. Walks + * the prototype chain until the property is found. + * + * @param Debugger.Object aObject + * The Debugger.Object to get the value from. + * @param String aKey + * The key to look for. + * @return Any + */ +this.getProperty = function getProperty(aObj, aKey) { + let root = aObj; + try { + do { + const desc = aObj.getOwnPropertyDescriptor(aKey); + if (desc) { + if ("value" in desc) { + return desc.value; + } + // Call the getter if it's safe. + return hasSafeGetter(desc) ? desc.get.call(root).return : undefined; + } + aObj = aObj.proto; + } while (aObj); + } catch (e) { + // If anything goes wrong report the error and return undefined. + reportException("getProperty", e); + } + return undefined; +}; + +/** + * Determines if a descriptor has a getter which doesn't call into JavaScript. + * + * @param Object aDesc + * The descriptor to check for a safe getter. + * @return Boolean + * Whether a safe getter was found. + */ +this.hasSafeGetter = function hasSafeGetter(aDesc) { + let fn = aDesc.get; + return fn && fn.callable && fn.class == "Function" && fn.script === undefined; +}; + diff --git a/toolkit/devtools/DevToolsUtils.jsm b/toolkit/devtools/DevToolsUtils.jsm index 45f1e035fdcf..eb929d3c08c2 100644 --- a/toolkit/devtools/DevToolsUtils.jsm +++ b/toolkit/devtools/DevToolsUtils.jsm @@ -24,5 +24,7 @@ this.DevToolsUtils = { makeInfallible: makeInfallible, yieldingEach: yieldingEach, reportingDisabled: false , // Used by tests. - defineLazyPrototypeGetter: defineLazyPrototypeGetter + defineLazyPrototypeGetter: defineLazyPrototypeGetter, + getProperty: getProperty, + hasSafeGetter: hasSafeGetter, }; diff --git a/toolkit/devtools/server/actors/script.js b/toolkit/devtools/server/actors/script.js index ae70da2f4d30..81c0924d2f36 100644 --- a/toolkit/devtools/server/actors/script.js +++ b/toolkit/devtools/server/actors/script.js @@ -2660,49 +2660,6 @@ function isObject(aValue) { return type == "object" ? aValue !== null : type == "function"; } -/** - * Determines if a descriptor has a getter which doesn't call into JavaScript. - * - * @param Object aDesc - * The descriptor to check for a safe getter. - * @return Boolean - * Whether a safe getter was found. - */ -function hasSafeGetter(aDesc) { - let fn = aDesc.get; - return fn && fn.callable && fn.class == "Function" && fn.script === undefined; -} - -/** - * Safely get the property value from a Debugger.Object for a given key. Walks - * the prototype chain until the property is found. - * - * @param Debugger.Object aObject - * The Debugger.Object to get the value from. - * @param String aKey - * The key to look for. - * @return Any - */ -function getProperty(aObj, aKey) { - try { - do { - const desc = aObj.getOwnPropertyDescriptor(aKey); - if (desc) { - if ("value" in desc) { - return desc.value - } - // Call the getter if it's safe. - return hasSafeGetter(desc) ? desc.get.call(aObj) : undefined; - } - aObj = aObj.proto; - } while (aObj); - } catch (e) { - // If anything goes wrong report the error and return undefined. - DevToolsUtils.reportException("getProperty", e); - } - return undefined; -} - /** * Create a function that can safely stringify Debugger.Objects of a given * builtin type. @@ -2725,14 +2682,14 @@ function createBuiltinStringifier(aCtor) { * The stringification of the object. */ function errorStringify(aObj) { - let name = getProperty(aObj, "name"); + let name = DevToolsUtils.getProperty(aObj, "name"); if (name === "" || name === undefined) { name = aObj.class; } else if (isObject(name)) { name = stringify(name); } - let message = getProperty(aObj, "message"); + let message = DevToolsUtils.getProperty(aObj, "message"); if (isObject(message)) { message = stringify(message); } @@ -2790,7 +2747,7 @@ let stringifiers = { seen.add(obj); - const len = getProperty(obj, "length"); + const len = DevToolsUtils.getProperty(obj, "length"); let string = ""; // The following check is only required because the debuggee could possibly @@ -2819,10 +2776,10 @@ let stringifiers = { return string; }, DOMException: obj => { - const message = getProperty(obj, "message") || ""; - const result = (+getProperty(obj, "result")).toString(16); - const code = getProperty(obj, "code"); - const name = getProperty(obj, "name") || ""; + const message = DevToolsUtils.getProperty(obj, "message") || ""; + const result = (+DevToolsUtils.getProperty(obj, "result")).toString(16); + const code = DevToolsUtils.getProperty(obj, "code"); + const name = DevToolsUtils.getProperty(obj, "name") || ""; return '[Exception... "' + message + '" ' + 'code: "' + code +'" ' + @@ -3102,7 +3059,7 @@ ObjectActor.prototype = { continue; } - if (hasSafeGetter(desc)) { + if (DevToolsUtils.hasSafeGetter(desc)) { getters.add(name); } } diff --git a/toolkit/devtools/webconsole/utils.js b/toolkit/devtools/webconsole/utils.js index bc224791950c..b77bee53faec 100644 --- a/toolkit/devtools/webconsole/utils.js +++ b/toolkit/devtools/webconsole/utils.js @@ -24,6 +24,7 @@ loader.lazyServiceGetter(this, "gActivityDistributor", loader.lazyImporter(this, "gDevTools", "resource:///modules/devtools/gDevTools.jsm"); loader.lazyImporter(this, "devtools", "resource://gre/modules/devtools/Loader.jsm"); loader.lazyImporter(this, "VariablesView", "resource:///modules/devtools/VariablesView.jsm"); +loader.lazyImporter(this, "DevToolsUtils", "resource://gre/modules/devtools/DevToolsUtils.jsm"); // Match the function name from the result of toString() or toSource(). // @@ -794,290 +795,250 @@ function JSPropertyProvider(aDbgObject, anEnvironment, aInputValue, aCursor) return null; } - let matches = null; - let matchProp = ""; - let lastDot = completionPart.lastIndexOf("."); if (lastDot > 0 && (completionPart[0] == "'" || completionPart[0] == '"') && completionPart[lastDot - 1] == completionPart[0]) { // We are completing a string literal. - let obj = String.prototype; - matchProp = completionPart.slice(lastDot + 1); - let matches = Object.keys(getMatchedProps(obj, {matchProp:matchProp})); - - return { - matchProp: matchProp, - matches: matches, - }; + let matchProp = completionPart.slice(lastDot + 1); + return getMatchedProps(String.prototype, matchProp); } - else { - // We are completing a variable / a property lookup. - let properties = completionPart.split("."); - if (properties.length > 1) { - matchProp = properties.pop().trimLeft(); - let obj; - //The first property must be found in the environment or the Debugger.Object - //depending of whether the debugger is paused or not - let prop = properties[0]; - if (anEnvironment) { - obj = getVariableInEnvironment(anEnvironment, prop); - } - else { - obj = getPropertyInDebuggerObject(aDbgObject, prop); - } - if (obj == null) { - return null; - } + // We are completing a variable / a property lookup. + let properties = completionPart.split("."); + let matchProp = properties.pop().trimLeft(); + let obj = aDbgObject; - //We get the rest of the properties recursively starting from the Debugger.Object - // that wraps the first property - for (let i = 1; i < properties.length; i++) { - let prop = properties[i].trim(); - if (!prop) { - return null; - } - - obj = getPropertyInDebuggerObject(obj, prop); - - // If obj is undefined or null (which is what "== null" does), - // then there is no chance to run completion on it. Exit here. - if (obj == null) { - return null; - } - } - - // If the final property is a primitive - if (typeof obj != 'object' || obj === null) { - matchProp = completionPart.slice(lastDot + 1); - let matches = Object.keys(getMatchedProps(obj, {matchProp:matchProp})); - - return { - matchProp: matchProp, - matches: matches, - }; - } - return getMatchedPropsInDbgObject(obj, matchProp); - } - else { - matchProp = properties[0].trimLeft(); - if (anEnvironment) { - return getMatchedPropsInEnvironment(anEnvironment, matchProp); - } - else { - if (typeof aDbgObject != 'object' || aDbgObject === null) { - matchProp = completionPart.slice(lastDot + 1); - let matches = Object.keys(getMatchedProps(aDbgObject, {matchProp:matchProp})); - - return { - matchProp: matchProp, - matches: matches, - }; - } - return getMatchedPropsInDbgObject(aDbgObject, matchProp); - } + // The first property must be found in the environment if the debugger is + // paused. + if (anEnvironment) { + if (properties.length == 0) { + return getMatchedPropsInEnvironment(anEnvironment, matchProp); } + obj = getVariableInEnvironment(anEnvironment, properties.shift()); } -} -/** - * Returns the value of aProp in anEnvironment as a debuggee value, by recursively checking the environment chain - * - * @param object anEnvironment - * A Debugger.Environment to look the aProp into. - * @param string aProp - * The property that is looked up. - * @returns null or object - * A Debugger.Object if aProp exists in the environment chain, null otherwise. - */ -function getVariableInEnvironment(anEnvironment, aProp) -{ - for (let env = anEnvironment; env; env = env.parent) { - try { - let obj = env.getVariable(aProp); - if (obj) { - return obj; - } + if (!isObjectUsable(obj)) { + return null; + } + + // We get the rest of the properties recursively starting from the Debugger.Object + // that wraps the first property + for (let prop of properties) { + prop = prop.trim(); + if (!prop) { + return null; } - catch (ex) { + + obj = DevToolsUtils.getProperty(obj, prop); + + if (!isObjectUsable(obj)) { return null; } } - return null; -} -/** - * Returns the value of aProp in aDbgObject as a debuggee value, by recursively checking the prototype chain - * - * @param object aDbgObject - * A Debugger.Object to look the aProp into. - * @param string aProp - * The property that is looked up. - * @returns null or object - * A Debugger.Object if aProp exists in the prototype chain, null otherwise. - */ -function getPropertyInDebuggerObject(aDbgObject, aProp) -{ - let dbgObject = aDbgObject; - while (dbgObject) { - try { - let desc = dbgObject.getOwnPropertyDescriptor(aProp) - if (desc) { - let obj = desc.value; - if (obj) - return obj; - obj = desc.get; - if (obj) - return obj; - } - dbgObject = dbgObject.proto; - } - catch (ex) { - return null; - } + // If the final property is a primitive + if (typeof obj != "object") { + return getMatchedProps(obj, matchProp); } - return null; + + return getMatchedPropsInDbgObject(obj, matchProp); } /** - * Get all properties on the given Debugger.Environment (and its parent chain) that match a given prefix. + * Check if the given Debugger.Object can be used for autocomplete. * - * @param Debugger.Environment anEnvironment - * Debugger.Environment whose properties we want to filter. - * - * @param string matchProp Filter for properties that match this one. - * - * @return object - * Object that contains the matchProp and the list of names. + * @param Debugger.Object aObject + * The Debugger.Object to check. + * @return boolean + * True if further inspection into the object is possible, or false + * otherwise. */ -function getMatchedPropsInEnvironment(anEnvironment, matchProp) +function isObjectUsable(aObject) { - let names = Object.create(null); - let c = MAX_COMPLETIONS; - for (let env = anEnvironment; env; env = env.parent) { - let ownNames = env.names(); - for (let i = 0; i < ownNames.length; i++) { - if (ownNames[i].indexOf(matchProp) != 0 || - ownNames[i] in names) { - continue; - } - c--; - if (c < 0) { - return { - matchProp: matchProp, - matches: Object.keys(names) - }; - } - names[ownNames[i]] = true; - } + if (aObject == null) { + return false; } - return { - matchProp: matchProp, - matches: Object.keys(names) - }; -} -/** - * Get all properties on the given Debugger.Object (and the prototype chain of the wrapped value) that match a given prefix. - * - * @param Debugger.Object aDbgObject - * Debugger.Object whose properties we want to filter. - * - * @param string matchProp Filter for properties that match this one. - * - * @return object - * Object that contains the matchProp and the list of names. - */ -function getMatchedPropsInDbgObject(aDbgObject, matchProp) -{ - let names = Object.create(null); - let c = MAX_COMPLETIONS; - for (let dbg = aDbgObject; dbg; dbg = dbg.proto) { - let raw = dbg.unsafeDereference(); - if (Cu.isDeadWrapper(raw)) { - return null; - } - let ownNames = dbg.getOwnPropertyNames(); - for (let i = 0; i < ownNames.length; i++) { - if (ownNames[i].indexOf(matchProp) != 0 || - ownNames[i] in names) { - continue; - } - c--; - if (c < 0) { - return { - matchProp: matchProp, - matches: Object.keys(names) - }; - } - names[ownNames[i]] = true; - } + if (typeof aObject == "object" && aObject.class == "DeadObject") { + return false; } - return { - matchProp: matchProp, - matches: Object.keys(names) - }; + + return true; } /** - * Get all accessible properties on this JS value. - * Filter those properties by name. - * Take only a certain number of those. - * - * @param mixed aObj - * JS value whose properties we want to collect. - * - * @param object aOptions - * Options that the algorithm takes. - * - matchProp (string): Filter for properties that match this one. - * Defaults to the empty string (which always matches). - * - * @return object - * Object whose keys are all accessible properties on the object. + * @see getExactMatch_impl() */ -function getMatchedProps(aObj, aOptions = {matchProp: ""}) +function getVariableInEnvironment(anEnvironment, aName) { - // Argument defaults. - aOptions.matchProp = aOptions.matchProp || ""; + return getExactMatch_impl(anEnvironment, aName, DebuggerEnvironmentSupport); +} - if (aObj == null) { return {}; } - try { - Object.getPrototypeOf(aObj); - } catch(e) { +/** + * @see getMatchedProps_impl() + */ +function getMatchedPropsInEnvironment(anEnvironment, aMatch) +{ + return getMatchedProps_impl(anEnvironment, aMatch, DebuggerEnvironmentSupport); +} + +/** + * @see getMatchedProps_impl() + */ +function getMatchedPropsInDbgObject(aDbgObject, aMatch) +{ + return getMatchedProps_impl(aDbgObject, aMatch, DebuggerObjectSupport); +} + +/** + * @see getMatchedProps_impl() + */ +function getMatchedProps(aObj, aMatch) +{ + if (typeof aObj != "object") { aObj = aObj.constructor.prototype; } - let c = MAX_COMPLETIONS; - let names = Object.create(null); // Using an Object to avoid duplicates. + return getMatchedProps_impl(aObj, aMatch, JSObjectSupport); +} + +/** + * Get all properties in the given object (and its parent prototype chain) that + * match a given prefix. + * + * @param mixed aObj + * Object whose properties we want to filter. + * @param string aMatch + * Filter for properties that match this string. + * @return object + * Object that contains the matchProp and the list of names. + */ +function getMatchedProps_impl(aObj, aMatch, {chainIterator, getProperties}) +{ + let matches = new Set(); // We need to go up the prototype chain. - let ownNames = null; - while (aObj !== null) { - ownNames = Object.getOwnPropertyNames(aObj); - for (let i = 0; i < ownNames.length; i++) { - // Filtering happens here. - // If we already have it in, no need to append it. - if (ownNames[i].indexOf(aOptions.matchProp) != 0 || - ownNames[i] in names) { + let iter = chainIterator(aObj); + for (let obj of iter) { + let props = getProperties(obj); + for (let prop of props) { + if (prop.indexOf(aMatch) != 0) { continue; } - c--; - if (c < 0) { - return names; - } + // If it is an array index, we can't take it. // This uses a trick: converting a string to a number yields NaN if // the operation failed, and NaN is not equal to itself. - if (+ownNames[i] != +ownNames[i]) { - names[ownNames[i]] = true; + if (+prop != +prop) { + matches.add(prop); + } + + if (matches.size > MAX_COMPLETIONS) { + break; } } - aObj = Object.getPrototypeOf(aObj); + + if (matches.size > MAX_COMPLETIONS) { + break; + } } - return names; + return { + matchProp: aMatch, + matches: [...matches], + }; } +/** + * Returns a property value based on its name from the given object, by + * recursively checking the object's prototype. + * + * @param object aObj + * An object to look the property into. + * @param string aName + * The property that is looked up. + * @returns object|undefined + * A Debugger.Object if the property exists in the object's prototype + * chain, undefined otherwise. + */ +function getExactMatch_impl(aObj, aName, {chainIterator, getProperty}) +{ + // We need to go up the prototype chain. + let iter = chainIterator(aObj); + for (let obj of iter) { + let prop = getProperty(obj, aName, aObj); + if (prop) { + return prop.value; + } + } + return undefined; +} + + +let JSObjectSupport = { + chainIterator: function(aObj) + { + while (aObj) { + yield aObj; + aObj = Object.getPrototypeOf(aObj); + } + }, + + getProperties: function(aObj) + { + return Object.getOwnPropertyNames(aObj); + }, + + getProperty: function() + { + // getProperty is unsafe with raw JS objects. + throw "Unimplemented!"; + }, +}; + +let DebuggerObjectSupport = { + chainIterator: function(aObj) + { + while (aObj) { + yield aObj; + aObj = aObj.proto; + } + }, + + getProperties: function(aObj) + { + return aObj.getOwnPropertyNames(); + }, + + getProperty: function(aObj, aName, aRootObj) + { + // This is left unimplemented in favor to DevToolsUtils.getProperty(). + throw "Unimplemented!"; + }, +}; + +let DebuggerEnvironmentSupport = { + chainIterator: function(aObj) + { + while (aObj) { + yield aObj; + aObj = aObj.parent; + } + }, + + getProperties: function(aObj) + { + return aObj.names(); + }, + + getProperty: function(aObj, aName) + { + // TODO: we should use getVariableDescriptor() here - bug 725815. + let result = aObj.getVariable(aName); + return result === undefined ? null : { value: result }; + }, +}; + exports.JSPropertyProvider = JSPropertyProvider; })(WebConsoleUtils);