Add parser finalization step

src/compiler-dispatcher/compiler-dispatcher-job.cc

Index: src/compiler-dispatcher/compiler-dispatcher-job.cc
diff --git a/src/compiler-dispatcher/compiler-dispatcher-job.cc b/src/compiler-dispatcher/compiler-dispatcher-job.cc
index 8234ae214092ee4a99363c04b661bc34bb3942bf..f9f4ec1f66109af22a1489bd97f5cb6e445705ab 100644
--- a/src/compiler-dispatcher/compiler-dispatcher-job.cc
+++ b/src/compiler-dispatcher/compiler-dispatcher-job.cc
@@ -46,19 +46,18 @@ void CompilerDispatcherJob::PrepareToParseOnMainThread() {
   Handle<Script> script(Script::cast(shared->script()), isolate_);
   Handle<String> source(String::cast(script->source()), isolate_);
   if (source->IsExternalTwoByteString()) {
-    can_parse_on_background_thread_ = true;
     character_stream_.reset(new ExternalTwoByteStringUtf16CharacterStream(
         Handle<ExternalTwoByteString>::cast(source), shared->start_position(),
         shared->end_position()));
   } else if (source->IsExternalOneByteString()) {
-    can_parse_on_background_thread_ = true;
     character_stream_.reset(new ExternalOneByteStringUtf16CharacterStream(
         Handle<ExternalOneByteString>::cast(source), shared->start_position(),
         shared->end_position()));
   } else {
-    can_parse_on_background_thread_ = false;

[vogelheim, 2016/07/29 12:46:18]

If the variable is no longer used, pls remove from class declaration.

+    source = String::Flatten(source);
+    source_ = Handle<String>::cast(isolate_->global_handles()->Create(*source));

[jochen (gone - plz use gerrit), 2016/07/29 12:23:31]

have to globalize the reference here, so it survives between function calls

[marja, 2016/08/01 07:50:24]

How about putting this comment into the source code :)

     character_stream_.reset(new GenericStringUtf16CharacterStream(
-        source, shared->start_position(), shared->end_position()));
+        source_, shared->start_position(), shared->end_position()));
   }
   parse_info_.reset(new ParseInfo(zone_.get()));
   parse_info_->set_isolate(isolate_);
@@ -93,5 +92,57 @@ void CompilerDispatcherJob::Parse() {
   status_ = CompileJobStatus::kParsed;
 }
 
+void CompilerDispatcherJob::FinalizeParsingOnMainThread() {
+  DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
+  DCHECK(status() == CompileJobStatus::kParsed);
+
+  if (!source_.is_null()) {
+    i::GlobalHandles::Destroy(Handle<Object>::cast(source_).location());
+  }
+
+  if (parse_info_->literal() == nullptr) {
+    status_ = CompileJobStatus::kFailed;
+    return;
+  }
+
+  InternalizeParsingResult();
+
+  status_ = CompileJobStatus::kReadyToCompile;
+}
+
+void CompilerDispatcherJob::ReportErrorsOnMainThread() {
+  DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
+  DCHECK(status() == CompileJobStatus::kFailed);
+
+  // Internalizing the parsing result will throw the error.
+  InternalizeParsingResult();
+
+  status_ = CompileJobStatus::kDone;
+}
+
+void CompilerDispatcherJob::InternalizeParsingResult() {
+  DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
+  DCHECK(status() == CompileJobStatus::kParsed ||
+         status() == CompileJobStatus::kFailed);
+
+  HandleScope scope(isolate_);
+  Handle<SharedFunctionInfo> shared(function_->shared(), isolate_);
+  Handle<Script> script(Script::cast(shared->script()), isolate_);
+
+  parse_info_->set_script(script);
+  parse_info_->set_context(handle(function_->context(), isolate_));
+
+  // Do the parsing tasks which need to be done on the main thread. This will
+  // also handle parse errors.
+  parser_->Internalize(isolate_, script, parse_info_->literal() == nullptr);
+  parser_->HandleSourceURLComments(isolate_, script);
+
+  parse_info_->set_character_stream(nullptr);
+  parse_info_->set_unicode_cache(nullptr);
+  parser_.reset();
+  unicode_cache_.reset();
+  character_stream_.reset();

[vogelheim, 2016/07/29 12:46:18]

[general comment:]

Since this class handles more & more resources, IMHO you should in the
destructor either:
- assert a proper terminal state (kFailed or kCompiled)
- or enforce cleanup in the destructor.

I prefer the former, since I guess the intention is that the dispatcher should
run the state machine to its proper end (& hence the job-class can assume it
will go through the proper life cycle).

Loosely related: If the intention is that the dispatcher controls the lifecycle,
I guess the job (eventually...) needs some sort of 'discard' edges in the state
machine, so that the dispatcher can cleanly abandon jobs.

[jochen (gone - plz use gerrit), 2016/07/29 12:49:42]

yeah, that makes sense

+}
+
 }  // namespace internal
 }  // namespace v8