src: standardise context embedder indices

PR-URL: #19135 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Matheus Marchini <[email protected]>
nodejs · Mar 7, 2018 · c9b4de5 · c9b4de5
1 parent a6c14b2
commit c9b4de5
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 28 deletions.
diff --git a/src/env-inl.h b/src/env-inl.h
@@ -31,6 +31,7 @@
 #include "uv.h"
 #include "v8.h"
 #include "node_perf_common.h"
+#include "node_context_data.h"
 
 #include <stddef.h>
 #include <stdint.h>
@@ -283,7 +284,8 @@ inline void Environment::TickInfo::set_has_thrown(bool state) {
 
 inline void Environment::AssignToContext(v8::Local<v8::Context> context,
                                          const ContextInfo& info) {
-  context->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex, this);
+  context->SetAlignedPointerInEmbedderData(
+      ContextEmbedderIndex::kEnvironment, this);
 #if HAVE_INSPECTOR
   inspector_agent()->ContextCreated(context, info);
 #endif  // HAVE_INSPECTOR
@@ -295,7 +297,8 @@ inline Environment* Environment::GetCurrent(v8::Isolate* isolate) {
 
 inline Environment* Environment::GetCurrent(v8::Local<v8::Context> context) {
   return static_cast<Environment*>(
-      context->GetAlignedPointerFromEmbedderData(kContextEmbedderDataIndex));
+      context->GetAlignedPointerFromEmbedderData(
+          ContextEmbedderIndex::kEnvironment));
 }
 
 inline Environment* Environment::GetCurrent(
@@ -368,8 +371,8 @@ inline Environment::~Environment() {
   inspector_agent_.reset();
 #endif
 
-  context()->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex,
-                                             nullptr);
+  context()->SetAlignedPointerInEmbedderData(
+      ContextEmbedderIndex::kEnvironment, nullptr);
 
   delete[] heap_statistics_buffer_;
   delete[] heap_space_statistics_buffer_;

diff --git a/src/env.h b/src/env.h
@@ -75,14 +75,6 @@ struct PackageConfig {
 };
 }  // namespace loader
 
-// Pick an index that's hopefully out of the way when we're embedded inside
-// another application. Performance-wise or memory-wise it doesn't matter:
-// Context::SetAlignedPointerInEmbedderData() is backed by a FixedArray,
-// worst case we pay a one-time penalty for resizing the array.
-#ifndef NODE_CONTEXT_EMBEDDER_DATA_INDEX
-#define NODE_CONTEXT_EMBEDDER_DATA_INDEX 32
-#endif
-
 // The number of items passed to push_values_to_array_function has diminishing
 // returns around 8. This should be used at all call sites using said function.
 #ifndef NODE_PUSH_VAL_TO_ARRAY_MAX
@@ -730,8 +722,6 @@ class Environment {
   inline HandleWrapQueue* handle_wrap_queue() { return &handle_wrap_queue_; }
   inline ReqWrapQueue* req_wrap_queue() { return &req_wrap_queue_; }
 
-  static const int kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX;
-
   void AddPromiseHook(promise_hook_func fn, void* arg);
   bool RemovePromiseHook(promise_hook_func fn, void* arg);
   bool EmitNapiWarning();

diff --git a/src/node_context_data.h b/src/node_context_data.h
@@ -0,0 +1,25 @@
+#ifndef SRC_NODE_CONTEXT_DATA_H_
+#define SRC_NODE_CONTEXT_DATA_H_
+
+namespace node {
+
+// Pick an index that's hopefully out of the way when we're embedded inside
+// another application. Performance-wise or memory-wise it doesn't matter:
+// Context::SetAlignedPointerInEmbedderData() is backed by a FixedArray,
+// worst case we pay a one-time penalty for resizing the array.
+#ifndef NODE_CONTEXT_EMBEDDER_DATA_INDEX
+#define NODE_CONTEXT_EMBEDDER_DATA_INDEX 32
+#endif
+
+#ifndef NODE_CONTEXT_SANDBOX_OBJECT_INDEX
+#define NODE_CONTEXT_SANDBOX_OBJECT_INDEX 33
+#endif
+
+enum ContextEmbedderIndex {
+  kEnvironment = NODE_CONTEXT_EMBEDDER_DATA_INDEX,
+  kSandboxObject = NODE_CONTEXT_SANDBOX_OBJECT_INDEX,
+};
+
+}  // namespace node
+
+#endif  // SRC_NODE_CONTEXT_DATA_H_
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -23,6 +23,7 @@
 #include "node_watchdog.h"
 #include "base_object-inl.h"
 #include "node_contextify.h"
+#include "node_context_data.h"
 
 namespace node {
 namespace contextify {
@@ -173,7 +174,7 @@ Local<Context> ContextifyContext::CreateV8Context(
   // embedder data field. However, we cannot hold a reference to a v8::Context
   // directly in an Object, we instead hold onto the new context's global
   // object instead (which then has a reference to the context).
-  ctx->SetEmbedderData(kSandboxObjectIndex, sandbox_obj);
+  ctx->SetEmbedderData(ContextEmbedderIndex::kSandboxObject, sandbox_obj);
   sandbox_obj->SetPrivate(env->context(),
                           env->contextify_global_private_symbol(),
                           ctx->Global());

diff --git a/src/node_contextify.h b/src/node_contextify.h
@@ -4,16 +4,13 @@
 #include "node_internals.h"
 #include "node_watchdog.h"
 #include "base_object-inl.h"
+#include "node_context_data.h"
 
 namespace node {
 namespace contextify {
 
 class ContextifyContext {
  protected:
-  // V8 reserves the first field in context objects for the debugger. We use the
-  // second field to hold a reference to the sandbox object.
-  enum { kSandboxObjectIndex = 1 };
-
   Environment* const env_;
   Persistent<v8::Context> context_;
 
@@ -45,7 +42,7 @@ class ContextifyContext {
 
   inline v8::Local<v8::Object> sandbox() const {
     return v8::Local<v8::Object>::Cast(
-        context()->GetEmbedderData(kSandboxObjectIndex));
+        context()->GetEmbedderData(ContextEmbedderIndex::kSandboxObject));
   }
 
  private:

diff --git a/src/node_postmortem_metadata.cc b/src/node_postmortem_metadata.cc
@@ -4,6 +4,7 @@
 #include "util-inl.h"
 #include "req_wrap.h"
 #include "v8abbr.h"
+#include "node_context_data.h"
 
 #define NODEDBG_SYMBOL(Name)  nodedbg_ ## Name
 
@@ -34,7 +35,7 @@
   V(ListNode_ReqWrap, next_, uintptr_t, ListNode<ReqWrap<uv_req_t>>::next_)
 
 extern "C" {
-int nodedbg_const_Environment__kContextEmbedderDataIndex__int;
+int nodedbg_const_ContextEmbedderIndex__kEnvironment__int;
 uintptr_t nodedbg_offset_ExternalString__data__uintptr_t;
 
 #define V(Class, Member, Type, Accessor)                                      \
@@ -46,8 +47,8 @@ uintptr_t nodedbg_offset_ExternalString__data__uintptr_t;
 namespace node {
 
 int GenDebugSymbols() {
-  nodedbg_const_Environment__kContextEmbedderDataIndex__int =
-      Environment::kContextEmbedderDataIndex;
+  nodedbg_const_ContextEmbedderIndex__kEnvironment__int =
+      ContextEmbedderIndex::kEnvironment;
 
   nodedbg_offset_ExternalString__data__uintptr_t = NODE_OFF_EXTSTR_DATA;
 

diff --git a/test/cctest/test_node_postmortem_metadata.cc b/test/cctest/test_node_postmortem_metadata.cc
@@ -13,7 +13,7 @@ extern uintptr_t
 extern uintptr_t
     nodedbg_offset_Environment__handle_wrap_queue___Environment_HandleWrapQueue;
 extern int debug_symbols_generated;
-extern int nodedbg_const_Environment__kContextEmbedderDataIndex__int;
+extern int nodedbg_const_ContextEmbedderIndex__kEnvironment__int;
 extern uintptr_t
     nodedbg_offset_Environment_HandleWrapQueue__head___ListNode_HandleWrap;
 extern uintptr_t
@@ -56,10 +56,10 @@ class TestReqWrap : public node::ReqWrap<uv_req_t> {
                                 node::AsyncWrap::PROVIDER_TIMERWRAP) {}
 };
 
-TEST_F(DebugSymbolsTest, ContextEmbedderDataIndex) {
-  int kContextEmbedderDataIndex = node::Environment::kContextEmbedderDataIndex;
-  EXPECT_EQ(nodedbg_const_Environment__kContextEmbedderDataIndex__int,
-            kContextEmbedderDataIndex);
+TEST_F(DebugSymbolsTest, ContextEmbedderEnvironmentIndex) {
+  int kEnvironmentIndex = node::ContextEmbedderIndex::kEnvironment;
+  EXPECT_EQ(nodedbg_const_ContextEmbedderIndex__kEnvironment__int,
+            kEnvironmentIndex);
 }
 
 TEST_F(DebugSymbolsTest, ExternalStringDataOffset) {