perf: do not convert object keys in ctx bridge as they are always pri…

…mitives (#24671) * perf: do not convert object keys in ctx bridge as they are always primitives * Update shell/renderer/api/electron_api_context_bridge.cc Co-authored-by: Jeremy Rose <jeremya@chromium.org> Co-authored-by: Jeremy Rose <jeremya@chromium.org>
electron · Jul 21, 2020 · d4a4269 · d4a4269
1 parent cd80554
commit d4a4269
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 13 deletions.
diff --git a/shell/renderer/api/electron_api_context_bridge.cc b/shell/renderer/api/electron_api_context_bridge.cc
@@ -427,22 +427,16 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
         gin::Dictionary::CreateEmpty(destination_context->GetIsolate());
     object_cache->CacheProxiedObject(api.GetHandle(), proxy.GetHandle());
     auto maybe_keys = api.GetHandle()->GetOwnPropertyNames(
-        source_context,
-        static_cast<v8::PropertyFilter>(v8::ONLY_ENUMERABLE | v8::SKIP_SYMBOLS),
-        v8::KeyConversionMode::kConvertToString);
+        source_context, static_cast<v8::PropertyFilter>(v8::ONLY_ENUMERABLE));
     if (maybe_keys.IsEmpty())
       return v8::MaybeLocal<v8::Object>(proxy.GetHandle());
     auto keys = maybe_keys.ToLocalChecked();
 
     uint32_t length = keys->Length();
-    std::string key_str;
     for (uint32_t i = 0; i < length; i++) {
       v8::Local<v8::Value> key =
           keys->Get(destination_context, i).ToLocalChecked();
-      // Try get the key as a string
-      if (!gin::ConvertFromV8(api.isolate(), key, &key_str)) {
-        continue;
-      }
+
       if (support_dynamic_properties) {
         v8::Context::Scope source_context_scope(source_context);
         auto maybe_desc = api.GetHandle()->GetOwnPropertyDescriptor(
@@ -476,22 +470,21 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
 
             v8::PropertyDescriptor desc(getter_proxy, setter_proxy);
             ignore_result(proxy.GetHandle()->DefineProperty(
-                destination_context, gin::StringToV8(api.isolate(), key_str),
-                desc));
+                destination_context, key.As<v8::Name>(), desc));
           }
           continue;
         }
       }
       v8::Local<v8::Value> value;
-      if (!api.Get(key_str, &value))
+      if (!api.Get(key, &value))
         continue;
 
       auto passed_value = PassValueToOtherContext(
           source_context, destination_context, value, object_cache,
           support_dynamic_properties, recursion_depth + 1);
       if (passed_value.IsEmpty())
         return v8::MaybeLocal<v8::Object>();
-      proxy.Set(key_str, passed_value.ToLocalChecked());
+      proxy.Set(key, passed_value.ToLocalChecked());
     }
 
     return proxy.GetHandle();

diff --git a/spec-main/api-context-bridge-spec.ts b/spec-main/api-context-bridge-spec.ts
@@ -327,6 +327,20 @@ describe('contextBridge', () => {
         expect(result).to.equal(true, 'symbols should be equal across contexts');
       });
 
+      it('should proxy symbols such that symbol key lookup works', async () => {
+        await makeBindingWindow(() => {
+          const mySymbol = Symbol('unique');
+          contextBridge.exposeInMainWorld('example', {
+            getSymbol: () => mySymbol,
+            getObject: () => ({ [mySymbol]: 123 })
+          });
+        });
+        const result = await callWithBindings((root: any) => {
+          return root.example.getObject()[root.example.getSymbol()];
+        });
+        expect(result).to.equal(123, 'symbols key lookup should work across contexts');
+      });
+
       it('should proxy typed arrays and regexps through the serializer', async () => {
         await makeBindingWindow(() => {
           contextBridge.exposeInMainWorld('example', {
@@ -509,14 +523,19 @@ describe('contextBridge', () => {
               arr: [123, 'string', true, ['foo']],
               getPromise: async () => ({ number: 123, string: 'string', boolean: true, fn: () => 'string', arr: [123, 'string', true, ['foo']] })
             },
-            receiveArguments: (fn: any) => fn({ key: 'value' })
+            receiveArguments: (fn: any) => fn({ key: 'value' }),
+            symbolKeyed: {
+              [Symbol('foo')]: 123
+            }
           });
         });
         const result = await callWithBindings(async (root: any) => {
           const { example } = root;
           let arg: any;
           example.receiveArguments((o: any) => { arg = o; });
           const protoChecks = [
+            ...Object.keys(example).map(key => [key, String]),
+            ...Object.getOwnPropertySymbols(example.symbolKeyed).map(key => [key, Symbol]),
             [example, Object],
             [example.number, Number],
             [example.string, String],