Reland of DevTools: merge array formatting logic

Reland of DevTools: merge array formatting logic

This CL makes array formatting consistent. ConsoleViewMessage lets
RemoteObjectPreviewFormatter checks for arrays and handle them, showing gaps
('undefined x n') only when the preview is guaranteed to hold all render-able
properties.

The original version of this CL was reverted:
https://codereview.chromium.org/2566443004/

BUG=666882
Committed: https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9
Cr-Commit-Position: refs/heads/master@{#441212}

[luoe, 2016-12-22 18:40:32]

Description was changed from

==========
wip

BUG=
==========

to

==========
Reland of DevTools: merge array formatting logic

This CL makes array formatting consistent. ConsoleViewMessage lets
RemoteObjectPreviewFormatter checks for arrays and handle them, showing gaps
('undefined x n') only when the preview is guaranteed to hold all render-able
properties.

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

[luoe, 2016-12-22 18:41:04]

Description was changed from

==========
Reland of DevTools: merge array formatting logic

This CL makes array formatting consistent. ConsoleViewMessage lets
RemoteObjectPreviewFormatter checks for arrays and handle them, showing gaps
('undefined x n') only when the preview is guaranteed to hold all render-able
properties.

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

to

==========
Reland of DevTools: merge array formatting logic

This CL makes array formatting consistent. ConsoleViewMessage lets
RemoteObjectPreviewFormatter checks for arrays and handle them, showing gaps
('undefined x n') only when the preview is guaranteed to hold all render-able
properties.

The original version of this CL was reverted:
https://codereview.chromium.org/2566443004/

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

[luoe, 2016-12-22 18:48:18]

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

[commit-bot: I haz the power, 2016-12-22 18:48:37]

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-22 18:55:30]

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

[luoe, 2016-12-22 18:55:31]

The differences between this CL and the reverted one are:
- Original CL: eval'ing a long array would not show any preview.  This CL: all
branches try to show a preview, so arrays use formatParameterAsObject()
- Gaps are not shown when the preview overflowed.  Since preview properties are
not guaranteed to be in any order, we cannot reliably show a gap unless we know
all the properties are there.

Please take a look

[dgozman, 2016-12-28 00:40:37]

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

https://codereview.chromium.org/2596133002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js:631:
array.getAllProperties(false, printArrayResult.bind(this));
Let's investigate how to trigger this, because the patch changes behavior in
this case.

[luoe, 2016-12-28 22:21:56]

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

https://codereview.chromium.org/2596133002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js:631:
array.getAllProperties(false, printArrayResult.bind(this));
It turns out that this does get triggered in a strange way!
Example 1:

var arr = [1];
console.dirxml(arr);
arr.push(2);

Actual result: [1]

dirxml() leads to the async getAllProperties() call, and the
printArrayResult(properties) is invoked with 2 index properties: Prop{name: "0",
value: 1} and Prop{name: "1", value: 2}.  The reason why we don't see [1, 2] is
that we're using the old remote object on line 669, and array.arrayLength()
returns 1.  If we want to see [1, 2], we should have been searching the results
for the "length" property and using that value as the arrayLength.

This behavior seems safe to remove if we want to make both formats consistent.

[dgozman, 2017-01-03 18:38:49]

lgtm

https://codereview.chromium.org/2596133002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/console/console-object-preview-expected.txt
(right):

https://codereview.chromium.org/2596133002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/console/console-object-preview-expected.txt:45:
console-object-preview.html:45 [undefined × 50, 50, undefined × 99]
console-message > source-code > console-message-url devtools-link >
console-message-text > console-view-object-properties-section object-value-array
source-code > tree-outline-disclosure tree-outline-disclosure-hide-overflow >
tree-outline source-code object-properties-section > parent
object-properties-section-root-element > selection fill > console-object-preview
> object-value-undefined > object-value-number > object-value-undefined >
children
Let's add a test for an array which has defined properties at indices 57*x+32
for x in [0..99].

[luoe, 2017-01-03 19:24:36]

https://codereview.chromium.org/2596133002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/console/console-object-preview-expected.txt
(right):

https://codereview.chromium.org/2596133002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/console/console-object-preview-expected.txt:45:
console-object-preview.html:45 [undefined × 50, 50, undefined × 99]
console-message > source-code > console-message-url devtools-link >
console-message-text > console-view-object-properties-section object-value-array
source-code > tree-outline-disclosure tree-outline-disclosure-hide-overflow >
tree-outline source-code object-properties-section > parent
object-properties-section-root-element > selection fill > console-object-preview
> object-value-undefined > object-value-number > object-value-undefined >
children
On 2017/01/03 18:38:49, dgozman wrote:
> Let's add a test for an array which has defined properties at indices 57*x+32
> for x in [0..99].

Done.

[luoe, 2017-01-03 19:25:01]

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

[commit-bot: I haz the power, 2017-01-03 19:25:21]

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, 2017-01-03 21:16:11]

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

[commit-bot: I haz the power, 2017-01-03 21:16:12]

Dry run: This issue passed the CQ dry run.

[luoe, 2017-01-03 21:28:37]

The CQ bit was checked by luoe@chromium.org

[luoe, 2017-01-03 21:28:37]

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

[commit-bot: I haz the power, 2017-01-03 21:28:55]

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, 2017-01-03 21:33:30]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1483478917498500,
"parent_rev": "ec319476abc11398ae5c13b12684964f145e8454", "commit_rev":
"a85c6636acc96b4f97f52551c3c22a65197f0528"}

[commit-bot: I haz the power, 2017-01-03 21:33:59]

Description was changed from

==========
Reland of DevTools: merge array formatting logic

This CL makes array formatting consistent. ConsoleViewMessage lets
RemoteObjectPreviewFormatter checks for arrays and handle them, showing gaps
('undefined x n') only when the preview is guaranteed to hold all render-able
properties.

The original version of this CL was reverted:
https://codereview.chromium.org/2566443004/

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

to

==========
Reland of DevTools: merge array formatting logic

This CL makes array formatting consistent. ConsoleViewMessage lets
RemoteObjectPreviewFormatter checks for arrays and handle them, showing gaps
('undefined x n') only when the preview is guaranteed to hold all render-able
properties.

The original version of this CL was reverted:
https://codereview.chromium.org/2566443004/

BUG=666882

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

[commit-bot: I haz the power, 2017-01-03 21:34:04]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2017-01-03 21:36:58]

Description was changed from

==========
Reland of DevTools: merge array formatting logic

This CL makes array formatting consistent. ConsoleViewMessage lets
RemoteObjectPreviewFormatter checks for arrays and handle them, showing gaps
('undefined x n') only when the preview is guaranteed to hold all render-able
properties.

The original version of this CL was reverted:
https://codereview.chromium.org/2566443004/

BUG=666882

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

to

==========
Reland of DevTools: merge array formatting logic

This CL makes array formatting consistent. ConsoleViewMessage lets
RemoteObjectPreviewFormatter checks for arrays and handle them, showing gaps
('undefined x n') only when the preview is guaranteed to hold all render-able
properties.

The original version of this CL was reverted:
https://codereview.chromium.org/2566443004/

BUG=666882

Committed: https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9
Cr-Commit-Position: refs/heads/master@{#441212}
==========

[commit-bot: I haz the power, 2017-01-03 21:36:59]

Patchset 6 (id:??) landed as
https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9
Cr-Commit-Position: refs/heads/master@{#441212}