Model each Dart library as its own ScriptState when devtools are enabled.

Model each Dart library as its own ScriptState when devtools are enabled.

BUG=
R=vsm@google.com

Committed: https://src.chromium.org/viewvc/multivm?view=rev&revision=1406

[Jacob, 2013-09-28 01:48:10]

This CL unfortunately hits a number of files outside the Dart directory but it
has some big benefits.

Benefits:
You can evaluate expressions against any loaded Dart library in the Dartium
devtools.
You can browse the list of libraries loaded by the current Dart application in
the Dartium devtools UI using the same list reserved for Dart isolated worlds.

Feel free to stop by my desk for a demo which will put these changes in more
context.
I still need to add a test that verifies that the evaluate method works when not
at a breakpoint.
This change should also make it easy for the Dart editor to also trigger
evaluating expressions against arbitrary Dart libraries.

There are some ways I could push slightly more of the logic into inspector/dart.

[vsm, 2013-09-30 22:08:02]

First set of comments.

https://chromiumcodereview.appspot.com/24989007/diff/5001/Source/bindings/dar...
File Source/bindings/dart/DartController.cpp (right):

https://chromiumcodereview.appspot.com/24989007/diff/5001/Source/bindings/dar...
Source/bindings/dart/DartController.cpp:895: for (Vector<Dart_Isolate>::iterator
it = m_isolates.begin(); it != m_isolates.end(); ++it)
This will become just a single isolate.  

Note: non-dom isolates don't make it into this vector even now AFAIK.  This is
probably a good thing.  I assume we have no way of breaking in them either?

https://chromiumcodereview.appspot.com/24989007/diff/5001/Source/bindings/dar...
File Source/bindings/dart/DartController.h (right):

https://chromiumcodereview.appspot.com/24989007/diff/5001/Source/bindings/dar...
Source/bindings/dart/DartController.h:45: class DartScriptState;
nit: alpha

https://chromiumcodereview.appspot.com/24989007/diff/5001/Source/bindings/v8/...
File Source/bindings/v8/ScriptState.h (right):

https://chromiumcodereview.appspot.com/24989007/diff/5001/Source/bindings/v8/...
Source/bindings/v8/ScriptState.h:89: ~ScriptState();
This should now be virtual.

https://chromiumcodereview.appspot.com/24989007/diff/5001/Source/bindings/v8/...
File Source/bindings/v8/custom/V8InjectedScriptHostCustom.cpp (right):

https://chromiumcodereview.appspot.com/24989007/diff/5001/Source/bindings/v8/...
Source/bindings/v8/custom/V8InjectedScriptHostCustom.cpp:362: 
Can this code be refactored to be a call back into bindings/dart?

https://chromiumcodereview.appspot.com/24989007/diff/5001/Source/core/inspect...
File Source/core/inspector/PageRuntimeAgent.cpp (right):

https://chromiumcodereview.appspot.com/24989007/diff/5001/Source/core/inspect...
Source/core/inspector/PageRuntimeAgent.cpp:48: #include <dart_api.h>
We should really avoid including this outside of bindings/dart.

[Jacob, 2013-10-01 00:07:05]

https://codereview.chromium.org/24989007/diff/5001/Source/bindings/dart/DartC...
File Source/bindings/dart/DartController.cpp (right):

https://codereview.chromium.org/24989007/diff/5001/Source/bindings/dart/DartC...
Source/bindings/dart/DartController.cpp:895: for (Vector<Dart_Isolate>::iterator
it = m_isolates.begin(); it != m_isolates.end(); ++it)
On 2013/09/30 22:08:03, vsm wrote:
> This will become just a single isolate.  
> 
> Note: non-dom isolates don't make it into this vector even now AFAIK.  This is
> probably a good thing.  I assume we have no way of breaking in them either?

You may be able to break into non-DOM isolates at the moment.  DartDebugServer
looks like it has the plumbing to support it although it wouldn't surprise me if
things went horribly wrong.  I believe you can set breakpoints in workers and
evaluate code in worker contexts in the JS side of the world.

https://codereview.chromium.org/24989007/diff/5001/Source/bindings/dart/DartC...
File Source/bindings/dart/DartController.h (right):

https://codereview.chromium.org/24989007/diff/5001/Source/bindings/dart/DartC...
Source/bindings/dart/DartController.h:45: class DartScriptState;
On 2013/09/30 22:08:03, vsm wrote:
> nit: alpha

Done.

https://codereview.chromium.org/24989007/diff/5001/Source/bindings/v8/ScriptS...
File Source/bindings/v8/ScriptState.h (right):

https://codereview.chromium.org/24989007/diff/5001/Source/bindings/v8/ScriptS...
Source/bindings/v8/ScriptState.h:89: ~ScriptState();
On 2013/09/30 22:08:03, vsm wrote:
> This should now be virtual.

Done.

https://codereview.chromium.org/24989007/diff/5001/Source/bindings/v8/custom/...
File Source/bindings/v8/custom/V8InjectedScriptHostCustom.cpp (right):

https://codereview.chromium.org/24989007/diff/5001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8InjectedScriptHostCustom.cpp:362: 
On 2013/09/30 22:08:03, vsm wrote:
> Can this code be refactored to be a call back into bindings/dart?
Moved it to a new file called DartInjectedScriptHost.

https://codereview.chromium.org/24989007/diff/5001/Source/core/inspector/Page...
File Source/core/inspector/PageRuntimeAgent.cpp (right):

https://codereview.chromium.org/24989007/diff/5001/Source/core/inspector/Page...
Source/core/inspector/PageRuntimeAgent.cpp:48: #include <dart_api.h>
On 2013/09/30 22:08:03, vsm wrote:
> We should really avoid including this outside of bindings/dart.  

Worse... I didn't even use this import anymore. I wish blink had a lint tool for
unused C++ includes.

[vsm, 2013-10-03 17:59:30]

lgtm

https://chromiumcodereview.appspot.com/24989007/diff/14001/Source/bindings/v8...
File Source/bindings/v8/custom/V8InjectedScriptHostCustom.cpp (right):

https://chromiumcodereview.appspot.com/24989007/diff/14001/Source/bindings/v8...
Source/bindings/v8/custom/V8InjectedScriptHostCustom.cpp:358:
v8::Handle<v8::Value> dartResult =
DartInjectedScriptHost::evaluateIfDartContext(args.Holder(), expression);
It'd be nice to make this a virtual method on something, somehow.

https://chromiumcodereview.appspot.com/24989007/diff/14001/Source/core/inspec...
File Source/core/inspector/PageRuntimeAgent.cpp (right):

https://chromiumcodereview.appspot.com/24989007/diff/14001/Source/core/inspec...
Source/core/inspector/PageRuntimeAgent.cpp:184: {
Is the runtime type of v8ScriptState a ScriptState or a DartScriptState?  If the
latter, can we move code over there?

https://chromiumcodereview.appspot.com/24989007/diff/14001/Source/core/inspec...
Source/core/inspector/PageRuntimeAgent.cpp:197: // FIXME: should Dart scripts
have a security origin?
I think so.  Can you file a bug on this?

[Jacob, 2013-10-03 20:42:06]

Committed patchset #3 manually as r1406 (presubmit successful).

[jacob314, 2013-10-03 20:43:14]

https://chromiumcodereview.appspot.com/24989007/diff/14001/Source/bindings/v8...
File Source/bindings/v8/custom/V8InjectedScriptHostCustom.cpp (right):

https://chromiumcodereview.appspot.com/24989007/diff/14001/Source/bindings/v8...
Source/bindings/v8/custom/V8InjectedScriptHostCustom.cpp:358:
v8::Handle<v8::Value> dartResult =
DartInjectedScriptHost::evaluateIfDartContext(args.Holder(), expression);
On 2013/10/03 17:59:31, vsm wrote:
> It'd be nice to make this a virtual method on something, somehow.

I agree this is the right long term solution.  Added a comment and filed a bug
dartbug.com/13804

https://chromiumcodereview.appspot.com/24989007/diff/14001/Source/core/inspec...
File Source/core/inspector/PageRuntimeAgent.cpp (right):

https://chromiumcodereview.appspot.com/24989007/diff/14001/Source/core/inspec...
Source/core/inspector/PageRuntimeAgent.cpp:184: {
On 2013/10/03 17:59:31, vsm wrote:
> Is the runtime type of v8ScriptState a ScriptState or a DartScriptState?  If
the
> latter, can we move code over there?

Unfortunately the runtime type of v8ScriptState is ScriptState.

https://chromiumcodereview.appspot.com/24989007/diff/14001/Source/core/inspec...
Source/core/inspector/PageRuntimeAgent.cpp:197: // FIXME: should Dart scripts
have a security origin?
On 2013/10/03 17:59:31, vsm wrote:
> I think so.  Can you file a bug on this?

updated comment and filed a bug.
dartbug.com/13801