Fix uncaught exception bug from liveEditScriptSource

Fix uncaught exception bug from liveEditScriptSource

BUG=chromium:683335
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2651153004
Cr-Commit-Position: refs/heads/master@{#42672}
Committed: https://chromium.googlesource.com/v8/v8/+/a81fbe802a605a4f098b2df00e9d60eabeac5168

[luoe, 2017-01-25 23:22:36]

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

[luoe, 2017-01-25 23:22:37]

Please take a look

[kozy, 2017-01-25 23:35:07]

Let me share best techniques for inspector tests with you if you don't mind.

https://codereview.chromium.org/2651153004/diff/1/test/inspector/debugger/set...
File test/inspector/debugger/set-script-source-exception-expected.txt (right):

https://codereview.chromium.org/2651153004/diff/1/test/inspector/debugger/set...
test/inspector/debugger/set-script-source-exception-expected.txt:1: a
Please upload correct expectations :)

https://codereview.chromium.org/2651153004/diff/1/test/inspector/debugger/set...
File test/inspector/debugger/set-script-source-exception.js (right):

https://codereview.chromium.org/2651153004/diff/1/test/inspector/debugger/set...
test/inspector/debugger/set-script-source-exception.js:8:
Protocol.Debugger.onScriptParsed(message =>
{runTests(message.params.scriptId)});
Protocol.Debugger.onceScriptParsed().then(message =>
runTests(message.params.scriptId));

https://codereview.chromium.org/2651153004/diff/1/test/inspector/debugger/set...
test/inspector/debugger/set-script-source-exception.js:9:
InspectorTest.addScript(`function foo() {}`);
No reason to use `` for single line source, please use '.

https://codereview.chromium.org/2651153004/diff/1/test/inspector/debugger/set...
test/inspector/debugger/set-script-source-exception.js:11: function
runTests(scriptId) {
Let's rewrite it a little:

InspectorTest.runTestSuite([
  function testIncorrectScriptId(next) {
    Protocol.Debugger.setScriptSource({ scriptId: '-1', scriptSource: '0')
      .then(InspectorTest.logMessage) // it's protocol test so let's just dump
protocol output without any formatting
      .then(next);
  },

  function testSourceWithSyntaxError(next) {
    Protocol.Debugger.onceScriptParsed()
      .then(message => Protocol.Debugger.setScriptSource({ scriptId:
message.params.scriptId, scriptSource: 'a # b'})
      .then(InspectorTest.logMessage)
      .then(next);
    InspectorTest.addScript('function foo() {}'); // will issue scriptParsed
event and resolve Protocol.Debugger.onceScriptParsed promise
  }

]);

https://codereview.chromium.org/2651153004/diff/1/test/inspector/debugger/set...
test/inspector/debugger/set-script-source-exception.js:20: if (message.result &&
message.result.exceptionDetails) {
Since it's protocol test we prefer to just dump protocol output without any
formatting.

[luoe, 2017-01-25 23:48:54]

Your test looks a lot cleaner, thank you

https://codereview.chromium.org/2651153004/diff/1/test/inspector/debugger/set...
File test/inspector/debugger/set-script-source-exception.js (right):

https://codereview.chromium.org/2651153004/diff/1/test/inspector/debugger/set...
test/inspector/debugger/set-script-source-exception.js:8:
Protocol.Debugger.onScriptParsed(message =>
{runTests(message.params.scriptId)});
On 2017/01/25 23:35:07, kozy wrote:
> Protocol.Debugger.onceScriptParsed().then(message =>
> runTests(message.params.scriptId));

Done.

https://codereview.chromium.org/2651153004/diff/1/test/inspector/debugger/set...
test/inspector/debugger/set-script-source-exception.js:9:
InspectorTest.addScript(`function foo() {}`);
On 2017/01/25 23:35:07, kozy wrote:
> No reason to use `` for single line source, please use '.

Done.

https://codereview.chromium.org/2651153004/diff/1/test/inspector/debugger/set...
test/inspector/debugger/set-script-source-exception.js:11: function
runTests(scriptId) {
On 2017/01/25 23:35:07, kozy wrote:
> Let's rewrite it a little:
> 
> InspectorTest.runTestSuite([
>   function testIncorrectScriptId(next) {
>     Protocol.Debugger.setScriptSource({ scriptId: '-1', scriptSource: '0')
>       .then(InspectorTest.logMessage) // it's protocol test so let's just dump
> protocol output without any formatting
>       .then(next);
>   },
> 
>   function testSourceWithSyntaxError(next) {
>     Protocol.Debugger.onceScriptParsed()
>       .then(message => Protocol.Debugger.setScriptSource({ scriptId:
> message.params.scriptId, scriptSource: 'a # b'})
>       .then(InspectorTest.logMessage)
>       .then(next);
>     InspectorTest.addScript('function foo() {}'); // will issue scriptParsed
> event and resolve Protocol.Debugger.onceScriptParsed promise
>   }
> 
> ]);
> 

Done.

https://codereview.chromium.org/2651153004/diff/1/test/inspector/debugger/set...
test/inspector/debugger/set-script-source-exception.js:20: if (message.result &&
message.result.exceptionDetails) {
On 2017/01/25 23:35:07, kozy wrote:
> Since it's protocol test we prefer to just dump protocol output without any
> formatting.

Done.

[kozy, 2017-01-25 23:50:55]

thank you!

lgtm.

[luoe, 2017-01-25 23:53:30]

Description was changed from

==========
Fix uncaught exception bug from liveEditScriptSource

BUG=chromium:683335
==========

to

==========
Fix uncaught exception bug from liveEditScriptSource

BUG=chromium:683335
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[luoe, 2017-01-25 23:53:35]

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

[commit-bot: I haz the power, 2017-01-25 23:53:47]

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-25 23:54:36]

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

[commit-bot: I haz the power, 2017-01-25 23:54:40]

Dry run: Try jobs failed on following builders:
  linux_precise_blink_rel on master.tryserver.blink (JOB_FAILED, no build URL)

[luoe, 2017-01-26 00:15:25]

Description was changed from

==========
Fix uncaught exception bug from liveEditScriptSource

BUG=chromium:683335
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Fix uncaught exception bug from liveEditScriptSource

BUG=chromium:683335
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[luoe, 2017-01-26 01:27:04]

The CQ bit was checked by luoe@chromium.org

[commit-bot: I haz the power, 2017-01-26 01:27:08]

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-26 01:29:01]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485394024160330,
"parent_rev": "c6f925d7ed002e4bbd57a674cf90627384c81964", "commit_rev":
"a81fbe802a605a4f098b2df00e9d60eabeac5168"}

[commit-bot: I haz the power, 2017-01-26 01:29:05]

Description was changed from

==========
Fix uncaught exception bug from liveEditScriptSource

BUG=chromium:683335
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Fix uncaught exception bug from liveEditScriptSource

BUG=chromium:683335
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2651153004
Cr-Commit-Position: refs/heads/master@{#42672}
Committed:
https://chromium.googlesource.com/v8/v8/+/a81fbe802a605a4f098b2df00e9d60eabea...
==========

[commit-bot: I haz the power, 2017-01-26 01:29:06]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/v8/v8/+/a81fbe802a605a4f098b2df00e9d60eabea...