From 67af1ad671d8d8a1dd6358339436f8b9718fc3d9 Mon Sep 17 00:00:00 2001 From: AnnaMag Date: Wed, 1 Feb 2017 13:00:40 +0000 Subject: [PATCH] src: refactor CopyProperties to remove JS CopyProperties() is refactored to use the V8 5.5 DefineProperty() API call. The change does not alter current behaviour. It is a step prior to removing the function CopyProperties, which becomes reduntant after fixes of V8 SetNamedPropertyHandler in 5.5. V8. Strings used as property attributes (value, enumerable etc) and accessors are defined as persistent strings in src/env.h PR-URL: https://github.com/nodejs/node/pull/11102 Reviewed-By: Franziska Hinkelmann Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell --- src/env.h | 5 +++ src/node_contextify.cc | 73 ++++++++++++++++++++++++------------------ 2 files changed, 46 insertions(+), 32 deletions(-) diff --git a/src/env.h b/src/env.h index 8c256ca9c7faa4..581d7e9aef3c67 100644 --- a/src/env.h +++ b/src/env.h @@ -82,6 +82,7 @@ namespace node { V(oncertcb_string, "oncertcb") \ V(onclose_string, "_onclose") \ V(code_string, "code") \ + V(configurable_string, "configurable") \ V(cwd_string, "cwd") \ V(dest_string, "dest") \ V(detached_string, "detached") \ @@ -89,6 +90,7 @@ namespace node { V(domain_string, "domain") \ V(emitting_top_level_domain_error_string, "_emittingTopLevelDomainError") \ V(exchange_string, "exchange") \ + V(enumerable_string, "enumerable") \ V(idle_string, "idle") \ V(irq_string, "irq") \ V(encoding_string, "encoding") \ @@ -112,6 +114,7 @@ namespace node { V(file_string, "file") \ V(fingerprint_string, "fingerprint") \ V(flags_string, "flags") \ + V(get_string, "get") \ V(gid_string, "gid") \ V(handle_string, "handle") \ V(heap_total_string, "heapTotal") \ @@ -191,6 +194,7 @@ namespace node { V(service_string, "service") \ V(servername_string, "servername") \ V(session_id_string, "sessionId") \ + V(set_string, "set") \ V(shell_string, "shell") \ V(signal_string, "signal") \ V(size_string, "size") \ @@ -217,6 +221,7 @@ namespace node { V(username_string, "username") \ V(valid_from_string, "valid_from") \ V(valid_to_string, "valid_to") \ + V(value_string, "value") \ V(verify_error_string, "verifyError") \ V(version_string, "version") \ V(weight_string, "weight") \ diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 5ed7d5faf0932e..4efb420fa128b7 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -33,6 +33,7 @@ using v8::ObjectTemplate; using v8::Persistent; using v8::PropertyAttribute; using v8::PropertyCallbackInfo; +using v8::PropertyDescriptor; using v8::Script; using v8::ScriptCompiler; using v8::ScriptOrigin; @@ -132,41 +133,49 @@ class ContextifyContext { return; if (!has.FromJust()) { - // Could also do this like so: - // - // PropertyAttribute att = global->GetPropertyAttributes(key_v); - // Local val = global->Get(key_v); - // sandbox->ForceSet(key_v, val, att); - // - // However, this doesn't handle ES6-style properties configured with - // Object.defineProperty, and that's exactly what we're up against at - // this point. ForceSet(key,val,att) only supports value properties - // with the ES3-style attribute flags (DontDelete/DontEnum/ReadOnly), - // which doesn't faithfully capture the full range of configurations - // that can be done using Object.defineProperty. - if (clone_property_method.IsEmpty()) { - Local code = FIXED_ONE_BYTE_STRING(env()->isolate(), - "(function cloneProperty(source, key, target) {\n" - " if (key === 'Proxy') return;\n" - " try {\n" - " var desc = Object.getOwnPropertyDescriptor(source, key);\n" - " if (desc.value === source) desc.value = target;\n" - " Object.defineProperty(target, key, desc);\n" - " } catch (e) {\n" - " // Catch sealed properties errors\n" - " }\n" - "})"); - - Local