feat: provide the frame URL with permission requests and checks (#18757)

* feat: provide the frame URL with permission requests and checks

Also provides a handy isMainFrame property to determine if it is an
iframe making the request

* chore: refactor to use base::Value

* chore: use Set<Type>Key over SetPath

atom/browser/api/atom_api_session.cc

@@ -442,11 +442,6 @@ void Session::SetCertVerifyProc(v8::Local<v8::Value> val,
 
 void Session::SetPermissionRequestHandler(v8::Local<v8::Value> val,
                                           mate::Arguments* args) {
-  using StatusCallback =
-      base::RepeatingCallback<void(blink::mojom::PermissionStatus)>;
-  using RequestHandler =
-      base::Callback<void(content::WebContents*, content::PermissionType,
-                          StatusCallback, const base::DictionaryValue&)>;
   auto* permission_manager = static_cast<AtomPermissionManager*>(
       browser_context()->GetPermissionControllerDelegate());
   if (val->IsNull()) {
@@ -454,16 +449,17 @@ void Session::SetPermissionRequestHandler(v8::Local<v8::Value> val,
         AtomPermissionManager::RequestHandler());
     return;
   }
-  auto handler = std::make_unique<RequestHandler>();
+  auto handler = std::make_unique<AtomPermissionManager::RequestHandler>();
   if (!mate::ConvertFromV8(args->isolate(), val, handler.get())) {
     args->ThrowError("Must pass null or function");
     return;
   }
   permission_manager->SetPermissionRequestHandler(base::BindRepeating(
-      [](RequestHandler* handler, content::WebContents* web_contents,
+      [](AtomPermissionManager::RequestHandler* handler,
+         content::WebContents* web_contents,
          content::PermissionType permission_type,
          AtomPermissionManager::StatusCallback callback,
-         const base::DictionaryValue& details) {
+         const base::Value& details) {
         handler->Run(web_contents, permission_type,
                      base::AdaptCallbackForRepeating(std::move(callback)),
                      details);

atom/browser/atom_permission_manager.cc

@@ -188,12 +188,13 @@ int AtomPermissionManager::RequestPermissionsWithDetails(
     const auto callback =
         base::BindRepeating(&AtomPermissionManager::OnPermissionResponse,
                             base::Unretained(this), request_id, i);
-    if (details == nullptr) {
-      request_handler_.Run(web_contents, permission, callback,
-                           base::DictionaryValue());
-    } else {
-      request_handler_.Run(web_contents, permission, callback, *details);
-    }
+    auto mutable_details =
+        details == nullptr ? base::DictionaryValue() : details->Clone();
+    mutable_details.SetStringKey(
+        "requestingUrl", render_frame_host->GetLastCommittedURL().spec());
+    mutable_details.SetBoolKey("isMainFrame",
+                               render_frame_host->GetParent() == nullptr);
+    request_handler_.Run(web_contents, permission, callback, mutable_details);
   }
 
   return request_id;
@@ -246,8 +247,14 @@ bool AtomPermissionManager::CheckPermissionWithDetails(
   }
   auto* web_contents =
       content::WebContents::FromRenderFrameHost(render_frame_host);
+  auto mutable_details =
+      details == nullptr ? base::DictionaryValue() : details->Clone();
+  mutable_details.SetStringKey("requestingUrl",
+                               render_frame_host->GetLastCommittedURL().spec());
+  mutable_details.SetBoolKey("isMainFrame",
+                             render_frame_host->GetParent() == nullptr);
   return check_handler_.Run(web_contents, permission, requesting_origin,
-                            *details);
+                            mutable_details);
 }
 
 blink::mojom::PermissionStatus

atom/browser/atom_permission_manager.h

@@ -32,11 +32,11 @@ class AtomPermissionManager : public content::PermissionControllerDelegate {
   using RequestHandler = base::Callback<void(content::WebContents*,
                                              content::PermissionType,
                                              StatusCallback,
-                                             const base::DictionaryValue&)>;
+                                             const base::Value&)>;
   using CheckHandler = base::Callback<bool(content::WebContents*,
                                            content::PermissionType,
                                            const GURL& requesting_origin,
-                                           const base::DictionaryValue&)>;
+                                           const base::Value&)>;
 
   // Handler to dispatch permission requests in JS.
   void SetPermissionRequestHandler(const RequestHandler& handler);

docs/api/session.md

@@ -284,15 +284,17 @@ win.webContents.session.setCertificateVerifyProc((request, callback) => {
 #### `ses.setPermissionRequestHandler(handler)`
 
 * `handler` Function | null
-  * `webContents` [WebContents](web-contents.md) - WebContents requesting the permission.
+  * `webContents` [WebContents](web-contents.md) - WebContents requesting the permission.  Please note that if the request comes from a subframe you should use `requestingUrl` to check the request origin.
   * `permission` String - Enum of 'media', 'geolocation', 'notifications', 'midiSysex',
     'pointerLock', 'fullscreen', 'openExternal'.
   * `callback` Function
     * `permissionGranted` Boolean - Allow or deny the permission.
   * `details` Object - Some properties are only available on certain permission types.
-    * `externalURL` String - The url of the `openExternal` request.
-    * `mediaTypes` String[] - The types of media access being requested, elements can be `video`
+    * `externalURL` String (Optional) - The url of the `openExternal` request.
+    * `mediaTypes` String[] (Optional) - The types of media access being requested, elements can be `video`
       or `audio`
+    * `requestingUrl` String - The last URL the requesting frame loaded
+    * `isMainFrame` Boolean - Whether the frame making the request is the main frame
 
 Sets the handler which can be used to respond to permission requests for the `session`.
 Calling `callback(true)` will allow the permission and `callback(false)` will reject it.
@@ -312,13 +314,15 @@ session.fromPartition('some-partition').setPermissionRequestHandler((webContents
 #### `ses.setPermissionCheckHandler(handler)`
 
 * `handler` Function<Boolean> | null
-  * `webContents` [WebContents](web-contents.md) - WebContents checking the permission.
+  * `webContents` [WebContents](web-contents.md) - WebContents checking the permission.  Please note that if the request comes from a subframe you should use `requestingUrl` to check the request origin.
   * `permission` String - Enum of 'media'.
   * `requestingOrigin` String - The origin URL of the permission check
   * `details` Object - Some properties are only available on certain permission types.
     * `securityOrigin` String - The security orign of the `media` check.
     * `mediaType` String - The type of media access being requested, can be `video`,
       `audio` or `unknown`
+    * `requestingUrl` String - The last URL the requesting frame loaded
+    * `isMainFrame` Boolean - Whether the frame making the request is the main frame
 
 Sets the handler which can be used to respond to permission checks for the `session`.
 Returning `true` will allow the permission and `false` will reject it.