Browse Source

fix: pass rfh instances through to the permission helper (#35419)

* fix: pass rfh instances through to the permission helper

* refactor: use WeakDocumentPtr instead of frame node id

* fix: handle missing initiator document

* fix: dispatch openExternal event for top level webview navs still
Samuel Attard 2 years ago
parent
commit
f65b05b8cc

+ 1 - 1
docs/api/session.md

@@ -635,7 +635,7 @@ win.webContents.session.setCertificateVerifyProc((request, callback) => {
     * `notifications` - Request notification creation and the ability to display them in the user's system tray.
     * `midi` - Request MIDI access in the `webmidi` API.
     * `midiSysex` - Request the use of system exclusive messages in the `webmidi` API.
-    * `pointerLock` - Request to directly interpret mouse movements as an input method. Click [here](https://developer.mozilla.org/en-US/docs/Web/API/Pointer_Lock_API) to know more.
+    * `pointerLock` - Request to directly interpret mouse movements as an input method. Click [here](https://developer.mozilla.org/en-US/docs/Web/API/Pointer_Lock_API) to know more. These requests always appear to originate from the main frame.
     * `fullscreen` - Request for the app to enter fullscreen mode.
     * `openExternal` - Request to open links in external applications.
     * `unknown` - An unrecognized permission request

+ 1 - 1
shell/browser/api/electron_api_web_contents.cc

@@ -1312,7 +1312,7 @@ void WebContents::EnterFullscreenModeForTab(
   auto callback =
       base::BindRepeating(&WebContents::OnEnterFullscreenModeForTab,
                           base::Unretained(this), requesting_frame, options);
-  permission_helper->RequestFullscreenPermission(callback);
+  permission_helper->RequestFullscreenPermission(requesting_frame, callback);
 }
 
 void WebContents::OnEnterFullscreenModeForTab(

+ 16 - 2
shell/browser/electron_browser_client.cc

@@ -49,6 +49,7 @@
 #include "content/public/browser/tts_controller.h"
 #include "content/public/browser/tts_platform.h"
 #include "content/public/browser/url_loader_request_interceptor.h"
+#include "content/public/browser/weak_document_ptr.h"
 #include "content/public/common/content_descriptors.h"
 #include "content/public/common/content_paths.h"
 #include "content/public/common/content_switches.h"
@@ -987,7 +988,7 @@ void ElectronBrowserClient::WebNotificationAllowed(
     return;
   }
   permission_helper->RequestWebNotificationPermission(
-      base::BindOnce(std::move(callback), web_contents->IsAudioMuted()));
+      rfh, base::BindOnce(std::move(callback), web_contents->IsAudioMuted()));
 }
 
 void ElectronBrowserClient::RenderProcessHostDestroyed(
@@ -1022,6 +1023,7 @@ void OnOpenExternal(const GURL& escaped_url, bool allowed) {
 
 void HandleExternalProtocolInUI(
     const GURL& url,
+    content::WeakDocumentPtr document_ptr,
     content::WebContents::OnceGetter web_contents_getter,
     bool has_user_gesture) {
   content::WebContents* web_contents = std::move(web_contents_getter).Run();
@@ -1033,9 +1035,18 @@ void HandleExternalProtocolInUI(
   if (!permission_helper)
     return;
 
+  content::RenderFrameHost* rfh = document_ptr.AsRenderFrameHostIfValid();
+  if (!rfh) {
+    // If the render frame host is not valid it means it was a top level
+    // navigation and the frame has already been disposed of.  In this case we
+    // take the current main frame and declare it responsible for the
+    // transition.
+    rfh = web_contents->GetPrimaryMainFrame();
+  }
+
   GURL escaped_url(base::EscapeExternalHandlerValue(url.spec()));
   auto callback = base::BindOnce(&OnOpenExternal, escaped_url);
-  permission_helper->RequestOpenExternalPermission(std::move(callback),
+  permission_helper->RequestOpenExternalPermission(rfh, std::move(callback),
                                                    has_user_gesture, url);
 }
 
@@ -1055,6 +1066,9 @@ bool ElectronBrowserClient::HandleExternalProtocol(
   content::GetUIThreadTaskRunner({})->PostTask(
       FROM_HERE,
       base::BindOnce(&HandleExternalProtocolInUI, url,
+                     initiator_document
+                         ? initiator_document->GetWeakDocumentPtr()
+                         : content::WeakDocumentPtr(),
                      std::move(web_contents_getter), has_user_gesture));
   return true;
 }

+ 13 - 4
shell/browser/web_contents_permission_helper.cc

@@ -173,16 +173,16 @@ WebContentsPermissionHelper::WebContentsPermissionHelper(
 WebContentsPermissionHelper::~WebContentsPermissionHelper() = default;
 
 void WebContentsPermissionHelper::RequestPermission(
+    content::RenderFrameHost* requesting_frame,
     blink::PermissionType permission,
     base::OnceCallback<void(bool)> callback,
     bool user_gesture,
     base::Value::Dict details) {
-  auto* rfh = web_contents_->GetPrimaryMainFrame();
   auto* permission_manager = static_cast<ElectronPermissionManager*>(
       web_contents_->GetBrowserContext()->GetPermissionControllerDelegate());
   auto origin = web_contents_->GetLastCommittedURL();
   permission_manager->RequestPermissionWithDetails(
-      permission, rfh, origin, false, std::move(details),
+      permission, requesting_frame, origin, false, std::move(details),
       base::BindOnce(&OnPermissionResponse, std::move(callback)));
 }
 
@@ -198,8 +198,10 @@ bool WebContentsPermissionHelper::CheckPermission(
 }
 
 void WebContentsPermissionHelper::RequestFullscreenPermission(
+    content::RenderFrameHost* requesting_frame,
     base::OnceCallback<void(bool)> callback) {
   RequestPermission(
+      requesting_frame,
       static_cast<blink::PermissionType>(PermissionType::FULLSCREEN),
       std::move(callback));
 }
@@ -225,13 +227,17 @@ void WebContentsPermissionHelper::RequestMediaAccessPermission(
 
   // The permission type doesn't matter here, AUDIO_CAPTURE/VIDEO_CAPTURE
   // are presented as same type in content_converter.h.
-  RequestPermission(blink::PermissionType::AUDIO_CAPTURE, std::move(callback),
+  RequestPermission(content::RenderFrameHost::FromID(request.render_process_id,
+                                                     request.render_frame_id),
+                    blink::PermissionType::AUDIO_CAPTURE, std::move(callback),
                     false, std::move(details));
 }
 
 void WebContentsPermissionHelper::RequestWebNotificationPermission(
+    content::RenderFrameHost* requesting_frame,
     base::OnceCallback<void(bool)> callback) {
-  RequestPermission(blink::PermissionType::NOTIFICATIONS, std::move(callback));
+  RequestPermission(requesting_frame, blink::PermissionType::NOTIFICATIONS,
+                    std::move(callback));
 }
 
 void WebContentsPermissionHelper::RequestPointerLockPermission(
@@ -240,6 +246,7 @@ void WebContentsPermissionHelper::RequestPointerLockPermission(
     base::OnceCallback<void(content::WebContents*, bool, bool, bool)>
         callback) {
   RequestPermission(
+      web_contents_->GetPrimaryMainFrame(),
       static_cast<blink::PermissionType>(PermissionType::POINTER_LOCK),
       base::BindOnce(std::move(callback), web_contents_, user_gesture,
                      last_unlocked_by_target),
@@ -247,12 +254,14 @@ void WebContentsPermissionHelper::RequestPointerLockPermission(
 }
 
 void WebContentsPermissionHelper::RequestOpenExternalPermission(
+    content::RenderFrameHost* requesting_frame,
     base::OnceCallback<void(bool)> callback,
     bool user_gesture,
     const GURL& url) {
   base::Value::Dict details;
   details.Set("externalURL", url.spec());
   RequestPermission(
+      requesting_frame,
       static_cast<blink::PermissionType>(PermissionType::OPEN_EXTERNAL),
       std::move(callback), user_gesture, std::move(details));
 }

+ 7 - 3
shell/browser/web_contents_permission_helper.h

@@ -33,7 +33,8 @@ class WebContentsPermissionHelper
   };
 
   // Asynchronous Requests
-  void RequestFullscreenPermission(base::OnceCallback<void(bool)> callback);
+  void RequestFullscreenPermission(content::RenderFrameHost* requesting_frame,
+                                   base::OnceCallback<void(bool)> callback);
   void RequestMediaAccessPermission(const content::MediaStreamRequest& request,
                                     content::MediaResponseCallback callback);
   void RequestPointerLockPermission(
@@ -42,8 +43,10 @@ class WebContentsPermissionHelper
       base::OnceCallback<void(content::WebContents*, bool, bool, bool)>
           callback);
   void RequestWebNotificationPermission(
+      content::RenderFrameHost* requesting_frame,
       base::OnceCallback<void(bool)> callback);
-  void RequestOpenExternalPermission(base::OnceCallback<void(bool)> callback,
+  void RequestOpenExternalPermission(content::RenderFrameHost* requesting_frame,
+                                     base::OnceCallback<void(bool)> callback,
                                      bool user_gesture,
                                      const GURL& url);
 
@@ -56,7 +59,8 @@ class WebContentsPermissionHelper
   explicit WebContentsPermissionHelper(content::WebContents* web_contents);
   friend class content::WebContentsUserData<WebContentsPermissionHelper>;
 
-  void RequestPermission(blink::PermissionType permission,
+  void RequestPermission(content::RenderFrameHost* requesting_frame,
+                         blink::PermissionType permission,
                          base::OnceCallback<void(bool)> callback,
                          bool user_gesture = false,
                          base::Value::Dict details = {});