fix: correctly track receiver for methods called via ctx bridge (#40262)

* fix: correctly track receiver for methods called via ctx bridge

Co-authored-by: Samuel Attard <[email protected]>

* spec: test for correct contextBridge passage

Co-authored-by: Shelley Vohr <[email protected]>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Samuel Attard <[email protected]>
Co-authored-by: Shelley Vohr <[email protected]>

shell/renderer/api/electron_api_context_bridge.cc

@@ -44,6 +44,8 @@ namespace api {
 namespace context_bridge {
 
 const char kProxyFunctionPrivateKey[] = "electron_contextBridge_proxy_fn";
+const char kProxyFunctionReceiverPrivateKey[] =
+    "electron_contextBridge_proxy_fn_receiver";
 const char kSupportsDynamicPropertiesPrivateKey[] =
     "electron_contextBridge_supportsDynamicProperties";
 const char kOriginalFunctionPrivateKey[] = "electron_contextBridge_original_fn";
@@ -138,6 +140,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
     v8::Local<v8::Context> source_context,
     v8::Local<v8::Context> destination_context,
     v8::Local<v8::Value> value,
+    v8::Local<v8::Value> parent_value,
     context_bridge::ObjectCache* object_cache,
     bool support_dynamic_properties,
     int recursion_depth,
@@ -199,6 +202,9 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
           v8::Object::New(destination_context->GetIsolate());
       SetPrivate(destination_context, state,
                  context_bridge::kProxyFunctionPrivateKey, func);
+      SetPrivate(destination_context, state,
+                 context_bridge::kProxyFunctionReceiverPrivateKey,
+                 parent_value);
       SetPrivate(destination_context, state,
                  context_bridge::kSupportsDynamicPropertiesPrivateKey,
                  gin::ConvertToV8(destination_context->GetIsolate(),
@@ -246,10 +252,12 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
           v8::MaybeLocal<v8::Value> val;
           {
             v8::TryCatch try_catch(isolate);
+            v8::Local<v8::Context> source_context =
+                global_source_context.Get(isolate);
             val = PassValueToOtherContext(
-                global_source_context.Get(isolate),
-                global_destination_context.Get(isolate), result, &object_cache,
-                false, 0, BridgeErrorTarget::kDestination);
+                source_context, global_destination_context.Get(isolate), result,
+                source_context->Global(), &object_cache, false, 0,
+                BridgeErrorTarget::kDestination);
             if (try_catch.HasCaught()) {
               if (try_catch.Message().IsEmpty()) {
                 proxied_promise->RejectWithErrorMessage(
@@ -292,10 +300,12 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
           v8::MaybeLocal<v8::Value> val;
           {
             v8::TryCatch try_catch(isolate);
+            v8::Local<v8::Context> source_context =
+                global_source_context.Get(isolate);
             val = PassValueToOtherContext(
-                global_source_context.Get(isolate),
-                global_destination_context.Get(isolate), result, &object_cache,
-                false, 0, BridgeErrorTarget::kDestination);
+                source_context, global_destination_context.Get(isolate), result,
+                source_context->Global(), &object_cache, false, 0,
+                BridgeErrorTarget::kDestination);
             if (try_catch.HasCaught()) {
               if (try_catch.Message().IsEmpty()) {
                 proxied_promise->RejectWithErrorMessage(
@@ -362,7 +372,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
     for (size_t i = 0; i < length; i++) {
       auto value_for_array = PassValueToOtherContext(
           source_context, destination_context,
-          arr->Get(source_context, i).ToLocalChecked(), object_cache,
+          arr->Get(source_context, i).ToLocalChecked(), value, object_cache,
           support_dynamic_properties, recursion_depth + 1, error_target);
       if (value_for_array.IsEmpty())
         return v8::MaybeLocal<v8::Value>();
@@ -440,8 +450,10 @@ void ProxyFunctionWrapper(const v8::FunctionCallbackInfo<v8::Value>& info) {
                  context_bridge::kSupportsDynamicPropertiesPrivateKey);
   v8::MaybeLocal<v8::Value> maybe_func = GetPrivate(
       calling_context, data, context_bridge::kProxyFunctionPrivateKey);
+  v8::MaybeLocal<v8::Value> maybe_recv = GetPrivate(
+      calling_context, data, context_bridge::kProxyFunctionReceiverPrivateKey);
   v8::Local<v8::Value> func_value;
-  if (sdp_value.IsEmpty() || maybe_func.IsEmpty() ||
+  if (sdp_value.IsEmpty() || maybe_func.IsEmpty() || maybe_recv.IsEmpty() ||
       !gin::ConvertFromV8(args.isolate(), sdp_value.ToLocalChecked(),
                           &support_dynamic_properties) ||
       !maybe_func.ToLocal(&func_value))
@@ -461,8 +473,9 @@ void ProxyFunctionWrapper(const v8::FunctionCallbackInfo<v8::Value>& info) {
 
     for (auto value : original_args) {
       auto arg = PassValueToOtherContext(
-          calling_context, func_owning_context, value, &object_cache,
-          support_dynamic_properties, 0, BridgeErrorTarget::kSource);
+          calling_context, func_owning_context, value,
+          calling_context->Global(), &object_cache, support_dynamic_properties,
+          0, BridgeErrorTarget::kSource);
       if (arg.IsEmpty())
         return;
       proxied_args.push_back(arg.ToLocalChecked());
@@ -473,8 +486,9 @@ void ProxyFunctionWrapper(const v8::FunctionCallbackInfo<v8::Value>& info) {
     v8::Local<v8::Value> error_message;
     {
       v8::TryCatch try_catch(args.isolate());
-      maybe_return_value = func->Call(func_owning_context, func,
-                                      proxied_args.size(), proxied_args.data());
+      maybe_return_value =
+          func->Call(func_owning_context, maybe_recv.ToLocalChecked(),
+                     proxied_args.size(), proxied_args.data());
       if (try_catch.HasCaught()) {
         did_error = true;
         v8::Local<v8::Value> exception = try_catch.Exception();
@@ -529,6 +543,7 @@ void ProxyFunctionWrapper(const v8::FunctionCallbackInfo<v8::Value>& info) {
       v8::TryCatch try_catch(args.isolate());
       ret = PassValueToOtherContext(func_owning_context, calling_context,
                                     maybe_return_value.ToLocalChecked(),
+                                    func_owning_context->Global(),
                                     &object_cache, support_dynamic_properties,
                                     0, BridgeErrorTarget::kDestination);
       if (try_catch.HasCaught()) {
@@ -605,18 +620,18 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
             v8::Local<v8::Value> getter_proxy;
             v8::Local<v8::Value> setter_proxy;
             if (!getter.IsEmpty()) {
-              if (!PassValueToOtherContext(source_context, destination_context,
-                                           getter, object_cache,
-                                           support_dynamic_properties, 1,
-                                           error_target)
+              if (!PassValueToOtherContext(
+                       source_context, destination_context, getter,
+                       api.GetHandle(), object_cache,
+                       support_dynamic_properties, 1, error_target)
                        .ToLocal(&getter_proxy))
                 continue;
             }
             if (!setter.IsEmpty()) {
-              if (!PassValueToOtherContext(source_context, destination_context,
-                                           setter, object_cache,
-                                           support_dynamic_properties, 1,
-                                           error_target)
+              if (!PassValueToOtherContext(
+                       source_context, destination_context, setter,
+                       api.GetHandle(), object_cache,
+                       support_dynamic_properties, 1, error_target)
                        .ToLocal(&setter_proxy))
                 continue;
             }
@@ -633,8 +648,9 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
         continue;
 
       auto passed_value = PassValueToOtherContext(
-          source_context, destination_context, value, object_cache,
-          support_dynamic_properties, recursion_depth + 1, error_target);
+          source_context, destination_context, value, api.GetHandle(),
+          object_cache, support_dynamic_properties, recursion_depth + 1,
+          error_target);
       if (passed_value.IsEmpty())
         return v8::MaybeLocal<v8::Object>();
       proxy.Set(key, passed_value.ToLocalChecked());
@@ -681,7 +697,8 @@ void ExposeAPIInWorld(v8::Isolate* isolate,
     v8::Context::Scope target_context_scope(target_context);
 
     v8::MaybeLocal<v8::Value> maybe_proxy = PassValueToOtherContext(
-        electron_isolated_context, target_context, api, &object_cache, false, 0,
+        electron_isolated_context, target_context, api,
+        electron_isolated_context->Global(), &object_cache, false, 0,
         BridgeErrorTarget::kSource);
     if (maybe_proxy.IsEmpty())
       return;
@@ -730,9 +747,11 @@ void OverrideGlobalValueFromIsolatedWorld(
   {
     v8::Context::Scope main_context_scope(main_context);
     context_bridge::ObjectCache object_cache;
+    v8::Local<v8::Context> source_context = value->GetCreationContextChecked();
     v8::MaybeLocal<v8::Value> maybe_proxy = PassValueToOtherContext(
-        value->GetCreationContextChecked(), main_context, value, &object_cache,
-        support_dynamic_properties, 1, BridgeErrorTarget::kSource);
+        source_context, main_context, value, source_context->Global(),
+        &object_cache, support_dynamic_properties, 1,
+        BridgeErrorTarget::kSource);
     DCHECK(!maybe_proxy.IsEmpty());
     auto proxy = maybe_proxy.ToLocalChecked();
 
@@ -766,15 +785,19 @@ bool OverrideGlobalPropertyFromIsolatedWorld(
     v8::Local<v8::Value> getter_proxy;
     v8::Local<v8::Value> setter_proxy;
     if (!getter->IsNullOrUndefined()) {
+      v8::Local<v8::Context> source_context =
+          getter->GetCreationContextChecked();
       v8::MaybeLocal<v8::Value> maybe_getter_proxy = PassValueToOtherContext(
-          getter->GetCreationContextChecked(), main_context, getter,
+          source_context, main_context, getter, source_context->Global(),
           &object_cache, false, 1, BridgeErrorTarget::kSource);
       DCHECK(!maybe_getter_proxy.IsEmpty());
       getter_proxy = maybe_getter_proxy.ToLocalChecked();
     }
     if (!setter->IsNullOrUndefined() && setter->IsObject()) {
+      v8::Local<v8::Context> source_context =
+          getter->GetCreationContextChecked();
       v8::MaybeLocal<v8::Value> maybe_setter_proxy = PassValueToOtherContext(
-          getter->GetCreationContextChecked(), main_context, setter,
+          source_context, main_context, setter, source_context->Global(),
           &object_cache, false, 1, BridgeErrorTarget::kSource);
       DCHECK(!maybe_setter_proxy.IsEmpty());
       setter_proxy = maybe_setter_proxy.ToLocalChecked();

shell/renderer/api/electron_api_context_bridge.h

@@ -36,6 +36,14 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
     v8::Local<v8::Context> source_context,
     v8::Local<v8::Context> destination_context,
     v8::Local<v8::Value> value,
+    /**
+     * Used to automatically bind a function across
+     * worlds to its appropriate default "this" value.
+     *
+     * If this value is the root of a tree going over
+     * the bridge set this to the "context" of the value.
+     */
+    v8::Local<v8::Value> parent_value,
     context_bridge::ObjectCache* object_cache,
     bool support_dynamic_properties,
     int recursion_depth,

shell/renderer/api/electron_api_web_frame.cc

@@ -140,9 +140,12 @@ class ScriptExecutionCallback {
     {
       v8::TryCatch try_catch(isolate);
       context_bridge::ObjectCache object_cache;
-      maybe_result = PassValueToOtherContext(
-          result->GetCreationContextChecked(), promise_.GetContext(), result,
-          &object_cache, false, 0, BridgeErrorTarget::kSource);
+      v8::Local<v8::Context> source_context =
+          result->GetCreationContextChecked();
+      maybe_result =
+          PassValueToOtherContext(source_context, promise_.GetContext(), result,
+                                  source_context->Global(), &object_cache,
+                                  false, 0, BridgeErrorTarget::kSource);
       if (maybe_result.IsEmpty() || try_catch.HasCaught()) {
         success = false;
       }

shell/renderer/renderer_client_base.cc

@@ -608,8 +608,8 @@ void RendererClientBase::SetupMainWorldOverrides(
   if (global.GetHidden("guestViewInternal", &guest_view_internal)) {
     api::context_bridge::ObjectCache object_cache;
     auto result = api::PassValueToOtherContext(
-        source_context, context, guest_view_internal, &object_cache, false, 0,
-        api::BridgeErrorTarget::kSource);
+        source_context, context, guest_view_internal, source_context->Global(),
+        &object_cache, false, 0, api::BridgeErrorTarget::kSource);
     if (!result.IsEmpty()) {
       isolated_api.Set("guestViewInternal", result.ToLocalChecked());
     }

spec/webview-spec.ts

@@ -2020,25 +2020,34 @@ describe('<webview> tag', function () {
 
     // TODO(miniak): figure out why this is failing on windows
     ifdescribe(process.platform !== 'win32')('<webview>.capturePage()', () => {
-      it('returns a Promise with a NativeImage', async () => {
+      it('returns a Promise with a NativeImage', async function () {
+        this.retries(5);
+
         const src = 'data:text/html,%3Ch1%3EHello%2C%20World!%3C%2Fh1%3E';
         await loadWebViewAndWaitForEvent(w, { src }, 'did-stop-loading');
 
-        // Retry a few times due to flake.
-        for (let i = 0; i < 5; i++) {
-          try {
-            const image = await w.executeJavaScript('webview.capturePage()');
-            const imgBuffer = image.toPNG();
-
-            // Check the 25th byte in the PNG.
-            // Values can be 0,2,3,4, or 6. We want 6, which is RGB + Alpha
-            expect(imgBuffer[25]).to.equal(6);
-            return;
-          } catch {
-            /* drop the error */
-          }
-        }
-        expect(false).to.be.true('could not successfully capture the page');
+        const image = await w.executeJavaScript('webview.capturePage()');
+        expect(image.isEmpty()).to.be.false();
+
+        // Check the 25th byte in the PNG.
+        // Values can be 0,2,3,4, or 6. We want 6, which is RGB + Alpha
+        const imgBuffer = image.toPNG();
+        expect(imgBuffer[25]).to.equal(6);
+      });
+
+      it('returns a Promise with a NativeImage in the renderer', async function () {
+        this.retries(5);
+
+        const src = 'data:text/html,%3Ch1%3EHello%2C%20World!%3C%2Fh1%3E';
+        await loadWebViewAndWaitForEvent(w, { src }, 'did-stop-loading');
+
+        const byte = await w.executeJavaScript(`new Promise(resolve => {
+          webview.capturePage().then(image => {
+            resolve(image.toPNG()[25])
+          });
+        })`);
+
+        expect(byte).to.equal(6);
       });
     });