On 2014/06/03 08:12:46, Yang wrote:
https://codereview.chromium.org/296953005/diff/40001/src/debug-debugger.js
File src/debug-debugger.js (right):
https://codereview.chromium.org/296953005/diff/40001/src/debug-debugger.js#newcode491
src/debug-debugger.js:491: return %DebugGetLoadedScripts();
On 2014/06/03 08:07:21, yurys wrote:
> This change looks wrong to me. By design of V8 debugging protocol mirror
cache
> should be kept while we are staying on a breakpoint so that the client
could
> access corresponding object by its mirror id. After this change,
however,
the
> cache will be cleared on any call to GetLoadedScripts() and the remote
id
will
> become invalid.
>
> In case of blink the problem is that we create mirror objects not only
when
we
> stay on a breakpoint and I'm not sure we clear them properly. Also we
don't
use
> id->mirror map in blink and I believe a right way to fix this would be
to
> disable mirror cache in blink completely. WDYT?
Not sure I understand. Which code path does blink use to get loaded
scripts?
Blink adds a bunch of convenience methods into the debug context all of
which
are defined in DebuggerScript.js [1] and they leverage mirror
infrastructure to
inspected user objects (one important difference compared to v8 debugger as
I
said is that we never need to resolve id into morror object so the cache is
useless in case of Blink). To collect current scripts
PageScriptDebugServer.cpp
calls [2] DebuggerScript.getScripts() which in turn calls Debug.scripts()
[3]
but all of this happens without entering debugger as simple function call
using
v8 api. As we don't enter debugger in that case ClearMirrorCache may never
be
called. This might be a bug in this particular case and we should use
v8::Debug::Call for invoking DebuggerScript.getScripts() to make sure
EnterDebugger is called and mirror cache is cleared after the call. But in
general to avoid bugs like this we could skip mirror cache for the mirrors
created by Blink.
[1]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/bindings/v8/DebuggerScript.js
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/bindings/v8/PageScriptDebugServer.cpp&q=PageScriptDebugServer.cpp&sq=package:chromium&type=cs&l=132
[3]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/bindings/v8/DebuggerScript.js&q=getScripts&sq=package:chromium&type=cs&l=132
https://codereview.chromium.org/296953005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
