expose handler from session and remove permission-request event

atom/browser/api/atom_api_session.cc

@@ -14,8 +14,10 @@
 #include "atom/browser/api/save_page_handler.h"
 #include "atom/browser/atom_browser_context.h"
 #include "atom/browser/atom_browser_main_parts.h"
+#include "atom/browser/atom_permission_manager.h"
 #include "atom/browser/net/atom_cert_verifier.h"
 #include "atom/common/native_mate_converters/callback.h"
+#include "atom/common/native_mate_converters/content_converter.h"
 #include "atom/common/native_mate_converters/gurl_converter.h"
 #include "atom/common/native_mate_converters/file_path_converter.h"
 #include "atom/common/native_mate_converters/net_converter.h"
@@ -397,6 +399,18 @@ void Session::SetCertVerifyProc(v8::Local<v8::Value> val,
   browser_context_->cert_verifier()->SetVerifyProc(proc);
 }
 
+void Session::SetPermissionRequestHandler(v8::Local<v8::Value> val,
+                                              mate::Arguments* args) {
+  AtomPermissionManager::RequestHandler handler;
+  if (!(val->IsNull() || mate::ConvertFromV8(args->isolate(), val, &handler))) {
+    args->ThrowError("Must pass null or function");
+    return;
+  }
+  auto permission_manager = static_cast<AtomPermissionManager*>(
+      browser_context()->GetPermissionManager());
+  permission_manager->SetPermissionRequestHandler(handler);
+}
+
 v8::Local<v8::Value> Session::Cookies(v8::Isolate* isolate) {
   if (cookies_.IsEmpty()) {
     auto handle = atom::api::Cookies::Create(isolate, browser_context());
@@ -448,6 +462,8 @@ void Session::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("enableNetworkEmulation", &Session::EnableNetworkEmulation)
       .SetMethod("disableNetworkEmulation", &Session::DisableNetworkEmulation)
       .SetMethod("setCertificateVerifyProc", &Session::SetCertVerifyProc)
+      .SetMethod("setPermissionRequestHandler",
+                 &Session::SetPermissionRequestHandler)
       .SetProperty("cookies", &Session::Cookies)
       .SetProperty("webRequest", &Session::WebRequest);
 }

atom/browser/api/atom_api_session.h

@@ -76,6 +76,8 @@ class Session: public mate::TrackableObject<Session>,
   void EnableNetworkEmulation(const mate::Dictionary& options);
   void DisableNetworkEmulation();
   void SetCertVerifyProc(v8::Local<v8::Value> proc, mate::Arguments* args);
+  void SetPermissionRequestHandler(v8::Local<v8::Value> val,
+                                   mate::Arguments* args);
   v8::Local<v8::Value> Cookies(v8::Isolate* isolate);
   v8::Local<v8::Value> WebRequest(v8::Isolate* isolate);
 

atom/browser/api/atom_api_web_contents.cc

@@ -13,7 +13,6 @@
 #include "atom/browser/atom_browser_client.h"
 #include "atom/browser/atom_browser_context.h"
 #include "atom/browser/atom_browser_main_parts.h"
-#include "atom/browser/atom_permission_manager.h"
 #include "atom/browser/native_window.h"
 #include "atom/browser/web_contents_permission_helper.h"
 #include "atom/browser/web_contents_preferences.h"
@@ -1067,18 +1066,6 @@ bool WebContents::IsGuest() const {
   return type_ == WEB_VIEW;
 }
 
-void WebContents::SetPermissionRequestHandler(v8::Local<v8::Value> val,
-                                              mate::Arguments* args) {
-  AtomPermissionManager::RequestHandler handler;
-  if (!(val->IsNull() || mate::ConvertFromV8(args->isolate(), val, &handler))) {
-    args->ThrowError("Must pass null or function");
-    return;
-  }
-  auto permission_manager = static_cast<AtomPermissionManager*>(
-      web_contents()->GetBrowserContext()->GetPermissionManager());
-  permission_manager->SetPermissionRequestHandler(GetID(), handler);
-}
-
 v8::Local<v8::Value> WebContents::GetWebPreferences(v8::Isolate* isolate) {
   WebContentsPreferences* web_preferences =
       WebContentsPreferences::FromWebContents(web_contents());
@@ -1178,8 +1165,6 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("_printToPDF", &WebContents::PrintToPDF)
       .SetMethod("addWorkSpace", &WebContents::AddWorkSpace)
       .SetMethod("removeWorkSpace", &WebContents::RemoveWorkSpace)
-      .SetMethod("_setPermissionRequestHandler",
-                 &WebContents::SetPermissionRequestHandler)
       .SetProperty("session", &WebContents::Session)
       .SetProperty("devToolsWebContents", &WebContents::DevToolsWebContents)
       .SetProperty("debugger", &WebContents::Debugger);

atom/browser/api/atom_api_web_contents.h

@@ -133,10 +133,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
   void SetAllowTransparency(bool allow);
   bool IsGuest() const;
 
-  // Handler for permission requests.
-  void SetPermissionRequestHandler(v8::Local<v8::Value> val,
-                                   mate::Arguments* args);
-
   // Returns the web preferences of current WebContents.
   v8::Local<v8::Value> GetWebPreferences(v8::Isolate* isolate);
 

atom/browser/atom_permission_manager.cc

@@ -8,6 +8,7 @@
 #include "content/public/browser/permission_type.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/web_contents.h"
 
 namespace atom {
 
@@ -19,33 +20,13 @@ AtomPermissionManager::~AtomPermissionManager() {
 }
 
 void AtomPermissionManager::SetPermissionRequestHandler(
-    int id,
     const RequestHandler& handler) {
-  if (handler.is_null()) {
-    request_handler_map_.erase(id);
-    return;
+  if (handler.is_null() && !pending_requests_.empty()) {
+    for (const auto& request : pending_requests_)
+      request.second.Run(content::PERMISSION_STATUS_DENIED);
+    pending_requests_.clear();
   }
-  request_handler_map_[id] = handler;
-}
-
-void AtomPermissionManager::RequestPermission(
-    content::PermissionType permission,
-    content::RenderFrameHost* render_frame_host,
-    const GURL& origin,
-    const base::Callback<void(bool)>& callback) {
-  bool user_gesture = false;
-  RequestPermission(permission, render_frame_host, origin, user_gesture,
-                    base::Bind(&AtomPermissionManager::OnPermissionResponse,
-                               base::Unretained(this), callback));
-}
-
-void AtomPermissionManager::OnPermissionResponse(
-    const base::Callback<void(bool)>& callback,
-    content::PermissionStatus status) {
-  if (status == content::PERMISSION_STATUS_GRANTED)
-    callback.Run(true);
-  else
-    callback.Run(false);
+  request_handler_ = handler;
 }
 
 int AtomPermissionManager::RequestPermission(
@@ -53,25 +34,39 @@ int AtomPermissionManager::RequestPermission(
     content::RenderFrameHost* render_frame_host,
     const GURL& requesting_origin,
     bool user_gesture,
-    const ResponseCallback& callback) {
-  int process_id = render_frame_host->GetProcess()->GetID();
-  auto request_handler = request_handler_map_.find(process_id);
-
+    const ResponseCallback& response_callback) {
   if (permission == content::PermissionType::MIDI_SYSEX) {
     content::ChildProcessSecurityPolicy::GetInstance()->
-        GrantSendMidiSysExMessage(process_id);
+        GrantSendMidiSysExMessage(render_frame_host->GetProcess()->GetID());
   }
 
-  if (request_handler != request_handler_map_.end()) {
-    pending_requests_[++request_id_] = callback;
-    request_handler->second.Run(permission, callback);
+  if (!request_handler_.is_null()) {
+    auto web_contents =
+        content::WebContents::FromRenderFrameHost(render_frame_host);
+    ++request_id_;
+    auto callback = base::Bind(&AtomPermissionManager::OnPermissionResponse,
+                               base::Unretained(this),
+                               request_id_,
+                               requesting_origin,
+                               response_callback);
+    pending_requests_[request_id_] = callback;
+    request_handler_.Run(web_contents, permission, callback);
     return request_id_;
   }
 
-  callback.Run(content::PERMISSION_STATUS_GRANTED);
+  response_callback.Run(content::PERMISSION_STATUS_GRANTED);
   return kNoPendingOperation;
 }
 
+void AtomPermissionManager::OnPermissionResponse(
+    int request_id,
+    const GURL& origin,
+    const ResponseCallback& callback,
+    content::PermissionStatus status) {
+  callback.Run(status);
+  pending_requests_.erase(request_id);
+}
+
 void AtomPermissionManager::CancelPermissionRequest(int request_id) {
   auto request = pending_requests_.find(request_id);
   if (request != pending_requests_.end()) {

atom/browser/atom_permission_manager.h

@@ -7,10 +7,13 @@
 
 #include <map>
 
-#include "base/callback_forward.h"
-#include "base/macros.h"
+#include "base/callback.h"
 #include "content/public/browser/permission_manager.h"
 
+namespace content {
+class WebContents;
+}
+
 namespace atom {
 
 class AtomPermissionManager : public content::PermissionManager {
@@ -21,22 +24,13 @@ class AtomPermissionManager : public content::PermissionManager {
   using ResponseCallback =
       base::Callback<void(content::PermissionStatus)>;
   using RequestHandler =
-      base::Callback<void(content::PermissionType,
+      base::Callback<void(content::WebContents*,
+                          content::PermissionType,
                           const ResponseCallback&)>;
 
   // Handler to dispatch permission requests in JS.
-  void SetPermissionRequestHandler(int id, const RequestHandler& handler);
+  void SetPermissionRequestHandler(const RequestHandler& handler);
 
-  void RequestPermission(
-      content::PermissionType permission,
-      content::RenderFrameHost* render_frame_host,
-      const GURL& origin,
-      const base::Callback<void(bool)>& callback);
-  void OnPermissionResponse(
-      const base::Callback<void(bool)>& callback,
-      content::PermissionStatus status);
-
- protected:
   // content::PermissionManager:
   int RequestPermission(
       content::PermissionType permission,
@@ -44,6 +38,14 @@ class AtomPermissionManager : public content::PermissionManager {
       const GURL& requesting_origin,
       bool user_gesture,
       const ResponseCallback& callback) override;
+
+ protected:
+  void OnPermissionResponse(int request_id,
+                            const GURL& url,
+                            const ResponseCallback& callback,
+                            content::PermissionStatus status);
+
+  // content::PermissionManager:
   void CancelPermissionRequest(int request_id) override;
   void ResetPermission(content::PermissionType permission,
                        const GURL& requesting_origin,
@@ -63,7 +65,7 @@ class AtomPermissionManager : public content::PermissionManager {
   void UnsubscribePermissionStatusChange(int subscription_id) override;
 
  private:
-  std::map<int, RequestHandler> request_handler_map_;
+  RequestHandler request_handler_;
 
   std::map<int, ResponseCallback> pending_requests_;
 

atom/browser/lib/guest-view-manager.js

@@ -42,7 +42,6 @@ var supportedWebViewEvents = [
 var nextInstanceId = 0;
 var guestInstances = {};
 var embedderElementsMap = {};
-var pendingRequestsMap = {};
 var reverseEmbedderElementsMap = {};
 
 // Moves the last element of array to the first one.
@@ -135,15 +134,7 @@ var createGuest = function(embedder, params) {
     if (params.allowtransparency != null) {
       this.setAllowTransparency(params.allowtransparency);
     }
-    guest.allowPopups = params.allowpopups;
-
-    // Dispatches permission request event.
-    this._setPermissionRequestHandler((permission, callback) => {
-      if (!pendingRequestsMap[this.viewInstanceId])
-        pendingRequestsMap[this.viewInstanceId] = {};
-      pendingRequestsMap[this.viewInstanceId][permission] = callback;
-      embedder.send.apply(embedder, ["ATOM_SHELL_GUEST_VIEW_INTERNAL_DISPATCH_EVENT-" + this.viewInstanceId, "permission-request", permission]);
-    });
+    return guest.allowPopups = params.allowpopups;
   });
 
   // Dispatch events to embedder.
@@ -170,7 +161,6 @@ var createGuest = function(embedder, params) {
     var args = 2 <= arguments.length ? slice.call(arguments, 1) : [];
     return embedder.send.apply(embedder, ["ATOM_SHELL_GUEST_VIEW_INTERNAL_SIZE_CHANGED-" + guest.viewInstanceId].concat(slice.call(args)));
   });
-
   return id;
 };
 
@@ -198,8 +188,7 @@ var attachGuest = function(embedder, elementInstanceId, guestInstanceId, params)
     nodeIntegration: (ref1 = params.nodeintegration) != null ? ref1 : false,
     plugins: params.plugins,
     webSecurity: !params.disablewebsecurity,
-    blinkFeatures: params.blinkfeatures,
-    webNotification: !params.disablewebnotification,
+    blinkFeatures: params.blinkfeatures
   };
   if (params.preload) {
     webPreferences.preloadURL = params.preload;
@@ -216,7 +205,6 @@ var destroyGuest = function(embedder, id) {
   webViewManager.removeGuest(embedder, id);
   guestInstances[id].guest.destroy();
   delete guestInstances[id];
-  delete pendingRequestsMap[id];
   key = reverseEmbedderElementsMap[id];
   if (key != null) {
     delete reverseEmbedderElementsMap[id];
@@ -246,13 +234,6 @@ ipcMain.on('ATOM_SHELL_GUEST_VIEW_MANAGER_SET_ALLOW_TRANSPARENCY', function(even
   return (ref1 = guestInstances[id]) != null ? ref1.guest.setAllowTransparency(allowtransparency) : void 0;
 });
 
-ipcMain.on('ATOM_SHELL_GUEST_VIEW_MANAGER_SET_PERMISSION_RESPONSE', function(event, id, permission, allowed) {
-  if (pendingRequestsMap[id] != null) {
-    const callback = pendingRequestsMap[id][permission];
-    callback.apply(null, [allowed]);
-  }
-});
-
 // Returns WebContents from its guest id.
 exports.getGuest = function(id) {
   var ref1;

atom/browser/web_contents_permission_helper.cc

@@ -66,6 +66,14 @@ void MediaAccessAllowed(
   callback.Run(devices, result, scoped_ptr<content::MediaStreamUI>());
 }
 
+void OnPermissionResponse(const base::Callback<void(bool)>& callback,
+                          content::PermissionStatus status) {
+  if (status == content::PERMISSION_STATUS_GRANTED)
+    callback.Run(true);
+  else
+    callback.Run(false);
+}
+
 }  // namespace
 
 WebContentsPermissionHelper::WebContentsPermissionHelper(
@@ -83,13 +91,18 @@ void WebContentsPermissionHelper::RequestPermission(
   auto permission_manager = static_cast<AtomPermissionManager*>(
       web_contents_->GetBrowserContext()->GetPermissionManager());
   auto origin = web_contents_->GetLastCommittedURL();
-  permission_manager->RequestPermission(permission, rfh, origin, callback);
+  bool user_gesture = false;
+  permission_manager->RequestPermission(
+      permission, rfh, origin, user_gesture,
+      base::Bind(&OnPermissionResponse, callback));
 }
 
 void WebContentsPermissionHelper::RequestMediaAccessPermission(
     const content::MediaStreamRequest& request,
     const content::MediaResponseCallback& response_callback) {
   auto callback = base::Bind(&MediaAccessAllowed, request, response_callback);
+  // The permission type doesn't matter here, AUDIO_CAPTURE/VIDEO_CAPTURE
+  // are presented as same type in content_converter.h.
   RequestPermission(content::PermissionType::AUDIO_CAPTURE, callback);
 }
 

atom/common/native_mate_converters/content_converter.cc

@@ -7,6 +7,7 @@
 #include <string>
 #include <vector>
 
+#include "atom/browser/api/atom_api_web_contents.h"
 #include "atom/common/native_mate_converters/callback.h"
 #include "atom/common/native_mate_converters/string16_converter.h"
 #include "content/public/browser/web_contents.h"
@@ -163,4 +164,10 @@ bool Converter<content::StopFindAction>::FromV8(
   return true;
 }
 
+// static
+v8::Local<v8::Value> Converter<content::WebContents*>::ToV8(
+    v8::Isolate* isolate, content::WebContents* val) {
+  return atom::api::WebContents::CreateFrom(isolate, val).ToV8();
+}
+
 }  // namespace mate

atom/common/native_mate_converters/content_converter.h

@@ -53,6 +53,12 @@ struct Converter<content::StopFindAction> {
                      content::StopFindAction* out);
 };
 
+template<>
+struct Converter<content::WebContents*> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   content::WebContents* val);
+};
+
 }  // namespace mate
 
 #endif  // ATOM_COMMON_NATIVE_MATE_CONVERTERS_CONTENT_CONVERTER_H_

atom/renderer/lib/web-view/guest-view-internal.js

@@ -34,8 +34,7 @@ var WEB_VIEW_EVENTS = {
   'page-favicon-updated': ['favicons'],
   'enter-html-full-screen': [],
   'leave-html-full-screen': [],
-  'found-in-page': ['result'],
-  'permission-request': ['permission', 'allow', 'deny']
+  'found-in-page': ['result']
 };
 
 var DEPRECATED_EVENTS = {
@@ -65,15 +64,6 @@ module.exports = {
     ipcRenderer.on("ATOM_SHELL_GUEST_VIEW_INTERNAL_DISPATCH_EVENT-" + viewInstanceId, function() {
       var eventName = arguments[1];
       var args = 3 <= arguments.length ? slice.call(arguments, 2) : [];
-      if (eventName === 'permission-request') {
-        var allow = function allow() {
-          ipcRenderer.send("ATOM_SHELL_GUEST_VIEW_MANAGER_SET_PERMISSION_RESPONSE", viewInstanceId, args[0], "granted");
-        };
-        var deny = function deny() {
-          ipcRenderer.send("ATOM_SHELL_GUEST_VIEW_MANAGER_SET_PERMISSION_RESPONSE", viewInstanceId, args[0], "denied");
-        };
-        args = args.concat([allow, deny]);
-      }
       return dispatchEvent.apply(null, [webView, eventName, eventName].concat(slice.call(args)));
     });
     ipcRenderer.on("ATOM_SHELL_GUEST_VIEW_INTERNAL_IPC_MESSAGE-" + viewInstanceId, function() {

docs/api/session.md

@@ -289,6 +289,29 @@ myWindow.webContents.session.setCertificateVerifyProc(function(hostname, cert, c
 });
 ```
 
+#### `ses.setPermissionRequestHandler(handler)`
+
+* `handler` Function
+  * `webContents` Object - [WebContents](web-contents.md) requesting the permission.
+  * `permission`  String - Enum of 'media', 'geolocation', 'notifications', 'midiSysex'.
+  * `callback`  Function - Allow or deny the permission.
+
+Sets the handler which can be used to respond to permission requests for the `session`.
+Calling `callback('granted')` will allow the permission and `callback('denied')` will reject it.
+
+```javascript
+session.fromPartition(partition).setPermissionRequestHandler(function(webContents, permission, callback) {
+  if (webContents.getURL() === host) {
+    if (permission == "notifications") {
+      callback(); // denied.
+      return;
+    }
+  }
+
+  callback('granted');
+});
+```
+
 #### `ses.webRequest`
 
 The `webRequest` API set allows to intercept and modify contents of a request at

docs/api/web-view-tag.md

@@ -736,22 +736,4 @@ Emitted when DevTools is closed.
 
 Emitted when DevTools is focused / opened.
 
-### Event: 'permission-request'
-
-Returns:
-
-* `permission` String - The type of permission being requested. Enum of 'media', 'notifications', 'midiSysex', 'geolocation'.
-* `allow` Function - Allows the permission.
-* `deny` Function - Deny the permission. This is the default behaviour if `allow` is not called.
-
-Emitted when guest page requires special permission.
-
-```javascript
-// This will deny guest page access to the webkitGetUserMedia API.
-webview.addEventListener('permission-request', function(e) {
-  if (e.permission === 'media')
-    e.deny();
-});
-```
-
 [blink-feature-string]: https://code.google.com/p/chromium/codesearch#chromium/src/out/Debug/gen/blink/platform/RuntimeEnabledFeatures.cpp&sq=package:chromium&type=cs&l=527

spec/webview-spec.js

@@ -634,19 +634,27 @@ describe('<webview> tag', function() {
     });
   });
   describe('permission-request event', function() {
+    function setUpRequestHandler(webview, requested_permission) {
+      const session = require('electron').remote.session;
+      var listener = function(webContents, permission, callback) {
+        if (webContents.getId() === webview.getId() ) {
+          assert.equal(permission, requested_permission);
+          callback("denied");
+        }
+      };
+      session.fromPartition(webview.partition).setPermissionRequestHandler(listener);
+    }
+
     it ('emits when using navigator.getUserMedia api', function(done) {
       webview.addEventListener('ipc-message', function(e) {
         assert(e.channel, 'message');
         assert(e.args, ['PermissionDeniedError']);
         done();
       });
-      webview.addEventListener('permission-request', function(e) {
-        if (e.permission === 'media') {
-          e.deny();
-        }
-      });
       webview.src = "file://" + fixtures + "/pages/permissions/media.html";
+      webview.partition = "permissionTest";
       webview.setAttribute('nodeintegration', 'on');
+      setUpRequestHandler(webview, "media");
       document.body.appendChild(webview);
     });
 
@@ -656,13 +664,10 @@ describe('<webview> tag', function() {
         assert(e.args, ['ERROR(1): User denied Geolocation']);
         done();
       });
-      webview.addEventListener('permission-request', function(e) {
-        if (e.permission === 'geolocation') {
-          e.deny();
-        }
-      });
       webview.src = "file://" + fixtures + "/pages/permissions/geolocation.html";
+      webview.partition = "permissionTest";
       webview.setAttribute('nodeintegration', 'on');
+      setUpRequestHandler(webview, "geolocation");
       document.body.appendChild(webview);
     });
 
@@ -672,13 +677,10 @@ describe('<webview> tag', function() {
         assert(e.args, ['SecurityError']);
         done();
       });
-      webview.addEventListener('permission-request', function(e) {
-        if (e.permission === 'midiSysex') {
-          e.deny();
-        }
-      });
       webview.src = "file://" + fixtures + "/pages/permissions/midi.html";
+      webview.partition = "permissionTest";
       webview.setAttribute('nodeintegration', 'on');
+      setUpRequestHandler(webview, "midiSysex");
       document.body.appendChild(webview);
     });
   });