[DevTools] Move wrapCallFrames from InjectedScriptSource.js to native

[DevTools] Move wrapCallFrames from InjectedScriptSource.js to native

BUG=595206
R=dgozman@chromium.org,pfeldman@chromium.org

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

[kozy, 2016-03-22 21:45:05]

Dmitry, please take a look!

[kozy, 2016-03-22 23:43:13]

Dmitry, I've removed V8JavaScriptCallFrame, please take a look!

[kozy, 2016-03-22 23:45:33]

Patchset #2 (id:20001) has been deleted

[dgozman, 2016-03-23 00:12:35]

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js (right):

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js:427: var
location = script.locationFromPosition(pos, true);
What a fix! Nice!

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js
(right):

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js:320: *
@param {!Array<!Object>} array
!Array<*>

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp
(right):

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:579:
*callFrames = currentCallFrames(errorString);
if error - return nothing?

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:682:
return createProtocolLocation(scriptId, actualLineNumber, actualColumnNumber);
buildLocation

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:758: if
(!errorString->isEmpty())
should we check newCallFrames instead?

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:1628: 
nit: extra blank line

[kozy, 2016-03-23 02:13:10]

All done. please take a look!

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js
(right):

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js:320: *
@param {!Array<!Object>} array
On 2016/03/23 00:12:35, dgozman wrote:
> !Array<*>

Done.

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp
(right):

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:579:
*callFrames = currentCallFrames(errorString);
On 2016/03/23 00:12:35, dgozman wrote:
> if error - return nothing?

Done.

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:682:
return createProtocolLocation(scriptId, actualLineNumber, actualColumnNumber);
On 2016/03/23 00:12:35, dgozman wrote:
> buildLocation

Done.

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:758: if
(!errorString->isEmpty())
On 2016/03/23 00:12:35, dgozman wrote:
> should we check newCallFrames instead?

Done.

https://codereview.chromium.org/1826623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:1628: 
On 2016/03/23 00:12:35, dgozman wrote:
> nit: extra blank line

Done.

[dgozman, 2016-03-23 18:46:31]

lgtm with comments

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js (right):

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js:497:
"lineNumber": functionLocation.line,
Previous code checked that there could be no functionLocation.

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js:498:
"columnNumber": functionLocation.column,
ditto

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js:573: "object":
lazyObject
details

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/JavaScriptCallFrame.h
(right):

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/JavaScriptCallFrame.h:56:
v8::Local<v8::Object> object() const;
details

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp
(right):

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:632: if
(!errorString->isEmpty())
Let's check the result value instead.

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:1423:
v8::HandleScope handles(injectedScript->isolate());
Let's extract isolate to a local var.

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:1436:
String16 callFrameId = "{\"ordinal\":" + String16::number(callFrameIndex) +
",\"injectedScriptId\":" + String16::number(injectedScript->contextId()) + "}";
Move this serialization to a static method on RemoteCallFrameId.

[kozy, 2016-03-23 19:15:11]

All done!

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js (right):

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js:497:
"lineNumber": functionLocation.line,
On 2016/03/23 18:46:31, dgozman wrote:
> Previous code checked that there could be no functionLocation.

Done.

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js:498:
"columnNumber": functionLocation.column,
On 2016/03/23 18:46:31, dgozman wrote:
> ditto

Done.

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js:573: "object":
lazyObject
On 2016/03/23 18:46:31, dgozman wrote:
> details

Done.

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/JavaScriptCallFrame.h
(right):

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/JavaScriptCallFrame.h:56:
v8::Local<v8::Object> object() const;
On 2016/03/23 18:46:31, dgozman wrote:
> details

Done.

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp
(right):

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:632: if
(!errorString->isEmpty())
On 2016/03/23 18:46:31, dgozman wrote:
> Let's check the result value instead.

Done.

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:1423:
v8::HandleScope handles(injectedScript->isolate());
On 2016/03/23 18:46:31, dgozman wrote:
> Let's extract isolate to a local var.

Done.

https://codereview.chromium.org/1826623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:1436:
String16 callFrameId = "{\"ordinal\":" + String16::number(callFrameIndex) +
",\"injectedScriptId\":" + String16::number(injectedScript->contextId()) + "}";
On 2016/03/23 18:46:31, dgozman wrote:
> Move this serialization to a static method on RemoteCallFrameId.

Done.

[kozy, 2016-03-23 19:15:18]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2016-03-23 19:15:18]

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

[commit-bot: I haz the power, 2016-03-23 19:15:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1826623002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1826623002/80001

[commit-bot: I haz the power, 2016-03-23 19:27:35]

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

[commit-bot: I haz the power, 2016-03-23 19:27:36]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[kozy, 2016-03-23 20:22:36]

Description was changed from

==========
[DevTools] Move wrapCallFrames from InjectedScriptSource.js to native

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

to

==========
[DevTools] Move wrapCallFrames from InjectedScriptSource.js to native

BUG=595206
R=dgozman@chromium.org,pfeldman@chromium.org
==========

[kozy, 2016-03-23 20:22:36]

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

[kozy, 2016-03-23 20:22:59]

Pavel, please take a look!
I need owner lgtm for third_party/WebKit/Source/platform/blink_platform.gypi

[pfeldman, 2016-03-23 20:27:17]

lgtm

[kozy, 2016-03-23 20:32:01]

The CQ bit was checked by kozyatinskiy@chromium.org

[commit-bot: I haz the power, 2016-03-23 20:32:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1826623002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1826623002/80001

[commit-bot: I haz the power, 2016-03-23 20:42:14]

Description was changed from

==========
[DevTools] Move wrapCallFrames from InjectedScriptSource.js to native

BUG=595206
R=dgozman@chromium.org,pfeldman@chromium.org
==========

to

==========
[DevTools] Move wrapCallFrames from InjectedScriptSource.js to native

BUG=595206
R=dgozman@chromium.org,pfeldman@chromium.org
==========

[commit-bot: I haz the power, 2016-03-23 20:42:15]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-03-23 20:44:16]

Description was changed from

==========
[DevTools] Move wrapCallFrames from InjectedScriptSource.js to native

BUG=595206
R=dgozman@chromium.org,pfeldman@chromium.org
==========

to

==========
[DevTools] Move wrapCallFrames from InjectedScriptSource.js to native

BUG=595206
R=dgozman@chromium.org,pfeldman@chromium.org

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

[commit-bot: I haz the power, 2016-03-23 20:44:17]

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