[inspector] migrate Runtime to new style

[inspector] migrate Runtime to new style

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

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

[kozy, 2016-11-02 01:28:20]

Dmitry, please take a look!

[kozy, 2016-11-02 01:52:06]

Description was changed from

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

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

to

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

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

[kozy, 2016-11-02 01:52:11]

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

[commit-bot: I haz the power, 2016-11-02 01:52:21]

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

[dgozman, 2016-11-02 19:54:00]

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
File src/inspector/injected-script.cc (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:150: toProtocolValue(context, resultValue,
1000, protocolValue);
&protocolValue

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:187:
std::unique_ptr<protocol::Runtime::RemoteObject> remoteObject =
*result = ....

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:235: result = v8::Local<v8::Value>();
remove

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:243: v8::Local<v8::Value> r =
function.call(hadException);
*result =

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:309: v8::Local<v8::Value>& result) {
v8::Local<>*

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:321: response = findObject(*remoteObjectId,
&object);
&object -> result

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:335: .ToLocal(&object)) {
.ToLocal(result)

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:353:
std::unique_ptr<protocol::Runtime::ExceptionDetails> exceptionDetails =
*result = ...

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:400: generatePreview, remoteObject);
remoteObject -> result

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:410: generatePreview &&
!exception->IsNativeError(), remoteObject);
ditto

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
File src/inspector/injected-script.h (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.h:72:
std::unique_ptr<protocol::Runtime::RemoteObject>& result) const;
unique_ptr<>*

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.h:94: Maybe<protocol::Runtime::ExceptionDetails>&
result);
Maybe<>*

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/remote-ob...
File src/inspector/remote-object-id.h (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/remote-ob...
src/inspector/remote-object-id.h:30: static Response parse(const String16&,
std::unique_ptr<RemoteObjectId>&);
& -> *

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/remote-ob...
src/inspector/remote-object-id.h:42: static Response parse(const String16&,
std::unique_ptr<RemoteCallFrameId>&);
ditto

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-consol...
File src/inspector/v8-console.cc (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-consol...
src/inspector/v8-console.cc:624: if (!wrappedObject) return;
Check the response instead.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-inspec...
File src/inspector/v8-inspector-session-impl.cc (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-inspec...
src/inspector/v8-inspector-session-impl.cc:197: if (!objectId) return
Response::OK();
This behavior is strange...

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-inspec...
src/inspector/v8-inspector-session-impl.cc:231: if (!response.isSuccess() &&
error) {
If error is null, you still have to return false.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-runtim...
File src/inspector/v8-runtime-agent-impl.cc (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-runtim...
src/inspector/v8-runtime-agent-impl.cc:234: Maybe<int> executionContextId, int&
contextId) {
int* contextId

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-value-...
File src/inspector/v8-value-copier.h (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-value-...
src/inspector/v8-value-copier.h:25: int maxDepth,
Let's not pass maxDepth ever and use a constant in .cc

[kozy, 2016-11-02 22:45:13]

All done, please take a loon!

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
File src/inspector/injected-script.cc (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:150: toProtocolValue(context, resultValue,
1000, protocolValue);
On 2016/11/02 19:54:00, dgozman wrote:
> &protocolValue

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:187:
std::unique_ptr<protocol::Runtime::RemoteObject> remoteObject =
On 2016/11/02 19:53:59, dgozman wrote:
> *result = ....

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:235: result = v8::Local<v8::Value>();
On 2016/11/02 19:53:59, dgozman wrote:
> remove

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:243: v8::Local<v8::Value> r =
function.call(hadException);
On 2016/11/02 19:53:59, dgozman wrote:
> *result =

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:309: v8::Local<v8::Value>& result) {
On 2016/11/02 19:54:00, dgozman wrote:
> v8::Local<>*

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:321: response = findObject(*remoteObjectId,
&object);
On 2016/11/02 19:53:59, dgozman wrote:
> &object -> result

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:335: .ToLocal(&object)) {
On 2016/11/02 19:53:59, dgozman wrote:
> .ToLocal(result)

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:353:
std::unique_ptr<protocol::Runtime::ExceptionDetails> exceptionDetails =
On 2016/11/02 19:53:59, dgozman wrote:
> *result = ...
I preferred to store it in separate variable since using
unique_ptr<ExceptionDetails> is easier then using Maybe.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:400: generatePreview, remoteObject);
On 2016/11/02 19:53:59, dgozman wrote:
> remoteObject -> result

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.cc:410: generatePreview &&
!exception->IsNativeError(), remoteObject);
On 2016/11/02 19:53:59, dgozman wrote:
> ditto

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
File src/inspector/injected-script.h (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.h:72:
std::unique_ptr<protocol::Runtime::RemoteObject>& result) const;
On 2016/11/02 19:54:00, dgozman wrote:
> unique_ptr<>*

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/injected-...
src/inspector/injected-script.h:94: Maybe<protocol::Runtime::ExceptionDetails>&
result);
On 2016/11/02 19:54:00, dgozman wrote:
> Maybe<>*

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/remote-ob...
File src/inspector/remote-object-id.h (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/remote-ob...
src/inspector/remote-object-id.h:30: static Response parse(const String16&,
std::unique_ptr<RemoteObjectId>&);
On 2016/11/02 19:54:00, dgozman wrote:
> & -> *

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/remote-ob...
src/inspector/remote-object-id.h:42: static Response parse(const String16&,
std::unique_ptr<RemoteCallFrameId>&);
On 2016/11/02 19:54:00, dgozman wrote:
> ditto

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-consol...
File src/inspector/v8-console.cc (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-consol...
src/inspector/v8-console.cc:624: if (!wrappedObject) return;
On 2016/11/02 19:54:00, dgozman wrote:
> Check the response instead.

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-inspec...
File src/inspector/v8-inspector-session-impl.cc (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-inspec...
src/inspector/v8-inspector-session-impl.cc:197: if (!objectId) return
Response::OK();
On 2016/11/02 19:54:00, dgozman wrote:
> This behavior is strange...

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-inspec...
src/inspector/v8-inspector-session-impl.cc:231: if (!response.isSuccess() &&
error) {
On 2016/11/02 19:54:00, dgozman wrote:
> If error is null, you still have to return false.

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-runtim...
File src/inspector/v8-runtime-agent-impl.cc (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-runtim...
src/inspector/v8-runtime-agent-impl.cc:234: Maybe<int> executionContextId, int&
contextId) {
On 2016/11/02 19:54:00, dgozman wrote:
> int* contextId

Done.

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-value-...
File src/inspector/v8-value-copier.h (right):

https://codereview.chromium.org/2467853003/diff/20001/src/inspector/v8-value-...
src/inspector/v8-value-copier.h:25: int maxDepth,
On 2016/11/02 19:54:00, dgozman wrote:
> Let's not pass maxDepth ever and use a constant in .cc

Done.

[dgozman, 2016-11-03 22:23:49]

lgtm

https://codereview.chromium.org/2467853003/diff/60001/src/inspector/v8-inspec...
File src/inspector/v8-inspector-session-impl.cc (right):

https://codereview.chromium.org/2467853003/diff/60001/src/inspector/v8-inspec...
src/inspector/v8-inspector-session-impl.cc:196: if (!objectId) return
Response::InternalError();
Let's just remove this line?

[kozy, 2016-11-03 22:26:20]

all done, thanks

https://codereview.chromium.org/2467853003/diff/60001/src/inspector/v8-inspec...
File src/inspector/v8-inspector-session-impl.cc (right):

https://codereview.chromium.org/2467853003/diff/60001/src/inspector/v8-inspec...
src/inspector/v8-inspector-session-impl.cc:196: if (!objectId) return
Response::InternalError();
On 2016/11/03 22:23:49, dgozman wrote:
> Let's just remove this line?

Done.

[kozy, 2016-11-03 22:26:26]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2016-11-03 22:26:27]

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

[commit-bot: I haz the power, 2016-11-03 22:26:30]

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-03 23:51:41]

Description was changed from

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

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

to

==========
[inspector] migrate Runtime 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-03 23:51:42]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-17 22:21:38]

Description was changed from

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

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

to

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

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

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

[commit-bot: I haz the power, 2016-11-17 22:21:40]

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