[inspector] added a test for crash in wrapping async evaluate result

[inspector] added a test for crash in wrapping async evaluate result

BUG=chromium:651211
R=dgozman@chromium.org

Committed: https://crrev.com/c9391d15ca7fd2f80d0f907c39f2765a20af8749
Cr-Commit-Position: refs/heads/master@{#39933}

[kozy, 2016-09-28 21:17:27]

Dmitry, please take a look.

[dgozman, 2016-09-28 23:16:27]

https://codereview.chromium.org/2381693002/diff/1/test/inspector-protocol/run...
File test/inspector-protocol/runtime/evaluate-async-with-wrap-error.js (right):

https://codereview.chromium.org/2381693002/diff/1/test/inspector-protocol/run...
test/inspector-protocol/runtime/evaluate-async-with-wrap-error.js:7:
InspectorTest.sendCommand("Runtime.evaluate", {
sendCommandPromise. The less callback-based calls we have the better.

https://codereview.chromium.org/2381693002/diff/1/test/inspector-protocol/run...
test/inspector-protocol/runtime/evaluate-async-with-wrap-error.js:10:
awaitPromise: true}, dumpErrorAndCompleTest);
This formatting is a mess...

https://codereview.chromium.org/2381693002/diff/1/test/inspector-protocol/run...
test/inspector-protocol/runtime/evaluate-async-with-wrap-error.js:15:
InspectorTest.logObject(message);
Let's finally introduce logMessage?

[kozy, 2016-09-28 23:51:42]

Patchset #2 (id:20001) has been deleted

[kozy, 2016-09-28 23:53:25]

all done, please take a look

https://codereview.chromium.org/2381693002/diff/1/test/inspector-protocol/run...
File test/inspector-protocol/runtime/evaluate-async-with-wrap-error.js (right):

https://codereview.chromium.org/2381693002/diff/1/test/inspector-protocol/run...
test/inspector-protocol/runtime/evaluate-async-with-wrap-error.js:7:
InspectorTest.sendCommand("Runtime.evaluate", {
On 2016/09/28 23:16:26, dgozman wrote:
> sendCommandPromise. The less callback-based calls we have the better.

Done.

https://codereview.chromium.org/2381693002/diff/1/test/inspector-protocol/run...
test/inspector-protocol/runtime/evaluate-async-with-wrap-error.js:10:
awaitPromise: true}, dumpErrorAndCompleTest);
On 2016/09/28 23:16:27, dgozman wrote:
> This formatting is a mess...

Done.

https://codereview.chromium.org/2381693002/diff/1/test/inspector-protocol/run...
test/inspector-protocol/runtime/evaluate-async-with-wrap-error.js:15:
InspectorTest.logObject(message);
On 2016/09/28 23:16:27, dgozman wrote:
> Let's finally introduce logMessage?

Done.

https://codereview.chromium.org/2381693002/diff/40001/test/inspector-protocol...
File test/inspector-protocol/runtime/evaluate-async-with-wrap-error.js (right):

https://codereview.chromium.org/2381693002/diff/40001/test/inspector-protocol...
test/inspector-protocol/runtime/evaluate-async-with-wrap-error.js:6: // async
result.
I think that we can put test description into comment at the begin of test file.
Otherwise we need to figure out what is best way to put long test description
under 80 line length limit.
WDYT?

[kozy, 2016-09-30 02:10:13]

Patchset #3 (id:60001) has been deleted

[kozy, 2016-09-30 02:14:40]

kozyatinskiy@chromium.org changed reviewers:
+ alph@chromium.org

[kozy, 2016-09-30 02:14:40]

Alexey, please take a look!

[dgozman, 2016-09-30 16:45:30]

lgtm

https://codereview.chromium.org/2381693002/diff/80001/test/inspector/protocol...
File test/inspector/protocol-test.js (right):

https://codereview.chromium.org/2381693002/diff/80001/test/inspector/protocol...
test/inspector/protocol-test.js:138: message.id = 0;
We can also zero-out objectId's inside the message. This will make it reusable
in more places.

https://codereview.chromium.org/2381693002/diff/80001/test/inspector/runtime/...
File test/inspector/runtime/evaluate-async-with-wrap-error.js (right):

https://codereview.chromium.org/2381693002/diff/80001/test/inspector/runtime/...
test/inspector/runtime/evaluate-async-with-wrap-error.js:5: // Test that
Runtime.evaluate correctly process errors during wrapping async
- print it
- typo: async async

[alph, 2016-09-30 21:43:54]

lgtm

https://codereview.chromium.org/2381693002/diff/80001/test/inspector/runtime/...
File test/inspector/runtime/evaluate-async-with-wrap-error.js (right):

https://codereview.chromium.org/2381693002/diff/80001/test/inspector/runtime/...
test/inspector/runtime/evaluate-async-with-wrap-error.js:14: .then((message) =>
InspectorTest.logMessage(message))
nit: you don't need wrap a single argument in ()

[kozy, 2016-10-03 01:34:03]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2016-10-03 01:34:06]

The CQ bit was unchecked by kozyatinskiy@chromium.org

[kozy, 2016-10-03 15:04:09]

All done.

https://codereview.chromium.org/2381693002/diff/80001/test/inspector/protocol...
File test/inspector/protocol-test.js (right):

https://codereview.chromium.org/2381693002/diff/80001/test/inspector/protocol...
test/inspector/protocol-test.js:138: message.id = 0;
On 2016/09/30 16:45:30, dgozman wrote:
> We can also zero-out objectId's inside the message. This will make it reusable
> in more places.

Done.

https://codereview.chromium.org/2381693002/diff/80001/test/inspector/runtime/...
File test/inspector/runtime/evaluate-async-with-wrap-error.js (right):

https://codereview.chromium.org/2381693002/diff/80001/test/inspector/runtime/...
test/inspector/runtime/evaluate-async-with-wrap-error.js:5: // Test that
Runtime.evaluate correctly process errors during wrapping async
On 2016/09/30 16:45:30, dgozman wrote:
> - print it
> - typo: async async

Done.

https://codereview.chromium.org/2381693002/diff/80001/test/inspector/runtime/...
test/inspector/runtime/evaluate-async-with-wrap-error.js:14: .then((message) =>
InspectorTest.logMessage(message))
On 2016/09/30 21:43:54, alph wrote:
> nit: you don't need wrap a single argument in ()

Done.

[kozy, 2016-10-03 15:04:42]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2016-10-03 15:04:43]

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

[commit-bot: I haz the power, 2016-10-03 15:04:48]

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

[kozy, 2016-10-03 15:22:42]

The CQ bit was unchecked by kozyatinskiy@chromium.org

[kozy, 2016-10-03 15:23:57]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2016-10-03 15:23:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, alph@chromium.org
Link to the patchset: https://codereview.chromium.org/2381693002/#ps120001
(title: "a")

[commit-bot: I haz the power, 2016-10-03 15:24:12]

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-10-03 15:51:19]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-10-03 15:51:37]

Description was changed from

==========
[inspector] added a test for crash in wrapping async evaluate result

BUG=chromium:651211
R=dgozman@chromium.org
==========

to

==========
[inspector] added a test for crash in wrapping async evaluate result

BUG=chromium:651211
R=dgozman@chromium.org

Committed: https://crrev.com/c9391d15ca7fd2f80d0f907c39f2765a20af8749
Cr-Commit-Position: refs/heads/master@{#39933}
==========

[commit-bot: I haz the power, 2016-10-03 15:51:38]

Patchset 5 (id:??) landed as
https://crrev.com/c9391d15ca7fd2f80d0f907c39f2765a20af8749
Cr-Commit-Position: refs/heads/master@{#39933}