[inspector] don't report frames for v8-debugger internal scripts

[inspector] don't report frames for v8-debugger internal scripts

We don't report scriptParsed for internal debugger scripts. Call frames should be filter out too.
Example of v8-debugger internal scripts:
- injected-script.js,
- debugger-script.js,
- temporary script to evaluate function definition in callFunctionOn protocol method,
...

BUG=none
R=yangguo@chromium.org

[kozy, 2016-11-15 02:24:08]

Yang, please take a look.

[kozy, 2016-11-15 02:25:54]

Maybe we can mark v8-debugger internal scripts as native and don't add new
condition to filter frames out, but I'm worried about code compilation, it looks
like IsBuiltin function is used in code generators.

[kozy, 2016-11-15 02:25:55]

Maybe we can mark v8-debugger internal scripts as native and don't add new
condition to filter frames out, but I'm worried about code compilation, it looks
like IsBuiltin function is used in code generators.

[Yang, 2016-11-15 08:15:47]

https://codereview.chromium.org/2499093003/diff/1/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/2499093003/diff/1/src/isolate.cc#newcode780
src/isolate.cc:780: }
Please change StackTraceFrameIterator::IsValidFrame to use
SharedFunctionInfo::IsSubjectToDebugging instead. Thanks.

https://codereview.chromium.org/2499093003/diff/1/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/2499093003/diff/1/src/objects-inl.h#newcode6510
src/objects-inl.h:6510: bool result = !IsBuiltin() && !HasAsmWasmData();
Can we combine these two lines to

if (IsBuiltin() || HasAsmWasmData()) return false;

https://codereview.chromium.org/2499093003/diff/1/src/objects-inl.h#newcode6515
src/objects-inl.h:6515: return
!script->origin_options().IsEmbedderDebugScript();
I think we should get rid of this origin_option, and instead set introduce a new
script type. IsBuiltin would then return true. Other places that check for
IsEmbedderDebugScript would also check for the script type instead.


btw, in V8Debugger::getCompiledScripts, do you care about getting V8's native
scripts? If not, we could use IsSubjectToDebugging there as well.

[Yang, 2016-11-22 09:25:45]

On 2016/11/15 08:15:47, Yang wrote:
> https://codereview.chromium.org/2499093003/diff/1/src/isolate.cc
> File src/isolate.cc (right):
> 
> https://codereview.chromium.org/2499093003/diff/1/src/isolate.cc#newcode780
> src/isolate.cc:780: }
> Please change StackTraceFrameIterator::IsValidFrame to use
> SharedFunctionInfo::IsSubjectToDebugging instead. Thanks.
> 
> https://codereview.chromium.org/2499093003/diff/1/src/objects-inl.h
> File src/objects-inl.h (right):
> 
>
https://codereview.chromium.org/2499093003/diff/1/src/objects-inl.h#newcode6510
> src/objects-inl.h:6510: bool result = !IsBuiltin() && !HasAsmWasmData();
> Can we combine these two lines to
> 
> if (IsBuiltin() || HasAsmWasmData()) return false;
> 
>
https://codereview.chromium.org/2499093003/diff/1/src/objects-inl.h#newcode6515
> src/objects-inl.h:6515: return
> !script->origin_options().IsEmbedderDebugScript();
> I think we should get rid of this origin_option, and instead set introduce a
new
> script type. IsBuiltin would then return true. Other places that check for
> IsEmbedderDebugScript would also check for the script type instead.
> 
> 
> btw, in V8Debugger::getCompiledScripts, do you care about getting V8's native
> scripts? If not, we could use IsSubjectToDebugging there as well.

Can we close this CL?