Fix for issue 19817

Fix for issue 19817
- do not try to finalize classes on every Dart API call which could
  potentially result in calls to dart code
- Invoke Dart_FinalizeLoading after all loads are done to ensure that
  classes are finalized

R=hausner@google.com

Committed: https://code.google.com/p/dart/source/detail?r=38471

[hausner, 2014-07-22 18:23:32]

LGTM with a few suggestions.

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/bin/dartutils...
runtime/bin/dartutils.cc:802: result = Dart_FinalizeLoading(false);
Maybe a comment that we need to finalize here so that we can invoke dart
functions below?

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/vm/dart_api_i...
File runtime/vm/dart_api_impl.cc (right):

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/vm/dart_api_i...
runtime/vm/dart_api_impl.cc:3873: // Since we have allocated an object it would
mean that all classes
all classes, or the type of the receiver?

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/vm/dart_api_i...
runtime/vm/dart_api_impl.cc:4937: RETURN_TYPE_ERROR(isolate, library, Library);
Shouldn't this be the same "library must be loaded" error message you have in
other places above?

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/vm/debugger_a...
File runtime/vm/debugger_api_impl.cc (left):

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/vm/debugger_a...
runtime/vm/debugger_api_impl.cc:403: Dart_Handle state =
Api::CheckIsolateState(isolate);
Should we check that the library has been successfully loaded? If so, just add a
TODO and I'll add the check later.

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/vm/debugger_a...
runtime/vm/debugger_api_impl.cc:441: Dart_Handle state =
Api::CheckIsolateState(isolate);
ditto

[siva, 2014-07-22 19:55:11]

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/bin/dartutils...
runtime/bin/dartutils.cc:802: result = Dart_FinalizeLoading(false);
On 2014/07/22 18:23:32, hausner wrote:
> Maybe a comment that we need to finalize here so that we can invoke dart
> functions below?

Done.

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/vm/dart_api_i...
File runtime/vm/dart_api_impl.cc (right):

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/vm/dart_api_i...
runtime/vm/dart_api_impl.cc:3873: // Since we have allocated an object it would
mean that all classes
Fixed the comment.

On 2014/07/22 18:23:32, hausner wrote:
> all classes, or the type of the receiver?

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/vm/dart_api_i...
runtime/vm/dart_api_impl.cc:4937: RETURN_TYPE_ERROR(isolate, library, Library);
True, good catch that was a cut and paste error.

On 2014/07/22 18:23:32, hausner wrote:
> Shouldn't this be the same "library must be loaded" error message you have in
> other places above?

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/vm/debugger_a...
File runtime/vm/debugger_api_impl.cc (left):

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/vm/debugger_a...
runtime/vm/debugger_api_impl.cc:403: Dart_Handle state =
Api::CheckIsolateState(isolate);
Added a check that the library passed in is loaded.

On 2014/07/22 18:23:32, hausner wrote:
> Should we check that the library has been successfully loaded? If so, just add
a
> TODO and I'll add the check later.

https://chromiumcodereview.appspot.com/411633002/diff/1/runtime/vm/debugger_a...
runtime/vm/debugger_api_impl.cc:441: Dart_Handle state =
Api::CheckIsolateState(isolate);
On 2014/07/22 18:23:32, hausner wrote:
> ditto

Done.

[siva, 2014-07-22 20:08:24]

Committed patchset #2 manually as r38471 (presubmit successful).