Add parser finalization step

BUG=v8:5215
R=vogelheim@chromium.org,marja@chromium.org

Review-Url: https://codereview.chromium.org/2193813002
Cr-Commit-Position: refs/heads/master@{#38203}

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

@@ -33,6 +33,8 @@ CompilerDispatcherJob::CompilerDispatcherJob(Isolate* isolate,
 
 CompilerDispatcherJob::~CompilerDispatcherJob() {
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
+  DCHECK(status_ == CompileJobStatus::kInitial ||
+         status_ == CompileJobStatus::kDone);
   i::GlobalHandles::Destroy(Handle<Object>::cast(function_).location());
 }
 
@@ -46,19 +48,20 @@ 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;
+    source = String::Flatten(source);
+    // Have to globalize the reference here, so it survives between function
+    // calls.
+    source_ = Handle<String>::cast(isolate_->global_handles()->Create(*source));
     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_);
@@ -76,7 +79,12 @@ void CompilerDispatcherJob::Parse() {
 
   DisallowHeapAllocation no_allocation;
   DisallowHandleAllocation no_handles;
-  DisallowHandleDereference no_deref;
+  std::unique_ptr<DisallowHandleDereference> no_deref;
+  // If we can't parse on a background thread, we need to be able to deref the
+  // source string.
+  if (can_parse_on_background_thread_) {
+    no_deref.reset(new DisallowHandleDereference());
+  }
 
   // Nullify the Isolate temporarily so that the parser doesn't accidentally
   // use it.
@@ -93,5 +101,75 @@ 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());
+    source_ = Handle<String>::null();
+  }
+
+  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::ResetOnMainThread() {
+  DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
+
+  parser_.reset();
+  unicode_cache_.reset();
+  character_stream_.reset();
+  parse_info_.reset();
+  zone_.reset();
+
+  if (!source_.is_null()) {
+    i::GlobalHandles::Destroy(Handle<Object>::cast(source_).location());
+    source_ = Handle<String>::null();
+  }
+
+  status_ = CompileJobStatus::kInitial;
+}
+
+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();
+}
+
 }  // namespace internal
 }  // namespace v8

src/compiler-dispatcher/compiler-dispatcher-job.h

@@ -18,6 +18,7 @@ class Isolate;
 class JSFunction;
 class ParseInfo;
 class Parser;
+class String;
 class UnicodeCache;
 class Utf16CharacterStream;
 class Zone;
@@ -26,6 +27,9 @@ enum class CompileJobStatus {
   kInitial,
   kReadyToParse,
   kParsed,
+  kReadyToCompile,
+  kFailed,
+  kDone,
 };
 
 class CompilerDispatcherJob {
@@ -45,10 +49,22 @@ class CompilerDispatcherJob {
   // Transition from kReadyToParse to kParsed.
   void Parse();
 
+  // Transition from kParsed to kReadyToCompile (or kFailed).
+  void FinalizeParsingOnMainThread();
+
+  // Transition from kFailed to kDone.
+  void ReportErrorsOnMainThread();
+
+  // Transition from any state to kInitial and free all resources.
+  void ResetOnMainThread();
+
  private:
+  void InternalizeParsingResult();
+
   CompileJobStatus status_ = CompileJobStatus::kInitial;
   Isolate* isolate_;
   Handle<JSFunction> function_;  // Global handle.
+  Handle<String> source_;        // Global handle.
   size_t max_stack_size_;
 
   // Members required for parsing.

test/unittests/compiler-dispatcher/compiler-dispatcher-job-unittest.cc

@@ -49,6 +49,7 @@ Handle<JSFunction> CreateFunction(
       isolate->factory()->NewStringFromStaticChars("f"), MaybeHandle<Code>(),
       false);
   SharedFunctionInfo::SetScript(shared, script);
+  shared->set_end_position(source->length() - 1);
   Handle<JSFunction> function =
       isolate->factory()->NewFunctionFromSharedFunctionInfo(
           shared, handle(isolate->context(), isolate));
@@ -58,36 +59,53 @@ Handle<JSFunction> CreateFunction(
 }  // namespace
 
 TEST_F(CompilerDispatcherJobTest, Construct) {
-  Isolate* i_isolate = reinterpret_cast<Isolate*>(isolate());
   std::unique_ptr<CompilerDispatcherJob> job(new CompilerDispatcherJob(
-      i_isolate, CreateFunction(i_isolate, nullptr), FLAG_stack_size));
+      i_isolate(), CreateFunction(i_isolate(), nullptr), FLAG_stack_size));
 }
 
 TEST_F(CompilerDispatcherJobTest, CanParseOnBackgroundThread) {
-  Isolate* i_isolate = reinterpret_cast<Isolate*>(isolate());
   {
     std::unique_ptr<CompilerDispatcherJob> job(new CompilerDispatcherJob(
-        i_isolate, CreateFunction(i_isolate, nullptr), FLAG_stack_size));
+        i_isolate(), CreateFunction(i_isolate(), nullptr), FLAG_stack_size));
     ASSERT_FALSE(job->can_parse_on_background_thread());
   }
   {
     ScriptResource script("script", strlen("script"));
     std::unique_ptr<CompilerDispatcherJob> job(new CompilerDispatcherJob(
-        i_isolate, CreateFunction(i_isolate, &script), FLAG_stack_size));
+        i_isolate(), CreateFunction(i_isolate(), &script), FLAG_stack_size));
     ASSERT_TRUE(job->can_parse_on_background_thread());
   }
 }
 
 TEST_F(CompilerDispatcherJobTest, StateTransitions) {
-  Isolate* i_isolate = reinterpret_cast<Isolate*>(isolate());
   std::unique_ptr<CompilerDispatcherJob> job(new CompilerDispatcherJob(
-      i_isolate, CreateFunction(i_isolate, nullptr), FLAG_stack_size));
+      i_isolate(), CreateFunction(i_isolate(), nullptr), FLAG_stack_size));
 
   ASSERT_TRUE(job->status() == CompileJobStatus::kInitial);
   job->PrepareToParseOnMainThread();
   ASSERT_TRUE(job->status() == CompileJobStatus::kReadyToParse);
   job->Parse();
   ASSERT_TRUE(job->status() == CompileJobStatus::kParsed);
+  job->FinalizeParsingOnMainThread();
+  ASSERT_TRUE(job->status() == CompileJobStatus::kReadyToCompile);
+  job->ResetOnMainThread();
+  ASSERT_TRUE(job->status() == CompileJobStatus::kInitial);
+}
+
+TEST_F(CompilerDispatcherJobTest, SyntaxError) {
+  ScriptResource script("^^^", strlen("^^^"));
+  std::unique_ptr<CompilerDispatcherJob> job(new CompilerDispatcherJob(
+      i_isolate(), CreateFunction(i_isolate(), &script), FLAG_stack_size));
+
+  job->PrepareToParseOnMainThread();
+  job->Parse();
+  job->FinalizeParsingOnMainThread();
+
+  ASSERT_TRUE(job->status() == CompileJobStatus::kFailed);
+  ASSERT_FALSE(i_isolate()->has_pending_exception());
+  job->ReportErrorsOnMainThread();
+  ASSERT_TRUE(job->status() == CompileJobStatus::kDone);
+  ASSERT_TRUE(i_isolate()->has_pending_exception());
 }
 
 }  // namespace internal

test/unittests/test-utils.h

@@ -43,6 +43,10 @@ class TestWithContext : public virtual TestWithIsolate {
 
   const Local<Context>& context() const { return context_; }
 
+  v8::internal::Isolate* i_isolate() const {
+    return reinterpret_cast<v8::internal::Isolate*>(isolate());
+  }
+
  private:
   Local<Context> context_;
   Context::Scope context_scope_;