DevTools: Fix console.log for arrays in some corner cases.

DevTools: Fix console.log for arrays in some corner cases.

We didn't show array values defined on Array.prototype[], as well as non-index property names.

BUG=320632, 163817
R=pfeldman

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165713

[aandrey, 2014-01-23 14:36:47]

ping?

[pfeldman, 2014-01-23 16:19:02]

https://codereview.chromium.org/143263003/diff/1/Source/core/inspector/Inject...
File Source/core/inspector/InjectedScriptSource.js (right):

https://codereview.chromium.org/143263003/diff/1/Source/core/inspector/Inject...
Source/core/inspector/InjectedScriptSource.js:1027: var isArrayIndex =
(this.subtype === "array" && isArrayIndexPropertyName(name));
Move subtype check to outside of the loop?

https://codereview.chromium.org/143263003/diff/1/Source/devtools/front_end/Co...
File Source/devtools/front_end/ConsoleMessage.js (right):

https://codereview.chromium.org/143263003/diff/1/Source/devtools/front_end/Co...
Source/devtools/front_end/ConsoleMessage.js:414:
_appendArrayOrObjectPropertyElements: function(parent, propertyElements,
overflow, arrayLength)
This only runs for preview, right? Could you name it as such?

https://codereview.chromium.org/143263003/diff/1/Source/devtools/front_end/Co...
Source/devtools/front_end/ConsoleMessage.js:438:
propertyElements.stableSort(comparator);
Why sorting properties? They should come in sorted.

https://codereview.chromium.org/143263003/diff/1/Source/devtools/front_end/Co...
Source/devtools/front_end/ConsoleMessage.js:439: var isFlatArray = isArray &&
(arrayLength <= maxFlatArrayLength);
So we only render as flat when < 100 actual values? I.e. large sparse arrays are
not flat?

https://codereview.chromium.org/143263003/diff/1/Source/devtools/front_end/ut...
File Source/devtools/front_end/utilities.js (right):

https://codereview.chromium.org/143263003/diff/1/Source/devtools/front_end/ut...
Source/devtools/front_end/utilities.js:283: String.isArrayIndexPropertyName =
function(name, arrayLength)
We define these on prototype.

[pfeldman, 2014-01-23 16:19:09]

lgtm

[aandrey, 2014-01-24 06:22:03]

https://codereview.chromium.org/143263003/diff/1/Source/core/inspector/Inject...
File Source/core/inspector/InjectedScriptSource.js (right):

https://codereview.chromium.org/143263003/diff/1/Source/core/inspector/Inject...
Source/core/inspector/InjectedScriptSource.js:1027: var isArrayIndex =
(this.subtype === "array" && isArrayIndexPropertyName(name));
On 2014/01/23 16:19:02, pfeldman wrote:
> Move subtype check to outside of the loop?

Done.

https://codereview.chromium.org/143263003/diff/1/Source/devtools/front_end/Co...
File Source/devtools/front_end/ConsoleMessage.js (right):

https://codereview.chromium.org/143263003/diff/1/Source/devtools/front_end/Co...
Source/devtools/front_end/ConsoleMessage.js:414:
_appendArrayOrObjectPropertyElements: function(parent, propertyElements,
overflow, arrayLength)
On 2014/01/23 16:19:02, pfeldman wrote:
> This only runs for preview, right? Could you name it as such?

not only for preview: from _printArray also.

https://codereview.chromium.org/143263003/diff/1/Source/devtools/front_end/Co...
Source/devtools/front_end/ConsoleMessage.js:438:
propertyElements.stableSort(comparator);
On 2014/01/23 16:19:02, pfeldman wrote:
> Why sorting properties? They should come in sorted.

They are sorted according to the index in the prototype chain first.
This is needed to handle Array.prototype[0] = value;

https://codereview.chromium.org/143263003/diff/1/Source/devtools/front_end/Co...
Source/devtools/front_end/ConsoleMessage.js:439: var isFlatArray = isArray &&
(arrayLength <= maxFlatArrayLength);
On 2014/01/23 16:19:02, pfeldman wrote:
> So we only render as flat when < 100 actual values? I.e. large sparse arrays
are
> not flat?

Yes. This has been the case up to now - I didn't change this logic.

https://codereview.chromium.org/143263003/diff/1/Source/devtools/front_end/ut...
File Source/devtools/front_end/utilities.js (right):

https://codereview.chromium.org/143263003/diff/1/Source/devtools/front_end/ut...
Source/devtools/front_end/utilities.js:283: String.isArrayIndexPropertyName =
function(name, arrayLength)
On 2014/01/23 16:19:02, pfeldman wrote:
> We define these on prototype.

Didn't work  in this case. Because:
typeof name === "string"
typeof this === "Object"

[commit-bot: I haz the power, 2014-01-24 06:25:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aandrey@chromium.org/143263003/240001

[commit-bot: I haz the power, 2014-01-24 07:35:48]

Change committed as 165713

[julianmraya_gmail.com, 2014-02-01 08:12:12]

This fix causes excess console text when the printed array values are 
typeof 'function'... which is usually the case when adding members to the 
prototype.
It would be nice if the functions were truncated and expandable like object 
methods.

for example:
> function SomeClass(){} > SomeClass.prototype.asdf = function(){ 
console.log('asdf'); } > var foo = new SomeClass(); > foo; // the function 
is truncated and key text is colored
SomeClass {asdf: function}

> function ArrayLike(){ Array.apply(this, arguments); }
> ArrayLike.prototype = [];
> ArrayLike.prototype.asdf = function(){
console.log('asdf');
}
> var bar = new ArrayLike();
> bar; // the entire function is printed and key text is black
[asdf: function (){ console.log('asdf'); }]

It gets pretty messy if you're using classes that extend Array, 

like jQuery:
> $('body'); // .. and that's just the minified version!!
[
   
   1. <body class=​"source public panelsVisible ready">​…​</body>​

, context: 
   
   1. #document

, selector: "body", constructor: function (a,b){return new h.fn.init(a,b,d)}, 
init: function (a,c,d){var e,f,g,i;if(!a)return this;if(a.nodeType)return 
this.context=this[0]=a,this.length=1,this;if("body"===a&&!c&&H.body)return 
this.context=H,this[0]=H.body,this.selector=a,this.length=1,this;if("string"==typeof

a){if(e="<"===a.charAt(0)&&">"===a.charAt(a.length-1)&&a.length>=3?[null,a,null]:k.exec(a),!e||!e[1]&&c)return!c||c.jquery?(c||d).find(a):this.constructor(c).find(a);if(e[1])return

c=c instanceof 
h?c[0]:c,i=c?c.ownerDocument||c:H,g=o.exec(a),g?h.isPlainObject(c)?(a=[H.createElement(g[1])],h.fn.attr.call(a,c,!0)):a=[i.createElement(g[1])]:(g=h.buildFragment([e[1]],[i]),a=(g.cacheable?h.clone(g.fragment):g.fragment).childNodes),h.merge(this,a);if(f=H.getElementById(e[2]),f&&f.parentNode){if(f.id!==e[2])return

d.find(a);this.length=1,this[0]=f}return 
this.context=H,this.selector=a,this}return 
h.isFunction(a)?d.ready(a):(a.selector!==b&&(this.selector=a.selector,this.context=a.context),h.makeArray(a,this))},

jquery: "1.7.1"…]

or d3:
> d3.select('body');
[
Array[1]
, select: function (t){var e,n,r,i,o=[];t=g(t);for(var 
a=-1,s=this.length;++a<s;){o.push(e=[]),e.parentNode=(r=this[a]).parentNode;for(var

u=-1,c=r.length;++u<c;)(i=r[u])?(e.push(n=t.call(i,i.__data__,u,a)),n&&"__data__"in

i&&(n.__data__=i.__data__)):e.push(null)}return p(o)}, selectAll: function 
(t){var e,n,r=[];t=m(t);for(var i=-1,o=this.length;++i<o;)for(var 
a=this[i],s=-1,u=a.length;++s<u;)(n=a[s])&&(r.push(e=Ga(t.call(n,n.__data__,s,i))),e.parentNode=n);return

p(r)}, attr: function (t,e){if(arguments.length<2){if("string"==typeof 
t){var n=this.node();return 
t=Xa.ns.qualify(t),t.local?n.getAttributeNS(t.space,t.local):n.getAttribute(t)}for(e

in t)this.each(v(e,t[e]));return this}return this.each(v(t,e))}, classed:
function 
(t,e){if(arguments.length<2){if("string"==typeof t){var 
n=this.node(),r=(t=t.trim().split(/^|\s+/g)).length,i=-1;if(e=n.classList){for(;++i<r;)if(!e.contains(t[i]))return!1}else

for(e=n.getAttribute("class");++i<r;)if(!b(t[i]).test(e))return!1;return!0}for(e

in t)this.each(x(e,t[e]));return this}return this.each(x(t,e))}, style: function

(t,e,n){var r=arguments.length;if(3>r){if("string"!=typeof 
t){2>r&&(e="");for(n in t)this.each(_(n,t[n],e));return this}if(2>r)return 
Qa.getComputedStyle(this.node(),null).getPropertyValue(t);n=""}return 
this.each(_(t,e,n))}…]

Also, grouping the array object's properties and the indexed members inside 
of square brackets can very confusing because values in square brackets (in 
javascript) are generally recognized as the array's indexed values. 

On Tuesday, January 21, 2014 1:30:50 AM UTC-8, aan...@chromium.org wrote:

> Reviewers: pfeldman, 
>
> Description: 
> DevTools: Fix console.log for arrays in some corner cases. 
>
> We didn't show array values defined on Array.prototype[], as well as   
> non-index 
> property names. 
>
> BUG=320632,163817 
> R=pfeldman 
>
> Please review this at https://codereview.chromium.org/143263003/ 
>
> SVN Base: svn://svn.chromium.org/blink/trunk 
>
> Affected files (+677, -147 lines): 
>    M LayoutTests/http/tests/inspector/inspector-test.js 
>    A   
> LayoutTests/inspector/console/console-format-array-prototype-expected.txt 
>    A LayoutTests/inspector/console/console-format-array-prototype.html 
>    M LayoutTests/inspector/console/console-format-expected.txt 
>    M LayoutTests/inspector/console/console-format.html 
>    M   
> LayoutTests/inspector/console/console-object-preview-accessors-expected.txt 
>
>    M LayoutTests/inspector/console/console-object-preview-expected.txt 
>    M Source/core/inspector/InjectedScriptSource.js 
>    M Source/devtools/front_end/ConsoleMessage.js 
>    M Source/devtools/front_end/utilities.js 
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[pfeldman, 2014-02-01 10:08:56]

A revert of this CL has been created in
https://codereview.chromium.org/135973010/ by pfeldman@chromium.org.

The reason for reverting is: Console output got messy..