[Compiler] Ensure we enter the correct context for compiler-dispatcher jobs.

When running main-thread compiler-dispatcher jobs, ensure that we enter the
correct Context. Also adds a test for compiling an extension in the compiler
dispatcher to ensure that idle tasks enter the correct context before
finalizing the compilation.

BUG=v8:5203

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

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

@@ -69,6 +69,8 @@ CompilerDispatcherJob::CompilerDispatcherJob(Isolate* isolate,
     : status_(CompileJobStatus::kInitial),
       isolate_(isolate),
       tracer_(tracer),
+      context_(Handle<Context>::cast(
+          isolate_->global_handles()->Create(isolate->context()))),
       shared_(Handle<SharedFunctionInfo>::cast(
           isolate_->global_handles()->Create(*shared))),
       max_stack_size_(max_stack_size),
@@ -93,6 +95,8 @@ CompilerDispatcherJob::CompilerDispatcherJob(
     : status_(CompileJobStatus::kAnalyzed),
       isolate_(isolate),
       tracer_(tracer),
+      context_(Handle<Context>::cast(
+          isolate_->global_handles()->Create(isolate->context()))),
       shared_(Handle<SharedFunctionInfo>::cast(
           isolate_->global_handles()->Create(*shared))),
       max_stack_size_(max_stack_size),
@@ -117,6 +121,7 @@ CompilerDispatcherJob::~CompilerDispatcherJob() {
   DCHECK(status_ == CompileJobStatus::kInitial ||
          status_ == CompileJobStatus::kDone);
   i::GlobalHandles::Destroy(Handle<Object>::cast(shared_).location());
+  i::GlobalHandles::Destroy(Handle<Object>::cast(context_).location());
 }
 
 bool CompilerDispatcherJob::IsAssociatedWith(
@@ -386,11 +391,14 @@ bool CompilerDispatcherJob::FinalizeCompilingOnMainThread() {
            static_cast<void*>(this));
   }
 
-  if (compile_job_->state() == CompilationJob::State::kFailed ||
-      !Compiler::FinalizeCompilationJob(compile_job_.release())) {
-    if (!isolate_->has_pending_exception()) isolate_->StackOverflow();
-    status_ = CompileJobStatus::kFailed;
-    return false;
+  {
+    HandleScope scope(isolate_);
+    if (compile_job_->state() == CompilationJob::State::kFailed ||
+        !Compiler::FinalizeCompilationJob(compile_job_.release())) {
+      if (!isolate_->has_pending_exception()) isolate_->StackOverflow();
+      status_ = CompileJobStatus::kFailed;
+      return false;
+    }
   }
 
   compile_job_.reset();

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

@@ -60,6 +60,8 @@ class V8_EXPORT_PRIVATE CompilerDispatcherJob {
 
   CompileJobStatus status() const { return status_; }
 
+  Context* context() { return *context_; }
+
   // Returns true if this CompilerDispatcherJob was created for the given
   // function.
   bool IsAssociatedWith(Handle<SharedFunctionInfo> shared) const;
@@ -105,6 +107,7 @@ class V8_EXPORT_PRIVATE CompilerDispatcherJob {
   CompileJobStatus status_;
   Isolate* isolate_;
   CompilerDispatcherTracer* tracer_;
+  Handle<Context> context_;            // Global handle.
   Handle<SharedFunctionInfo> shared_;  // Global handle.
   Handle<String> source_;        // Global handle.
   Handle<String> wrapper_;       // Global handle.

src/compiler-dispatcher/compiler-dispatcher.cc

@@ -24,6 +24,11 @@ enum class ExceptionHandling { kSwallow, kThrow };
 bool DoNextStepOnMainThread(Isolate* isolate, CompilerDispatcherJob* job,
                             ExceptionHandling exception_handling) {
   DCHECK(ThreadId::Current().Equals(isolate->thread_id()));
+
+  // Ensure we are in the correct context for the job.
+  SaveContext save(isolate);
+  isolate->set_context(job->context());
+
   switch (job->status()) {
     case CompileJobStatus::kInitial:
       job->PrepareToParseOnMainThread();

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

@@ -5,6 +5,7 @@
 #include "src/compiler-dispatcher/compiler-dispatcher.h"
 
 #include "include/v8-platform.h"
+#include "src/api.h"
 #include "src/base/platform/semaphore.h"
 #include "src/compiler-dispatcher/compiler-dispatcher-job.h"
 #include "src/compiler-dispatcher/compiler-dispatcher-tracer.h"
@@ -22,34 +23,67 @@
 namespace v8 {
 namespace internal {
 
+class CompilerDispatcherTestFlags {
+ public:
+  static void SetFlagsForTest() {
+    old_compiler_dispatcher_flag_ = i::FLAG_compiler_dispatcher;
+    i::FLAG_compiler_dispatcher = true;
+    old_ignition_flag_ = i::FLAG_ignition;
+    i::FLAG_ignition = true;
+  }
+
+  static void RestoreFlags() {
+    i::FLAG_compiler_dispatcher = old_compiler_dispatcher_flag_;
+    i::FLAG_ignition = old_ignition_flag_;
+  }
+
+ private:
+  static bool old_compiler_dispatcher_flag_;
+  static bool old_ignition_flag_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(CompilerDispatcherTestFlags);
+};
+
+bool CompilerDispatcherTestFlags::old_compiler_dispatcher_flag_;
+bool CompilerDispatcherTestFlags::old_ignition_flag_;
+
 class CompilerDispatcherTest : public TestWithContext {
  public:
   CompilerDispatcherTest() = default;
   ~CompilerDispatcherTest() override = default;
 
   static void SetUpTestCase() {
-    old_flag_ = i::FLAG_ignition;
-    i::FLAG_compiler_dispatcher = true;
-    old_ignition_flag_ = i::FLAG_ignition;
-    i::FLAG_ignition = true;
+    CompilerDispatcherTestFlags::SetFlagsForTest();
     TestWithContext::SetUpTestCase();
   }
 
   static void TearDownTestCase() {
     TestWithContext::TearDownTestCase();
-    i::FLAG_compiler_dispatcher = old_flag_;
-    i::FLAG_ignition = old_ignition_flag_;
+    CompilerDispatcherTestFlags::RestoreFlags();
   }
 
  private:
-  static bool old_flag_;
-  static bool old_ignition_flag_;
-
   DISALLOW_COPY_AND_ASSIGN(CompilerDispatcherTest);
 };
 
-bool CompilerDispatcherTest::old_flag_;
-bool CompilerDispatcherTest::old_ignition_flag_;
+class CompilerDispatcherTestWithoutContext : public v8::TestWithIsolate {
+ public:
+  CompilerDispatcherTestWithoutContext() = default;
+  ~CompilerDispatcherTestWithoutContext() override = default;
+
+  static void SetUpTestCase() {
+    CompilerDispatcherTestFlags::SetFlagsForTest();
+    TestWithContext::SetUpTestCase();
+  }
+
+  static void TearDownTestCase() {
+    TestWithContext::TearDownTestCase();
+    CompilerDispatcherTestFlags::RestoreFlags();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CompilerDispatcherTestWithoutContext);
+};
 
 namespace {
 
@@ -939,5 +973,79 @@ TEST_F(CompilerDispatcherTest, CompileParsedOutOfScope) {
   ASSERT_TRUE(shared->is_compiled());
 }
 
+namespace {
+
+const char kExtensionSource[] = "native function Dummy();";
+
+class MockNativeFunctionExtension : public Extension {
+ public:
+  MockNativeFunctionExtension()
+      : Extension("mock-extension", kExtensionSource), function_(&Dummy) {}
+
+  virtual v8::Local<v8::FunctionTemplate> GetNativeFunctionTemplate(
+      v8::Isolate* isolate, v8::Local<v8::String> name) {
+    return v8::FunctionTemplate::New(isolate, function_);
+  }
+
+  static void Dummy(const v8::FunctionCallbackInfo<v8::Value>& args) { return; }
+
+ private:
+  v8::FunctionCallback function_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockNativeFunctionExtension);
+};
+
+}  // namespace
+
+TEST_F(CompilerDispatcherTestWithoutContext, CompileExtensionWithoutContext) {
+  MockPlatform platform;
+  CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
+  Local<v8::Context> context = v8::Context::New(isolate());
+
+  MockNativeFunctionExtension extension;
+  Handle<String> script_str =
+      i_isolate()
+          ->factory()
+          ->NewStringFromUtf8(CStrVector(kExtensionSource))
+          .ToHandleChecked();
+  Handle<Script> script = i_isolate()->factory()->NewScript(script_str);
+  script->set_type(Script::TYPE_EXTENSION);
+
+  Handle<SharedFunctionInfo> shared;
+  {
+    v8::Context::Scope scope(context);
+
+    ParseInfo parse_info(script);
+    parse_info.set_extension(&extension);
+
+    ASSERT_TRUE(parsing::ParseAny(&parse_info));
+    Handle<FixedArray> shared_infos_array(i_isolate()->factory()->NewFixedArray(
+        parse_info.max_function_literal_id() + 1));
+    parse_info.script()->set_shared_function_infos(*shared_infos_array);
+    DeferredHandleScope handles_scope(i_isolate());
+    { ASSERT_TRUE(Compiler::Analyze(&parse_info)); }
+    std::shared_ptr<DeferredHandles> compilation_handles(
+        handles_scope.Detach());
+
+    shared = i_isolate()->factory()->NewSharedFunctionInfoForLiteral(
+        parse_info.literal(), script);
+    parse_info.set_shared_info(shared);
+
+    ASSERT_FALSE(platform.IdleTaskPending());
+    ASSERT_TRUE(dispatcher.Enqueue(
+        shared, parse_info.literal(), parse_info.zone_shared(),
+        parse_info.deferred_handles(), compilation_handles));
+    ASSERT_TRUE(platform.IdleTaskPending());
+  }
+  // Exit the context scope before running the idle task.
+
+  // Since time doesn't progress on the MockPlatform, this is enough idle time
+  // to finish compiling the function.
+  platform.RunIdleTask(1000.0, 0.0);
+
+  ASSERT_FALSE(dispatcher.IsEnqueued(shared));
+  ASSERT_TRUE(shared->is_compiled());
+}
+
 }  // namespace internal
 }  // namespace v8

test/unittests/test-utils.h

@@ -24,6 +24,10 @@ class TestWithIsolate : public virtual ::testing::Test {
 
   Isolate* isolate() const { return isolate_; }
 
+  v8::internal::Isolate* i_isolate() const {
+    return reinterpret_cast<v8::internal::Isolate*>(isolate());
+  }
+
   static void SetUpTestCase();
   static void TearDownTestCase();
 
@@ -44,10 +48,6 @@ 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_;