DevTools: merge array formatting logic

DevTools: merge array formatting logic

This CL makes array formatting consistent and prepares
RemoteObjectPreviewFormatter (ROPF) to introduce character cutoff logic.
Today, array formatting is split between two files. The logic in
ConsoleViewMessage has been folded into ROPF.js with all arrays showing gaps
with 'undefined x n' as much as possible then falling back to 'key: value'.

For background on character cutoff logic, see C1 in the console preview doc.

BUG=666882
Committed: https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e
Cr-Commit-Position: refs/heads/master@{#438294}

[luoe, 2016-12-08 22:35:25]

The CQ bit was checked by luoe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-08 22:35:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[luoe, 2016-12-08 22:36:44]

luoe@chromium.org changed reviewers:
+ lushnikov@chromium.org

[luoe, 2016-12-08 22:36:45]

Please take a look.

https://codereview.chromium.org/2566443004/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js
(right):

https://codereview.chromium.org/2566443004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js:29:
this._appendArrayPropertiesPreview(parentElement, preview,
formatArrayByIndices);
The plan is to pass preview.entries / preview.properties instead of passing in
the entire preview.  Each _append() will call some helper to allocate lengths to
each property to handle cutoff logic.

This will happen in a future patch.

https://codereview.chromium.org/2566443004/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
(left):

https://codereview.chromium.org/2566443004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js:653:
elements[name] = this._formatAsArrayEntry(property.value);
this._formatAsAccessorProperty and this._formatAsArrayEntry are now used solely
for getters in console.table().  In the future, I would like to get rid of
formatAsArrayEntry() and fold this logic into ROPF as well.

[luoe, 2016-12-08 22:39:49]

Description was changed from

==========
DevTools: move array formatting logic into RemoteObjectPreviewFormatter

BUG=666882
==========

to

==========
DevTools: move array formatting logic into RemoteObjectPreviewFormatter

This CL takes a step towards consistent array formatting and prepares
RemoteObjectPreviewFormatter (ROPF) to introduce character cutoff logic. Right
now, array formatting is split between two files. The logic in
ConsoleViewMessage gets moved to ROPF.js.

For background on character cutoff logic, see C1 in the console preview doc.

BUG=666882
==========

[lushnikov, 2016-12-08 23:23:09]

https://codereview.chromium.org/2566443004/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js
(right):

https://codereview.chromium.org/2566443004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js:83:
_appendArrayPropertiesPreview(parentElement, preview, formatArrayByIndices) {
let's not introduce one more parameter for array formatting. Let's do a single,
but comprehensive array-format!

For example, consider the following scenario: http://imgur.com/a/ZCqRE
Notice, how neither console.log(m), nor just "m" showed me the correct array.

That's due to this flag!

https://codereview.chromium.org/2566443004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
(left):

https://codereview.chromium.org/2566443004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js:637:
result.appendChild(this._formatParameterAsObject(array, false));
what happened to this logic?

[luoe, 2016-12-09 01:17:52]

Description was changed from

==========
DevTools: move array formatting logic into RemoteObjectPreviewFormatter

This CL takes a step towards consistent array formatting and prepares
RemoteObjectPreviewFormatter (ROPF) to introduce character cutoff logic. Right
now, array formatting is split between two files. The logic in
ConsoleViewMessage gets moved to ROPF.js.

For background on character cutoff logic, see C1 in the console preview doc.

BUG=666882
==========

to

==========
DevTools: merge array formatting logic

This CL makes array formatting consistent and prepares
RemoteObjectPreviewFormatter (ROPF) to introduce character cutoff logic.
Today, array formatting is split between two files. The logic in
ConsoleViewMessage has been folded into ROPF.js with all arrays showing gaps
with 'undefined x n' as much as possible then falling back to 'key: value'.

For background on character cutoff logic, see C1 in the console preview doc.

BUG=666882
==========

[luoe, 2016-12-09 01:18:35]

ptal, I've merged the two formats into one that includes information from both. 
A screenshot is in the bug.

https://codereview.chromium.org/2566443004/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js
(right):

https://codereview.chromium.org/2566443004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js:83:
_appendArrayPropertiesPreview(parentElement, preview, formatArrayByIndices) {
Sounds good to me!

https://codereview.chromium.org/2566443004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
(left):

https://codereview.chromium.org/2566443004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js:637:
result.appendChild(this._formatParameterAsObject(array, false));
Good call.  We used to fallback to the description when we couldn't get
properties.  The ObjectPreview can be undefined, so we should bail out as well. 
I've added a condition.

[lushnikov, 2016-12-12 22:10:43]

lgtm

https://codereview.chromium.org/2566443004/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js
(right):

https://codereview.chromium.org/2566443004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js:13:
static objectPropertyComparator(a, b) {
should it be private?

https://codereview.chromium.org/2566443004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js:84:
var indexProperties = preview.properties.filter(p => toArrayIndex(p.name) !==
-1).stableSort();
don't you need to pass in comparator here as well?

[luoe, 2016-12-13 03:57:35]

https://codereview.chromium.org/2566443004/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js
(right):

https://codereview.chromium.org/2566443004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js:13:
static objectPropertyComparator(a, b) {
On 2016/12/12 22:10:43, lushnikov wrote:
> should it be private?

Done.

https://codereview.chromium.org/2566443004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js:84:
var indexProperties = preview.properties.filter(p => toArrayIndex(p.name) !==
-1).stableSort();
On 2016/12/12 22:10:43, lushnikov wrote:
> don't you need to pass in comparator here as well?

Done.

[luoe, 2016-12-13 03:57:42]

The CQ bit was checked by luoe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 03:57:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 05:39:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 05:39:51]

Dry run: This issue passed the CQ dry run.

[luoe, 2016-12-13 17:51:22]

The CQ bit was checked by luoe@chromium.org

[luoe, 2016-12-13 17:51:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
lushnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2566443004/#ps100001
(title: "ac")

[commit-bot: I haz the power, 2016-12-13 17:51:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 17:59:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 17:59:52]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[luoe, 2016-12-13 19:40:25]

The CQ bit was checked by luoe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 19:41:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 21:26:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 21:26:50]

Dry run: This issue passed the CQ dry run.

[luoe, 2016-12-13 21:35:48]

The CQ bit was checked by luoe@chromium.org

[luoe, 2016-12-13 21:35:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
lushnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2566443004/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2016-12-13 21:36:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 21:48:59]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1481664948312000,
"parent_rev": "b23fe3ada44e1530a2af437e2a99f83a7ad2b422", "commit_rev":
"cedbed96233df1ad7b6cb5f094edb573252d2903"}

[commit-bot: I haz the power, 2016-12-13 21:49:26]

Description was changed from

==========
DevTools: merge array formatting logic

This CL makes array formatting consistent and prepares
RemoteObjectPreviewFormatter (ROPF) to introduce character cutoff logic.
Today, array formatting is split between two files. The logic in
ConsoleViewMessage has been folded into ROPF.js with all arrays showing gaps
with 'undefined x n' as much as possible then falling back to 'key: value'.

For background on character cutoff logic, see C1 in the console preview doc.

BUG=666882
==========

to

==========
DevTools: merge array formatting logic

This CL makes array formatting consistent and prepares
RemoteObjectPreviewFormatter (ROPF) to introduce character cutoff logic.
Today, array formatting is split between two files. The logic in
ConsoleViewMessage has been folded into ROPF.js with all arrays showing gaps
with 'undefined x n' as much as possible then falling back to 'key: value'.

For background on character cutoff logic, see C1 in the console preview doc.

BUG=666882

Review-Url: https://codereview.chromium.org/2566443004
==========

[commit-bot: I haz the power, 2016-12-13 21:49:28]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-12-13 21:52:16]

Description was changed from

==========
DevTools: merge array formatting logic

This CL makes array formatting consistent and prepares
RemoteObjectPreviewFormatter (ROPF) to introduce character cutoff logic.
Today, array formatting is split between two files. The logic in
ConsoleViewMessage has been folded into ROPF.js with all arrays showing gaps
with 'undefined x n' as much as possible then falling back to 'key: value'.

For background on character cutoff logic, see C1 in the console preview doc.

BUG=666882

Review-Url: https://codereview.chromium.org/2566443004
==========

to

==========
DevTools: merge array formatting logic

This CL makes array formatting consistent and prepares
RemoteObjectPreviewFormatter (ROPF) to introduce character cutoff logic.
Today, array formatting is split between two files. The logic in
ConsoleViewMessage has been folded into ROPF.js with all arrays showing gaps
with 'undefined x n' as much as possible then falling back to 'key: value'.

For background on character cutoff logic, see C1 in the console preview doc.

BUG=666882

Committed: https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e
Cr-Commit-Position: refs/heads/master@{#438294}
==========

[commit-bot: I haz the power, 2016-12-13 21:52:18]

Patchset 7 (id:??) landed as
https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e
Cr-Commit-Position: refs/heads/master@{#438294}

[luoe, 2016-12-16 23:27:16]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/2586623002/ by luoe@chromium.org.

The reason for reverting is: Console previews for arrays have been compromised. 
When logging an array with 100+ non-empty properties, the overflow properties
(after the first 100 in the preview) show up as 'undefined x N' when they are
not undefined.

Repro case:
var b = new Array(200);
b.fill(2);
console.log(b);

crbug.com/675259.