[inspector] migrate Debugger to new style

[inspector] migrate Debugger to new style

BUG=none
R=dgozman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[kozy, 2016-11-02 02:20:27]

Dmitry, please take a look.

[kozy, 2016-11-02 02:20:49]

Description was changed from

==========
[inspector] migrate Debugger to new style

BUG=none
R=dgozman@chromium.org
==========

to

==========
[inspector] migrate Debugger to new style

BUG=none
R=dgozman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[kozy, 2016-11-04 00:34:49]

The CQ bit was checked by kozyatinskiy@chromium.org

[commit-bot: I haz the power, 2016-11-04 00:34: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-11-04 00:34:52]

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

[commit-bot: I haz the power, 2016-11-04 00:34:53]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-v8-committers".
Note that this has nothing to do with OWNERS files.

[kozy, 2016-11-04 00:35:14]

The CQ bit was unchecked by kozyatinskiy@chromium.org

[kozy, 2016-11-04 01:04:57]

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

[commit-bot: I haz the power, 2016-11-04 01:04:59]

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

[kozy, 2016-11-04 01:05:05]

please take a look!

[dgozman, 2016-11-04 01:49:04]

lgtm

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
File src/inspector/v8-debugger-agent-impl.cc (right):

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
src/inspector/v8-debugger-agent-impl.cc:212: CHECK(response.isSuccess());
Just ignore the result?

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
src/inspector/v8-debugger-agent-impl.cc:302: static void parseLocation(
Just remove this method.

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
src/inspector/v8-debugger-agent-impl.cc:522: if (!response.isSuccess() ||
compileError) return response;
If there was a compile error, don't you want to return something else in
response?

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
src/inspector/v8-debugger-agent-impl.cc:554: return Response::Error("Internal
error");
InternalError()

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
File src/inspector/v8-debugger.cc (left):

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
src/inspector/v8-debugger.cc:375: return false;
This means *compileError = true

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
File src/inspector/v8-debugger.cc (right):

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
src/inspector/v8-debugger.cc:426: return Response::Error("Unknown error.");
InternalError

[commit-bot: I haz the power, 2016-11-04 02:01:24]

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

[commit-bot: I haz the power, 2016-11-04 02:01:25]

Dry run: This issue passed the CQ dry run.

[kozy, 2016-11-04 14:33:45]

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

[commit-bot: I haz the power, 2016-11-04 14:33:54]

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

[kozy, 2016-11-04 14:33:57]

All done, thank you!

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
File src/inspector/v8-debugger-agent-impl.cc (right):

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
src/inspector/v8-debugger-agent-impl.cc:212: CHECK(response.isSuccess());
On 2016/11/04 01:49:04, dgozman wrote:
> Just ignore the result?

Done.

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
src/inspector/v8-debugger-agent-impl.cc:302: static void parseLocation(
On 2016/11/04 01:49:04, dgozman wrote:
> Just remove this method.

Done.

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
src/inspector/v8-debugger-agent-impl.cc:522: if (!response.isSuccess() ||
compileError) return response;
On 2016/11/04 01:49:04, dgozman wrote:
> If there was a compile error, don't you want to return something else in
> response?

No, I'll return exception details in this case and Response::OK().

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
src/inspector/v8-debugger-agent-impl.cc:554: return Response::Error("Internal
error");
On 2016/11/04 01:49:04, dgozman wrote:
> InternalError()

Done.

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
File src/inspector/v8-debugger.cc (left):

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
src/inspector/v8-debugger.cc:375: return false;
On 2016/11/04 01:49:04, dgozman wrote:
> This means *compileError = true

No, it means compileError = false and Response::error().

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
File src/inspector/v8-debugger.cc (right):

https://codereview.chromium.org/2471583003/diff/60001/src/inspector/v8-debugg...
src/inspector/v8-debugger.cc:426: return Response::Error("Unknown error.");
On 2016/11/04 01:49:04, dgozman wrote:
> InternalError

Done.

[commit-bot: I haz the power, 2016-11-04 15:28:17]

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

[commit-bot: I haz the power, 2016-11-04 15:28:18]

Dry run: This issue passed the CQ dry run.

[kozy, 2016-11-04 19:41:08]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2016-11-04 19:41:09]

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/2471583003/#ps80001
(title: "addressed comments")

[commit-bot: I haz the power, 2016-11-04 19:41:15]

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-11-04 19:43:15]

Description was changed from

==========
[inspector] migrate Debugger to new style

BUG=none
R=dgozman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
[inspector] migrate Debugger to new style

BUG=none
R=dgozman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-11-04 19:43:17]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-17 22:23:44]

Description was changed from

==========
[inspector] migrate Debugger to new style

BUG=none
R=dgozman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
[inspector] migrate Debugger to new style

BUG=none
R=dgozman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[commit-bot: I haz the power, 2016-11-17 22:23:45]

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