libservo: Flesh out permissions API (#35396)

- Update the script crate to better reflect the modern Permission
  specifcation -- removing the necessity for an `Insecure` variant of
  the permissions prompt.
- Have all allow/deny type requests in the internal API use an
  `AllowOrDeny` enum for clarity.
- Expose `PermissionsRequest` and `PermissionFeature` data types to the
  API and use them in the delegate method.
- Update both servoshell implementations to use the API.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>

components/devtools/lib.rs

@@ -26,7 +26,7 @@ use devtools_traits::{
     DevtoolsControlMsg, DevtoolsPageInfo, LogLevel, NavigationState, NetworkEvent, PageError,
     ScriptToDevtoolsControlMsg, WorkerId,
 };
-use embedder_traits::{EmbedderMsg, EmbedderProxy};
+use embedder_traits::{AllowOrDeny, EmbedderMsg, EmbedderProxy};
 use ipc_channel::ipc::{self, IpcSender};
 use log::{debug, trace, warn};
 use serde::Serialize;
@@ -764,5 +764,5 @@ fn allow_devtools_client(stream: &mut TcpStream, embedder: &EmbedderProxy, token
     // No token found. Prompt user
     let (request_sender, request_receiver) = ipc::channel().expect("Failed to create IPC channel!");
     embedder.send(EmbedderMsg::RequestDevtoolsConnection(request_sender));
-    request_receiver.recv().unwrap()
+    request_receiver.recv().unwrap() == AllowOrDeny::Allow
 }

components/script/dom/bluetooth.rs

@@ -29,7 +29,7 @@ use crate::dom::bluetoothpermissionresult::BluetoothPermissionResult;
 use crate::dom::bluetoothuuid::{BluetoothServiceUUID, BluetoothUUID, UUID};
 use crate::dom::eventtarget::EventTarget;
 use crate::dom::globalscope::GlobalScope;
-use crate::dom::permissions::{get_descriptor_permission_state, PermissionAlgorithm};
+use crate::dom::permissions::{descriptor_permission_state, PermissionAlgorithm};
 use crate::dom::promise::Promise;
 use crate::script_runtime::{CanGc, JSContext};
 use crate::task::TaskOnce;
@@ -227,7 +227,7 @@ impl Bluetooth {
 
         // Step 4 - 5.
         if let PermissionState::Denied =
-            get_descriptor_permission_state(PermissionName::Bluetooth, None)
+            descriptor_permission_state(PermissionName::Bluetooth, None)
         {
             return p.reject_error(Error::NotFound);
         }
@@ -649,7 +649,7 @@ impl PermissionAlgorithm for Bluetooth {
         // Step 1: We are not using the `global` variable.
 
         // Step 2.
-        status.set_state(get_descriptor_permission_state(status.get_query(), None));
+        status.set_state(descriptor_permission_state(status.get_query(), None));
 
         // Step 3.
         if let PermissionState::Denied = status.get_state() {

components/script/dom/document.rs

@@ -24,8 +24,8 @@ use cssparser::match_ignore_ascii_case;
 use devtools_traits::ScriptToDevtoolsControlMsg;
 use dom_struct::dom_struct;
 use embedder_traits::{
-    ClipboardEventType, EmbedderMsg, LoadStatus, MouseButton, MouseEventType, TouchEventType,
+    AllowOrDeny, ClipboardEventType, EmbedderMsg, LoadStatus, MouseButton, MouseEventType,
-    TouchId, WheelDelta,
+    TouchEventType, TouchId, WheelDelta,
 };
 use encoding_rs::{Encoding, UTF_8};
 use euclid::default::{Point2D, Rect, Size2D};
@@ -94,6 +94,7 @@ use crate::dom::bindings::codegen::Bindings::NavigatorBinding::Navigator_Binding
 use crate::dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
 use crate::dom::bindings::codegen::Bindings::NodeFilterBinding::NodeFilter;
 use crate::dom::bindings::codegen::Bindings::PerformanceBinding::PerformanceMethods;
+use crate::dom::bindings::codegen::Bindings::PermissionStatusBinding::PermissionName;
 use crate::dom::bindings::codegen::Bindings::ShadowRootBinding::ShadowRootMethods;
 use crate::dom::bindings::codegen::Bindings::TouchBinding::TouchMethods;
 use crate::dom::bindings::codegen::Bindings::WindowBinding::{
@@ -2492,7 +2493,7 @@ impl Document {
             let (chan, port) = ipc::channel().expect("Failed to create IPC channel!");
             let msg = EmbedderMsg::AllowUnload(self.webview_id(), chan);
             self.send_to_embedder(msg);
-            can_unload = port.recv().unwrap();
+            can_unload = port.recv().unwrap() == AllowOrDeny::Allow;
         }
         // Step 9
         if !recursive_flag {
@@ -3309,6 +3310,26 @@ impl Document {
             .parse(url)
             .map(ServoUrl::from)
     }
+
+    /// <https://html.spec.whatwg.org/multipage/#allowed-to-use>
+    pub(crate) fn allowed_to_use_feature(&self, _feature: PermissionName) -> bool {
+        // Step 1. If document's browsing context is null, then return false.
+        if !self.has_browsing_context {
+            return false;
+        }
+
+        // Step 2. If document is not fully active, then return false.
+        if !self.is_fully_active() {
+            return false;
+        }
+
+        // Step 3. If the result of running is feature enabled in document for origin on
+        // feature, document, and document's origin is "Enabled", then return true.
+        // Step 4. Return false.
+        // TODO: All features are currently enabled for `Document`s because we do not
+        // implement the Permissions Policy specification.
+        true
+    }
 }
 
 fn is_character_value_key(key: &Key) -> bool {

components/script/dom/globalscope.rs

@@ -81,7 +81,9 @@ use crate::dom::bindings::codegen::Bindings::ImageBitmapBinding::{
 };
 use crate::dom::bindings::codegen::Bindings::NavigatorBinding::NavigatorMethods;
 use crate::dom::bindings::codegen::Bindings::PerformanceBinding::Performance_Binding::PerformanceMethods;
-use crate::dom::bindings::codegen::Bindings::PermissionStatusBinding::PermissionState;
+use crate::dom::bindings::codegen::Bindings::PermissionStatusBinding::{
+    PermissionName, PermissionState,
+};
 use crate::dom::bindings::codegen::Bindings::VoidFunctionBinding::VoidFunction;
 use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
 use crate::dom::bindings::codegen::Bindings::WorkerGlobalScopeBinding::WorkerGlobalScopeMethods;
@@ -273,7 +275,7 @@ pub(crate) struct GlobalScope {
     creation_url: Option<ServoUrl>,
 
     /// A map for storing the previous permission state read results.
-    permission_state_invocation_results: DomRefCell<HashMap<String, PermissionState>>,
+    permission_state_invocation_results: DomRefCell<HashMap<PermissionName, PermissionState>>,
 
     /// The microtask queue associated with this global.
     ///
@@ -1965,7 +1967,7 @@ impl GlobalScope {
 
     pub(crate) fn permission_state_invocation_results(
         &self,
-    ) -> &DomRefCell<HashMap<String, PermissionState>> {
+    ) -> &DomRefCell<HashMap<PermissionName, PermissionState>> {
         &self.permission_state_invocation_results
     }
 
@@ -2918,7 +2920,12 @@ impl GlobalScope {
         self.https_state.set(https_state);
     }
 
+    /// <https://html.spec.whatwg.org/multipage/#secure-context>
     pub(crate) fn is_secure_context(&self) -> bool {
+        // This differs from the specification, but it seems that
+        // `inherited_secure_context` implements more-or-less the exact same logic, in a
+        // different manner. Workers inherit whether or not their in a secure context and
+        // worklets do as well (they can only be created in secure contexts).
         if Some(false) == self.inherited_secure_context {
             return false;
         }

components/script/dom/permissions.rs

@@ -5,18 +5,21 @@
 use std::rc::Rc;
 
 use dom_struct::dom_struct;
-use embedder_traits::{self, EmbedderMsg, PermissionPrompt, PermissionRequest};
+use embedder_traits::{self, AllowOrDeny, EmbedderMsg, PermissionFeature};
 use ipc_channel::ipc;
 use js::conversions::ConversionResult;
 use js::jsapi::JSObject;
 use js::jsval::{ObjectValue, UndefinedValue};
+use script_bindings::inheritance::Castable;
 use servo_config::pref;
 
+use super::window::Window;
 use crate::conversions::Convert;
 use crate::dom::bindings::codegen::Bindings::PermissionStatusBinding::{
     PermissionDescriptor, PermissionName, PermissionState, PermissionStatusMethods,
 };
 use crate::dom::bindings::codegen::Bindings::PermissionsBinding::PermissionsMethods;
+use crate::dom::bindings::codegen::Bindings::WindowBinding::Window_Binding::WindowMethods;
 use crate::dom::bindings::error::Error;
 use crate::dom::bindings::reflector::{reflect_dom_object, DomGlobal, Reflector};
 use crate::dom::bindings::root::DomRoot;
@@ -142,7 +145,7 @@ impl Permissions {
                         globalscope
                             .permission_state_invocation_results()
                             .borrow_mut()
-                            .remove(&root_desc.name.to_string());
+                            .remove(&root_desc.name);
 
                         // (Revoke) Step 4.
                         Bluetooth::permission_revoke(&bluetooth_desc, &result, can_gc)
@@ -174,7 +177,7 @@ impl Permissions {
                         globalscope
                             .permission_state_invocation_results()
                             .borrow_mut()
-                            .remove(&root_desc.name.to_string());
+                            .remove(&root_desc.name);
 
                         // (Revoke) Step 4.
                         Permissions::permission_revoke(&root_desc, &status, can_gc);
@@ -231,15 +234,25 @@ impl PermissionAlgorithm for Permissions {
         }
     }
 
-    // https://w3c.github.io/permissions/#boolean-permission-query-algorithm
+    /// <https://w3c.github.io/permissions/#dfn-permission-query-algorithm>
+    ///
+    /// > permission query algorithm:
+    /// > Takes an instance of the permission descriptor type and a new or existing instance of
+    /// > the permission result type, and updates the permission result type instance with the
+    /// > query result. Used by Permissions' query(permissionDesc) method and the
+    /// > PermissionStatus update steps. If unspecified, this defaults to the default permission
+    /// > query algorithm.
+    ///
+    /// > The default permission query algorithm, given a PermissionDescriptor
+    /// > permissionDesc and a PermissionStatus status, runs the following steps:
     fn permission_query(
         _cx: JSContext,
         _promise: &Rc<Promise>,
         _descriptor: &PermissionDescriptor,
         status: &PermissionStatus,
     ) {
-        // Step 1.
+        // Step 1. Set status's state to permissionDesc's permission state.
-        status.set_state(get_descriptor_permission_state(status.get_query(), None));
+        status.set_state(descriptor_permission_state(status.get_query(), None));
     }
 
     // https://w3c.github.io/permissions/#boolean-permission-request-algorithm
@@ -255,16 +268,14 @@ impl PermissionAlgorithm for Permissions {
         match status.State() {
             // Step 3.
             PermissionState::Prompt => {
-                let perm_name = status.get_query();
-                let prompt = PermissionPrompt::Request(perm_name.convert());
-
                 // https://w3c.github.io/permissions/#request-permission-to-use (Step 3 - 4)
+                let permission_name = status.get_query();
                 let globalscope = GlobalScope::current().expect("No current global object");
-                let state = prompt_user_from_embedder(prompt, &globalscope);
+                let state = prompt_user_from_embedder(permission_name, &globalscope);
                 globalscope
                     .permission_state_invocation_results()
                     .borrow_mut()
-                    .insert(perm_name.to_string(), state);
+                    .insert(permission_name, state);
             },
 
             // Step 2.
@@ -283,95 +294,73 @@ impl PermissionAlgorithm for Permissions {
     }
 }
 
-// https://w3c.github.io/permissions/#permission-state
+/// <https://w3c.github.io/permissions/#dfn-permission-state>
-pub(crate) fn get_descriptor_permission_state(
+pub(crate) fn descriptor_permission_state(
-    permission_name: PermissionName,
+    feature: PermissionName,
     env_settings_obj: Option<&GlobalScope>,
 ) -> PermissionState {
-    // Step 1.
+    // Step 1. If settings wasn't passed, set it to the current settings object.
-    let globalscope = match env_settings_obj {
+    let global_scope = match env_settings_obj {
         Some(env_settings_obj) => DomRoot::from_ref(env_settings_obj),
         None => GlobalScope::current().expect("No current global object"),
     };
 
-    // Step 2.
+    // Step 2. If settings is a non-secure context, return "denied".
-    // TODO: The `is the environment settings object a non-secure context` check is missing.
+    if !global_scope.is_secure_context() {
-    // The current solution is a workaround with a message box to warn about this,
+        if pref!(dom_permissions_testing_allowed_in_nonsecure_contexts) {
-    // if the feature is not allowed in non-secure contexcts,
+            return PermissionState::Granted;
-    // and let the user decide to grant the permission or not.
+        }
-    let state = if allowed_in_nonsecure_contexts(&permission_name) {
+        return PermissionState::Denied;
-        PermissionState::Prompt
+    }
-    } else if pref!(dom_permissions_testing_allowed_in_nonsecure_contexts) {
-        PermissionState::Granted
-    } else {
-        globalscope
-            .permission_state_invocation_results()
-            .borrow_mut()
-            .remove(&permission_name.to_string());
-        prompt_user_from_embedder(
-            PermissionPrompt::Insecure(permission_name.convert()),
-            &globalscope,
-        )
-    };
 
-    // Step 3.
+    // Step 3. Let feature be descriptor's name.
-    if let Some(prev_result) = globalscope
+    // The caller has already converted the descriptor into a name.
+
+    // Step 4. If there exists a policy-controlled feature for feature and settings'
+    // relevant global object has an associated Document run the following step:
+    //   1. Let document be settings' relevant global object's associated Document.
+    //   2. If document is not allowed to use feature, return "denied".
+    if let Some(window) = global_scope.downcast::<Window>() {
+        if !window.Document().allowed_to_use_feature(feature) {
+            return PermissionState::Denied;
+        }
+    }
+
+    // Step 5. Let key be the result of generating a permission key for descriptor with settings.
+    // Step 6. Let entry be the result of getting a permission store entry with descriptor and key.
+    // Step 7. If entry is not null, return a PermissionState enum value from entry's state.
+    //
+    // TODO: We aren't making a key based on the descriptor, but on the descriptor's name. This really
+    // only matters for WebBluetooth, which adds more fields to the descriptor beyond the name.
+    if let Some(entry) = global_scope
         .permission_state_invocation_results()
         .borrow()
-        .get(&permission_name.to_string())
+        .get(&feature)
     {
-        return *prev_result;
+        return *entry;
     }
 
-    // Store the invocation result
+    // Step 8. Return the PermissionState enum value that represents the permission state
-    globalscope
+    // of feature, taking into account any permission state constraints for descriptor's
-        .permission_state_invocation_results()
+    // name.
-        .borrow_mut()
+    PermissionState::Prompt
-        .insert(permission_name.to_string(), state);
-
-    // Step 4.
-    state
 }
 
-// https://w3c.github.io/permissions/#allowed-in-non-secure-contexts
+fn prompt_user_from_embedder(name: PermissionName, global_scope: &GlobalScope) -> PermissionState {
-fn allowed_in_nonsecure_contexts(permission_name: &PermissionName) -> bool {
+    let Some(webview_id) = global_scope.webview_id() else {
-    match *permission_name {
-        // https://w3c.github.io/permissions/#dom-permissionname-geolocation
-        PermissionName::Geolocation => true,
-        // https://w3c.github.io/permissions/#dom-permissionname-notifications
-        PermissionName::Notifications => true,
-        // https://w3c.github.io/permissions/#dom-permissionname-push
-        PermissionName::Push => false,
-        // https://w3c.github.io/permissions/#dom-permissionname-midi
-        PermissionName::Midi => true,
-        // https://w3c.github.io/permissions/#dom-permissionname-camera
-        PermissionName::Camera => false,
-        // https://w3c.github.io/permissions/#dom-permissionname-microphone
-        PermissionName::Microphone => false,
-        // https://w3c.github.io/permissions/#dom-permissionname-speaker
-        PermissionName::Speaker => false,
-        // https://w3c.github.io/permissions/#dom-permissionname-device-info
-        PermissionName::Device_info => false,
-        // https://w3c.github.io/permissions/#dom-permissionname-background-sync
-        PermissionName::Background_sync => false,
-        // https://webbluetoothcg.github.io/web-bluetooth/#dom-permissionname-bluetooth
-        PermissionName::Bluetooth => false,
-        // https://storage.spec.whatwg.org/#dom-permissionname-persistent-storage
-        PermissionName::Persistent_storage => false,
-    }
-}
-
-fn prompt_user_from_embedder(prompt: PermissionPrompt, gs: &GlobalScope) -> PermissionState {
-    let Some(webview_id) = gs.webview_id() else {
         warn!("Requesting permissions from non-webview-associated global scope");
         return PermissionState::Denied;
     };
     let (sender, receiver) = ipc::channel().expect("Failed to create IPC channel!");
-    gs.send_to_embedder(EmbedderMsg::PromptPermission(webview_id, prompt, sender));
+    global_scope.send_to_embedder(EmbedderMsg::PromptPermission(
+        webview_id,
+        name.convert(),
+        sender,
+    ));
 
     match receiver.recv() {
-        Ok(PermissionRequest::Granted) => PermissionState::Granted,
+        Ok(AllowOrDeny::Allow) => PermissionState::Granted,
-        Ok(PermissionRequest::Denied) => PermissionState::Denied,
+        Ok(AllowOrDeny::Deny) => PermissionState::Denied,
         Err(e) => {
             warn!(
                 "Failed to receive permission state from embedder ({:?}).",
@@ -382,22 +371,20 @@ fn prompt_user_from_embedder(prompt: PermissionPrompt, gs: &GlobalScope) -> Perm
     }
 }
 
-impl Convert<embedder_traits::PermissionName> for PermissionName {
+impl Convert<PermissionFeature> for PermissionName {
-    fn convert(self) -> embedder_traits::PermissionName {
+    fn convert(self) -> PermissionFeature {
         match self {
-            PermissionName::Geolocation => embedder_traits::PermissionName::Geolocation,
+            PermissionName::Geolocation => PermissionFeature::Geolocation,
-            PermissionName::Notifications => embedder_traits::PermissionName::Notifications,
+            PermissionName::Notifications => PermissionFeature::Notifications,
-            PermissionName::Push => embedder_traits::PermissionName::Push,
+            PermissionName::Push => PermissionFeature::Push,
-            PermissionName::Midi => embedder_traits::PermissionName::Midi,
+            PermissionName::Midi => PermissionFeature::Midi,
-            PermissionName::Camera => embedder_traits::PermissionName::Camera,
+            PermissionName::Camera => PermissionFeature::Camera,
-            PermissionName::Microphone => embedder_traits::PermissionName::Microphone,
+            PermissionName::Microphone => PermissionFeature::Microphone,
-            PermissionName::Speaker => embedder_traits::PermissionName::Speaker,
+            PermissionName::Speaker => PermissionFeature::Speaker,
-            PermissionName::Device_info => embedder_traits::PermissionName::DeviceInfo,
+            PermissionName::Device_info => PermissionFeature::DeviceInfo,
-            PermissionName::Background_sync => embedder_traits::PermissionName::BackgroundSync,
+            PermissionName::Background_sync => PermissionFeature::BackgroundSync,
-            PermissionName::Bluetooth => embedder_traits::PermissionName::Bluetooth,
+            PermissionName::Bluetooth => PermissionFeature::Bluetooth,
-            PermissionName::Persistent_storage => {
+            PermissionName::Persistent_storage => PermissionFeature::PersistentStorage,
-                embedder_traits::PermissionName::PersistentStorage
-            },
         }
     }
 }

components/script_bindings/codegen/Bindings.conf

@@ -596,7 +596,10 @@ Dictionaries = {
 
 Enums = {
 'GPUFeatureName': {
-    'derives': ['Hash', 'Eq']
+    'derives': ['Eq', 'Hash', ]
+},
+'PermissionName': {
+    'derives': ['Eq', 'Hash']
 }
 }
 

components/servo/lib.rs

@@ -118,7 +118,9 @@ pub use {
 use crate::proxies::ConstellationProxy;
 pub use crate::servo_delegate::{ServoDelegate, ServoError};
 pub use crate::webview::WebView;
-pub use crate::webview_delegate::{AllowOrDenyRequest, NavigationRequest, WebViewDelegate};
+pub use crate::webview_delegate::{
+    AllowOrDenyRequest, NavigationRequest, PermissionRequest, WebViewDelegate,
+};
 
 #[cfg(feature = "webdriver")]
 fn webdriver(port: u16, constellation: Sender<ConstellationMsg>) {
@@ -801,7 +803,7 @@ impl Servo {
                     let request = AllowOrDenyRequest {
                         response_sender,
                         response_sent: false,
-                        default_response: true,
+                        default_response: AllowOrDeny::Allow,
                     };
                     webview.delegate().request_unload(webview, request);
                 }
@@ -914,13 +916,19 @@ impl Servo {
                     );
                 }
             },
-            EmbedderMsg::PromptPermission(webview_id, permission_prompt, result_sender) => {
+            EmbedderMsg::PromptPermission(webview_id, requested_feature, response_sender) => {
                 if let Some(webview) = self.get_webview_handle(webview_id) {
-                    webview.delegate().request_permission(
+                    let permission_request = PermissionRequest {
-                        webview,
+                        requested_feature,
-                        permission_prompt,
+                        allow_deny_request: AllowOrDenyRequest {
-                        result_sender,
+                            response_sender,
-                    );
+                            response_sent: false,
+                            default_response: AllowOrDeny::Deny,
+                        },
+                    };
+                    webview
+                        .delegate()
+                        .request_permission(webview, permission_request);
                 }
             },
             EmbedderMsg::ShowIME(webview_id, input_method_type, text, multiline, position) => {
@@ -961,7 +969,7 @@ impl Servo {
                     AllowOrDenyRequest {
                         response_sender,
                         response_sent: false,
-                        default_response: false,
+                        default_response: AllowOrDeny::Deny,
                     },
                 );
             },

components/servo/webview_delegate.rs

@@ -7,8 +7,8 @@ use std::path::PathBuf;
 use base::id::PipelineId;
 use compositing_traits::ConstellationMsg;
 use embedder_traits::{
-    CompositorEventVariant, ContextMenuResult, Cursor, FilterPattern, GamepadHapticEffectType,
+    AllowOrDeny, CompositorEventVariant, ContextMenuResult, Cursor, FilterPattern,
-    InputMethodType, LoadStatus, MediaSessionEvent, PermissionPrompt, PermissionRequest,
+    GamepadHapticEffectType, InputMethodType, LoadStatus, MediaSessionEvent, PermissionFeature,
     PromptDefinition, PromptOrigin, WebResourceRequest, WebResourceResponseMsg,
 };
 use ipc_channel::ipc::IpcSender;
@@ -59,20 +59,42 @@ impl Drop for NavigationRequest {
     }
 }
 
+/// A permissions request for a [`WebView`] The embedder should allow or deny the request,
+/// either by reading a cached value or querying the user for permission via the user
+/// interface.
+pub struct PermissionRequest {
+    pub(crate) requested_feature: PermissionFeature,
+    pub(crate) allow_deny_request: AllowOrDenyRequest,
+}
+
+impl PermissionRequest {
+    pub fn feature(&self) -> PermissionFeature {
+        self.requested_feature
+    }
+
+    pub fn allow(self) {
+        self.allow_deny_request.allow();
+    }
+
+    pub fn deny(self) {
+        self.allow_deny_request.deny();
+    }
+}
+
 pub struct AllowOrDenyRequest {
-    pub(crate) response_sender: IpcSender<bool>,
+    pub(crate) response_sender: IpcSender<AllowOrDeny>,
     pub(crate) response_sent: bool,
-    pub(crate) default_response: bool,
+    pub(crate) default_response: AllowOrDeny,
 }
 
 impl AllowOrDenyRequest {
     pub fn allow(mut self) {
-        let _ = self.response_sender.send(true);
+        let _ = self.response_sender.send(AllowOrDeny::Allow);
         self.response_sent = true;
     }
 
     pub fn deny(mut self) {
-        let _ = self.response_sender.send(false);
+        let _ = self.response_sender.send(AllowOrDeny::Deny);
         self.response_sent = true;
     }
 }
@@ -148,15 +170,11 @@ pub trait WebViewDelegate {
     fn request_open_auxiliary_webview(&self, _parent_webview: WebView) -> Option<WebView> {
         None
     }
-    /// Open interface to request permission specified by prompt.
+
-    fn request_permission(
+    /// Content in a [`WebView`] is requesting permission to access a feature requiring
-        &self,
+    /// permission from the user. The embedder should allow or deny the request, either by
-        _webview: WebView,
+    /// reading a cached value or querying the user for permission via the user interface.
-        _: PermissionPrompt,
+    fn request_permission(&self, _webview: WebView, _: PermissionRequest) {}
-        result_sender: IpcSender<PermissionRequest>,
-    ) {
-        let _ = result_sender.send(PermissionRequest::Denied);
-    }
 
     /// Show dialog to user
     /// TODO: This API needs to be reworked to match the new model of how responses are sent.

components/shared/embedder/lib.rs

@@ -147,6 +147,13 @@ pub enum PromptResult {
     Dismissed,
 }
 
+/// A response to a request to allow or deny an action.
+#[derive(Clone, Copy, Deserialize, PartialEq, Serialize)]
+pub enum AllowOrDeny {
+    Allow,
+    Deny,
+}
+
 #[derive(Deserialize, Serialize)]
 pub enum EmbedderMsg {
     /// A status message to be displayed by the browser chrome.
@@ -179,7 +186,7 @@ pub enum EmbedderMsg {
     /// All webviews lost focus for keyboard events.
     WebViewBlurred,
     /// Wether or not to unload a document
-    AllowUnload(WebViewId, IpcSender<bool>),
+    AllowUnload(WebViewId, IpcSender<AllowOrDeny>),
     /// Sends an unconsumed key event back to the embedder.
     Keyboard(WebViewId, KeyboardEvent),
     /// Inform embedder to clear the clipboard
@@ -215,7 +222,7 @@ pub enum EmbedderMsg {
         IpcSender<Option<Vec<PathBuf>>>,
     ),
     /// Open interface to request permission specified by prompt.
-    PromptPermission(WebViewId, PermissionPrompt, IpcSender<PermissionRequest>),
+    PromptPermission(WebViewId, PermissionFeature, IpcSender<AllowOrDeny>),
     /// Request to present an IME to the user when an editable element is focused.
     /// If the input is text, the second parameter defines the pre-existing string
     /// text content and the zero-based index into the string locating the insertion point.
@@ -237,7 +244,7 @@ pub enum EmbedderMsg {
     /// Report the status of Devtools Server with a token that can be used to bypass the permission prompt.
     OnDevtoolsStarted(Result<u16, ()>, String),
     /// Ask the user to allow a devtools client to connect.
-    RequestDevtoolsConnection(IpcSender<bool>),
+    RequestDevtoolsConnection(IpcSender<AllowOrDeny>),
     /// Notify the embedder that it needs to present a new frame.
     ReadyToPresent(Vec<WebViewId>),
     /// The given event was delivered to a pipeline in the given browser.
@@ -377,8 +384,8 @@ pub enum MediaSessionEvent {
 }
 
 /// Enum with variants that match the DOM PermissionName enum
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Copy, Debug, Deserialize, Serialize)]
-pub enum PermissionName {
+pub enum PermissionFeature {
     Geolocation,
     Notifications,
     Push,
@@ -392,20 +399,6 @@ pub enum PermissionName {
     PersistentStorage,
 }
 
-/// Information required to display a permission prompt
-#[derive(Clone, Debug, Deserialize, Serialize)]
-pub enum PermissionPrompt {
-    Insecure(PermissionName),
-    Request(PermissionName),
-}
-
-/// Status for prompting user for permission.
-#[derive(Clone, Debug, Deserialize, Serialize)]
-pub enum PermissionRequest {
-    Granted,
-    Denied,
-}
-
 /// Used to specify the kind of input method editor appropriate to edit a field.
 /// This is a subset of htmlinputelement::InputType because some variants of InputType
 /// don't make sense in this context.

ports/servoshell/desktop/app_state.rs

@@ -18,8 +18,8 @@ use servo::webrender_api::units::{DeviceIntPoint, DeviceIntSize};
 use servo::webrender_api::ScrollLocation;
 use servo::{
     AllowOrDenyRequest, CompositorEventVariant, FilterPattern, GamepadHapticEffectType, LoadStatus,
-    PermissionPrompt, PermissionRequest, PromptCredentialsInput, PromptDefinition, PromptOrigin,
+    PermissionRequest, PromptCredentialsInput, PromptDefinition, PromptOrigin, PromptResult, Servo,
-    PromptResult, Servo, ServoDelegate, ServoError, TouchEventType, WebView, WebViewDelegate,
+    ServoDelegate, ServoError, TouchEventType, WebView, WebViewDelegate,
 };
 use tinyfiledialogs::{self, MessageBoxIcon, OkCancel};
 use url::Url;
@@ -509,16 +509,10 @@ impl WebViewDelegate for RunningAppState {
         inner_mut.need_present = true;
     }
 
-    fn request_permission(
+    fn request_permission(&self, _webview: servo::WebView, request: PermissionRequest) {
-        &self,
+        if !self.inner().headless {
-        _webview: servo::WebView,
+            prompt_user(request);
-        prompt: PermissionPrompt,
+        }
-        result_sender: IpcSender<PermissionRequest>,
-    ) {
-        let _ = result_sender.send(match self.inner().headless {
-            true => PermissionRequest::Denied,
-            false => prompt_user(prompt),
-        });
     }
 
     fn notify_new_frame_ready(&self, _webview: servo::WebView) {
@@ -564,37 +558,27 @@ impl WebViewDelegate for RunningAppState {
 }
 
 #[cfg(target_os = "linux")]
-fn prompt_user(prompt: PermissionPrompt) -> PermissionRequest {
+fn prompt_user(request: PermissionRequest) {
     use tinyfiledialogs::YesNo;
 
-    let message = match prompt {
+    let message = format!(
-        PermissionPrompt::Request(permission_name) => {
+        "Do you want to grant permission for {:?}?",
-            format!("Do you want to grant permission for {:?}?", permission_name)
+        request.feature()
-        },
+    );
-        PermissionPrompt::Insecure(permission_name) => {
-            format!(
-                "The {:?} feature is only safe to use in secure context, but servo can't guarantee\n\
-                that the current context is secure. Do you want to proceed and grant permission?",
-                permission_name
-            )
-        },
-    };
-
     match tinyfiledialogs::message_box_yes_no(
         "Permission request dialog",
         &message,
         MessageBoxIcon::Question,
         YesNo::No,
     ) {
-        YesNo::Yes => PermissionRequest::Granted,
+        YesNo::Yes => request.allow(),
-        YesNo::No => PermissionRequest::Denied,
+        YesNo::No => request.deny(),
     }
 }
 
 #[cfg(not(target_os = "linux"))]
-fn prompt_user(_prompt: PermissionPrompt) -> PermissionRequest {
+fn prompt_user(_request: PermissionRequest) {
-    // TODO popup only supported on linux
+    // Requests are denied by default.
-    PermissionRequest::Denied
 }
 
 #[cfg(target_os = "linux")]

ports/servoshell/egl/app_state.rs

@@ -21,9 +21,8 @@ use servo::webrender_traits::SurfmanRenderingContext;
 use servo::{
     AllowOrDenyRequest, ContextMenuResult, EmbedderProxy, EventLoopWaker, InputMethodType, Key,
     KeyState, KeyboardEvent, LoadStatus, MediaSessionActionType, MediaSessionEvent, MouseButton,
-    NavigationRequest, PermissionPrompt, PermissionRequest, PromptDefinition, PromptOrigin,
+    NavigationRequest, PermissionRequest, PromptDefinition, PromptOrigin, PromptResult, Servo,
-    PromptResult, Servo, ServoDelegate, ServoError, TouchEventType, TouchId, WebView,
+    ServoDelegate, ServoError, TouchEventType, TouchId, WebView, WebViewDelegate,
-    WebViewDelegate,
 };
 use url::Url;
 
@@ -209,31 +208,15 @@ impl WebViewDelegate for RunningAppState {
         Some(new_webview)
     }
 
-    fn request_permission(
+    fn request_permission(&self, _webview: WebView, request: PermissionRequest) {
-        &self,
+        let message = format!(
-        _webview: WebView,
+            "Do you want to grant permission for {:?}?",
-        prompt: PermissionPrompt,
+            request.feature()
-        result_sender: IpcSender<PermissionRequest>,
+        );
-    ) {
-        let message = match prompt {
-            PermissionPrompt::Request(permission_name) => {
-                format!("Do you want to grant permission for {:?}?", permission_name)
-            },
-            PermissionPrompt::Insecure(permission_name) => {
-                format!(
-                    "The {:?} feature is only safe to use in secure context, but servo can't guarantee\n\
-                        that the current context is secure. Do you want to proceed and grant permission?",
-                    permission_name
-                )
-            },
-        };
-
         let result = match self.callbacks.host_callbacks.prompt_yes_no(message, true) {
-            PromptResult::Primary => PermissionRequest::Granted,
+            PromptResult::Primary => request.allow(),
-            PromptResult::Secondary | PromptResult::Dismissed => PermissionRequest::Denied,
+            PromptResult::Secondary | PromptResult::Dismissed => request.deny(),
         };
-
-        let _ = result_sender.send(result);
     }
 
     fn request_resize_to(&self, _webview: WebView, size: DeviceIntSize) {