From f51a5661f823d8441bc851b66a36d576146e1916 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Mon, 10 Feb 2025 16:50:33 +0100 Subject: [PATCH] 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 Co-authored-by: Mukilan Thiyagarajan --- components/devtools/lib.rs | 4 +- components/script/dom/bluetooth.rs | 6 +- components/script/dom/document.rs | 27 ++- components/script/dom/globalscope.rs | 13 +- components/script/dom/permissions.rs | 177 ++++++++---------- .../script_bindings/codegen/Bindings.conf | 5 +- components/servo/lib.rs | 26 ++- components/servo/webview_delegate.rs | 48 +++-- components/shared/embedder/lib.rs | 31 ++- ports/servoshell/desktop/app_state.rs | 46 ++--- ports/servoshell/egl/app_state.rs | 35 +--- 11 files changed, 211 insertions(+), 207 deletions(-) diff --git a/components/devtools/lib.rs b/components/devtools/lib.rs index 07cf2757777..4192b778c9a 100644 --- a/components/devtools/lib.rs +++ b/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 } diff --git a/components/script/dom/bluetooth.rs b/components/script/dom/bluetooth.rs index 43e323488a3..4e47c9664bf 100644 --- a/components/script/dom/bluetooth.rs +++ b/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() { diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 87efd354fee..90f7f61dfbd 100644 --- a/components/script/dom/document.rs +++ b/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, - TouchId, WheelDelta, + AllowOrDeny, ClipboardEventType, EmbedderMsg, LoadStatus, MouseButton, MouseEventType, + 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) } + + /// + 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 { diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index f9d47f04708..11d38cf29c7 100644 --- a/components/script/dom/globalscope.rs +++ b/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, /// A map for storing the previous permission state read results. - permission_state_invocation_results: DomRefCell>, + permission_state_invocation_results: DomRefCell>, /// The microtask queue associated with this global. /// @@ -1965,7 +1967,7 @@ impl GlobalScope { pub(crate) fn permission_state_invocation_results( &self, - ) -> &DomRefCell> { + ) -> &DomRefCell> { &self.permission_state_invocation_results } @@ -2918,7 +2920,12 @@ impl GlobalScope { self.https_state.set(https_state); } + /// 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; } diff --git a/components/script/dom/permissions.rs b/components/script/dom/permissions.rs index c7aefec2199..3e9573bacb4 100644 --- a/components/script/dom/permissions.rs +++ b/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 + /// + /// + /// > 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, _descriptor: &PermissionDescriptor, status: &PermissionStatus, ) { - // Step 1. - status.set_state(get_descriptor_permission_state(status.get_query(), None)); + // Step 1. Set status's state to permissionDesc's permission state. + 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 -pub(crate) fn get_descriptor_permission_state( - permission_name: PermissionName, +/// +pub(crate) fn descriptor_permission_state( + feature: PermissionName, env_settings_obj: Option<&GlobalScope>, ) -> PermissionState { - // Step 1. - let globalscope = match env_settings_obj { + // Step 1. If settings wasn't passed, set it to the current settings object. + 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. - // TODO: The `is the environment settings object a non-secure context` check is missing. - // The current solution is a workaround with a message box to warn about this, - // if the feature is not allowed in non-secure contexcts, - // and let the user decide to grant the permission or not. - let state = if allowed_in_nonsecure_contexts(&permission_name) { - 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 2. If settings is a non-secure context, return "denied". + if !global_scope.is_secure_context() { + if pref!(dom_permissions_testing_allowed_in_nonsecure_contexts) { + return PermissionState::Granted; + } + return PermissionState::Denied; + } - // Step 3. - if let Some(prev_result) = globalscope + // Step 3. Let feature be descriptor's name. + // 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::() { + 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 - globalscope - .permission_state_invocation_results() - .borrow_mut() - .insert(permission_name.to_string(), state); - - // Step 4. - state + // Step 8. Return the PermissionState enum value that represents the permission state + // of feature, taking into account any permission state constraints for descriptor's + // name. + PermissionState::Prompt } -// https://w3c.github.io/permissions/#allowed-in-non-secure-contexts -fn allowed_in_nonsecure_contexts(permission_name: &PermissionName) -> bool { - 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 { +fn prompt_user_from_embedder(name: PermissionName, global_scope: &GlobalScope) -> PermissionState { + let Some(webview_id) = global_scope.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(PermissionRequest::Denied) => PermissionState::Denied, + Ok(AllowOrDeny::Allow) => PermissionState::Granted, + 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 for PermissionName { - fn convert(self) -> embedder_traits::PermissionName { +impl Convert for PermissionName { + fn convert(self) -> PermissionFeature { match self { - PermissionName::Geolocation => embedder_traits::PermissionName::Geolocation, - PermissionName::Notifications => embedder_traits::PermissionName::Notifications, - PermissionName::Push => embedder_traits::PermissionName::Push, - PermissionName::Midi => embedder_traits::PermissionName::Midi, - PermissionName::Camera => embedder_traits::PermissionName::Camera, - PermissionName::Microphone => embedder_traits::PermissionName::Microphone, - PermissionName::Speaker => embedder_traits::PermissionName::Speaker, - PermissionName::Device_info => embedder_traits::PermissionName::DeviceInfo, - PermissionName::Background_sync => embedder_traits::PermissionName::BackgroundSync, - PermissionName::Bluetooth => embedder_traits::PermissionName::Bluetooth, - PermissionName::Persistent_storage => { - embedder_traits::PermissionName::PersistentStorage - }, + PermissionName::Geolocation => PermissionFeature::Geolocation, + PermissionName::Notifications => PermissionFeature::Notifications, + PermissionName::Push => PermissionFeature::Push, + PermissionName::Midi => PermissionFeature::Midi, + PermissionName::Camera => PermissionFeature::Camera, + PermissionName::Microphone => PermissionFeature::Microphone, + PermissionName::Speaker => PermissionFeature::Speaker, + PermissionName::Device_info => PermissionFeature::DeviceInfo, + PermissionName::Background_sync => PermissionFeature::BackgroundSync, + PermissionName::Bluetooth => PermissionFeature::Bluetooth, + PermissionName::Persistent_storage => PermissionFeature::PersistentStorage, } } } diff --git a/components/script_bindings/codegen/Bindings.conf b/components/script_bindings/codegen/Bindings.conf index 450aa386fb8..492a20e1e1f 100644 --- a/components/script_bindings/codegen/Bindings.conf +++ b/components/script_bindings/codegen/Bindings.conf @@ -596,7 +596,10 @@ Dictionaries = { Enums = { 'GPUFeatureName': { - 'derives': ['Hash', 'Eq'] + 'derives': ['Eq', 'Hash', ] +}, +'PermissionName': { + 'derives': ['Eq', 'Hash'] } } diff --git a/components/servo/lib.rs b/components/servo/lib.rs index bb55854b519..6902c8d6b6a 100644 --- a/components/servo/lib.rs +++ b/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) { @@ -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( - webview, - permission_prompt, - result_sender, - ); + let permission_request = PermissionRequest { + requested_feature, + allow_deny_request: AllowOrDenyRequest { + 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, }, ); }, diff --git a/components/servo/webview_delegate.rs b/components/servo/webview_delegate.rs index 6a6608a5d6a..6eb71ed7230 100644 --- a/components/servo/webview_delegate.rs +++ b/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, - InputMethodType, LoadStatus, MediaSessionEvent, PermissionPrompt, PermissionRequest, + AllowOrDeny, CompositorEventVariant, ContextMenuResult, Cursor, FilterPattern, + 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, + pub(crate) response_sender: IpcSender, 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 { None } - /// Open interface to request permission specified by prompt. - fn request_permission( - &self, - _webview: WebView, - _: PermissionPrompt, - result_sender: IpcSender, - ) { - let _ = result_sender.send(PermissionRequest::Denied); - } + + /// Content in a [`WebView`] is requesting permission to access a feature requiring + /// permission from the user. The embedder should allow or deny the request, either by + /// reading a cached value or querying the user for permission via the user interface. + fn request_permission(&self, _webview: WebView, _: PermissionRequest) {} /// Show dialog to user /// TODO: This API needs to be reworked to match the new model of how responses are sent. diff --git a/components/shared/embedder/lib.rs b/components/shared/embedder/lib.rs index b1aae2f3ad4..8dc71ed02d8 100644 --- a/components/shared/embedder/lib.rs +++ b/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), + AllowUnload(WebViewId, IpcSender), /// 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>>, ), /// Open interface to request permission specified by prompt. - PromptPermission(WebViewId, PermissionPrompt, IpcSender), + PromptPermission(WebViewId, PermissionFeature, IpcSender), /// 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, String), /// Ask the user to allow a devtools client to connect. - RequestDevtoolsConnection(IpcSender), + RequestDevtoolsConnection(IpcSender), /// Notify the embedder that it needs to present a new frame. ReadyToPresent(Vec), /// 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)] -pub enum PermissionName { +#[derive(Clone, Copy, Debug, Deserialize, Serialize)] +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. diff --git a/ports/servoshell/desktop/app_state.rs b/ports/servoshell/desktop/app_state.rs index 501b92024cc..f0e6d60932c 100644 --- a/ports/servoshell/desktop/app_state.rs +++ b/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, - PromptResult, Servo, ServoDelegate, ServoError, TouchEventType, WebView, WebViewDelegate, + PermissionRequest, PromptCredentialsInput, PromptDefinition, PromptOrigin, PromptResult, Servo, + 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( - &self, - _webview: servo::WebView, - prompt: PermissionPrompt, - result_sender: IpcSender, - ) { - let _ = result_sender.send(match self.inner().headless { - true => PermissionRequest::Denied, - false => prompt_user(prompt), - }); + fn request_permission(&self, _webview: servo::WebView, request: PermissionRequest) { + if !self.inner().headless { + prompt_user(request); + } } 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 { - 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 message = format!( + "Do you want to grant permission for {:?}?", + request.feature() + ); match tinyfiledialogs::message_box_yes_no( "Permission request dialog", &message, MessageBoxIcon::Question, YesNo::No, ) { - YesNo::Yes => PermissionRequest::Granted, - YesNo::No => PermissionRequest::Denied, + YesNo::Yes => request.allow(), + YesNo::No => request.deny(), } } #[cfg(not(target_os = "linux"))] -fn prompt_user(_prompt: PermissionPrompt) -> PermissionRequest { - // TODO popup only supported on linux - PermissionRequest::Denied +fn prompt_user(_request: PermissionRequest) { + // Requests are denied by default. } #[cfg(target_os = "linux")] diff --git a/ports/servoshell/egl/app_state.rs b/ports/servoshell/egl/app_state.rs index f4299a612a4..5dd9520b899 100644 --- a/ports/servoshell/egl/app_state.rs +++ b/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, - PromptResult, Servo, ServoDelegate, ServoError, TouchEventType, TouchId, WebView, - WebViewDelegate, + NavigationRequest, PermissionRequest, PromptDefinition, PromptOrigin, PromptResult, Servo, + ServoDelegate, ServoError, TouchEventType, TouchId, WebView, WebViewDelegate, }; use url::Url; @@ -209,31 +208,15 @@ impl WebViewDelegate for RunningAppState { Some(new_webview) } - fn request_permission( - &self, - _webview: WebView, - prompt: PermissionPrompt, - result_sender: IpcSender, - ) { - 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 - ) - }, - }; - + fn request_permission(&self, _webview: WebView, request: PermissionRequest) { + let message = format!( + "Do you want to grant permission for {:?}?", + request.feature() + ); let result = match self.callbacks.host_callbacks.prompt_yes_no(message, true) { - PromptResult::Primary => PermissionRequest::Granted, - PromptResult::Secondary | PromptResult::Dismissed => PermissionRequest::Denied, + PromptResult::Primary => request.allow(), + PromptResult::Secondary | PromptResult::Dismissed => request.deny(), }; - - let _ = result_sender.send(result); } fn request_resize_to(&self, _webview: WebView, size: DeviceIntSize) {