This patch is the propagation version of https://codereview.chromium.org/10824032 patch

Improve the JitCodeEventHandler API to add support for line position information. 

This includes:

* adding the CODE_ADD_LINE_POS_INFO, CODE_START_LINE_INFO_RECORDING, CODE_END_LINE_INFO_RECORDING event and the corresponding functionality.
 * adding the JITCodeLineInfo struct to record the code line info. I added this definition because Danno mentioned that "we'd like to cleanup and decouple the external debugging functionality"
 * some other small changes.

Committed: https://code.google.com/p/v8/source/detail?r=13686

[chunyang.dai, 2013-02-06 10:16:00]

hello. Daniel. The original CL 
https://codereview.chromium.org/11552033 
can not be accessed. I don't know the reason. So I create
this CL. Please help to review. 

As far as the "code move" support. My plan is that currently we use the
"--nocompact_code_space" flag to disable it. I will submit another patch to
support it so that the current patch can be accepted in time.
Is it ok?

[danno, 2013-02-06 14:21:34]

Thanks for the new patch set. It's getting closer, but there are still a handful
of mostly minor issues that you need to address.

https://codereview.chromium.org/12223027/diff/7001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/12223027/diff/7001/include/v8.h#newcode3051
include/v8.h:3051: // RelocInfo::kNoPosition field.
This comment refers to internal implementation details (RelocInfo::kNoPosition)
in a public API. If you need to clarify the semantic of this type, please
describe it here completely rather than referring to an internal implementation
detail.

https://codereview.chromium.org/12223027/diff/7001/include/v8.h#newcode3066
include/v8.h:3066: // for CODE_ADD_LINE_POS_INFO and
CODE_END_LINE_INFO_RECORDING event
More explanation is needed here, since it's not just for "other events". Right
now, this is just used for *_LINE_INFO_* events, and the semantic (the value
returned from the START_LINE_INFO_RECORDING event in this field is passed to
subsequent CODE_START_LINE_INFO_RECORDING and  CODE_END_LINE_INFO_RECORDING
events) should be captured in the documentation.

https://codereview.chromium.org/12223027/diff/7001/src/assembler.cc
File src/assembler.cc (right):

https://codereview.chromium.org/12223027/diff/7001/src/assembler.cc#newcode1527
src/assembler.cc:1527: assembler_->pc_offset(),
why assembler_ rather than assembler()?

https://codereview.chromium.org/12223027/diff/7001/src/assembler.cc#newcode1544
src/assembler.cc:1544: assembler_->pc_offset(),
why assembler_ rather than assembler()?

https://codereview.chromium.org/12223027/diff/7001/src/assembler.h
File src/assembler.h (right):

https://codereview.chromium.org/12223027/diff/7001/src/assembler.h#newcode891
src/assembler.h:891: Assembler* assembler() {
Why is this necessary? You access directly assembler_ directly elsewhere in this
CL, and I don't see uses of assembler()?

https://codereview.chromium.org/12223027/diff/7001/src/log.cc
File src/log.cc (right):

https://codereview.chromium.org/12223027/diff/7001/src/log.cc#newcode479
src/log.cc:479: event.script =
v8::Handle<v8::Script>(reinterpret_cast<v8::Script*>(script));
Why don't you use "<Script>(ToApi<Script>(script))"? You will have to pass in
the script as an internal handle to use this, though.

https://codereview.chromium.org/12223027/diff/7001/src/log.cc#newcode534
src/log.cc:534: JitCodeEvent event;
How about making sure the event is a well-defined state before the call (memset
0)? Or at fill it with kZapValue here and elsewhere when "#if DEBUG" is true.

https://codereview.chromium.org/12223027/diff/7001/src/log.cc#newcode547
src/log.cc:547: code_event_handler_(&event);
same here

https://codereview.chromium.org/12223027/diff/7001/src/log.h
File src/log.h (right):

https://codereview.chromium.org/12223027/diff/7001/src/log.h#newcode250
src/log.h:250: // Emits a code line info add event with Postion type .
nit: remove space before .

https://codereview.chromium.org/12223027/diff/7001/src/log.h#newcode261
src/log.h:261: // It's the callee's responsibility to dispose the parameter
line_info data.
There is no "line_info data" parameter?

https://codereview.chromium.org/12223027/diff/7001/src/log.h#newcode381
src/log.h:381: Script* script,
Should be a Handle<Script>

https://codereview.chromium.org/12223027/diff/7001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/12223027/diff/7001/test/cctest/test-api.cc#ne...
test/cctest/test-api.cc:11690: i::ComputePointerHash(line_info),
strange indentation, please follow style guidelines. The presubmit script in the
tools directory should find style problems like this one.

[chunyang.dai, 2013-02-07 13:18:51]

Hello. Daniel.
please review this patch.

[danno, 2013-02-11 15:17:56]

Very close. If you address my last comment (sorry, I realize it deviates
slightly from my last round of comments, I apologize for that), then I think
this is ready to land.

https://codereview.chromium.org/12223027/diff/13003/src/log.h
File src/log.h (right):

https://codereview.chromium.org/12223027/diff/13003/src/log.h#newcode381
src/log.h:381: Handle<Script> script,
Sorry, I think I gave you slightly misleading advice in the last set of
comments. It does appear that the Issue APIs take "naked" object pointers. So do
indeed pass a Script* here (sorry!!) for consistency with the other issue API.
However, still clean up how the Handle is cobbled together in the implementation
of IssueCodeAddedEvent. Please move the logic that you now have at the call
sites of IssueCodeAddedEvent that creates a Handle<Script> into
IssueCodeAddedEvent to make the handle right before you do the ToApi call.

[chunyang.dai, 2013-02-12 17:28:31]

Hello, Daniel.
Sorry for the late response because of our Spring Festival holiday.
Thanks for comments and I updated the code. I hope this this It will be ok. :)

Please help to review it.
Thanks.

[danno, 2013-02-13 10:23:30]

lgtm

[danno, 2013-02-13 10:23:30]

lgtm

lgtm

[danno, 2013-02-13 10:24:03]

I'll land this for you as soon as the V8 tree goes green again.

[chunyang.dai, 2013-02-18 14:13:49]

hello, Daniel.
thanks for your help. 
Could you please continue to help another patch 
https://codereview.chromium.org/11574031.  
it's another component of this feature.

thanks.

[danno, 2013-02-18 14:39:29]

I will land this patch for you, but I just noticed that it doesn't apply on V8
bleeding_edge. Could you please rebase on bleeding_edge and re-upload?

[chunyang.dai, 2013-02-18 15:36:38]

Hello, Daniel.
I rebased it. 
thanks for your help.