Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move tinyfiledialog call from script to embedder #23651

Closed
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Some generated files are not rendered by default. Learn more.

@@ -154,6 +154,9 @@ pub enum EmbedderMsg {
GetSelectedBluetoothDevice(Vec<String>, IpcSender<Option<String>>),
/// Open file dialog to select files. Set boolean flag to true allows to select multiple files.
SelectFiles(Vec<FilterPattern>, bool, IpcSender<Option<Vec<String>>>),
/// Open yes/no message for user to allow permission specified by first String.
/// With dialog title specified by second String.
PromptPermission(String, String, IpcSender<PermissionRequest>),

This comment has been minimized.

@jdm

jdm Jun 28, 2019

Member

I propose instead of sending two strings, we send an enum:

enum PermissionName {
   // variants that match the DOM PermissionName enum
}

enum PermissionPrompt {
    Insecure(PermissionName),
    Request(PermissionName),
}

This gives embedders the information they require in order to display whatever native prompt interface would make sense on that platform and supports localization better in the future.

/// Request to present an IME to the user when an editable element is focused.
ShowIME(InputMethodType),
/// Request to hide the IME when the editable element is blurred.
@@ -186,6 +189,7 @@ impl Debug for EmbedderMsg {
EmbedderMsg::Panic(..) => write!(f, "Panic"),
EmbedderMsg::GetSelectedBluetoothDevice(..) => write!(f, "GetSelectedBluetoothDevice"),
EmbedderMsg::SelectFiles(..) => write!(f, "SelectFiles"),
EmbedderMsg::PromptPermission(..) => write!(f, "PromptPermission"),
EmbedderMsg::ShowIME(..) => write!(f, "ShowIME"),
EmbedderMsg::HideIME => write!(f, "HideIME"),
EmbedderMsg::Shutdown => write!(f, "Shutdown"),
@@ -200,3 +204,10 @@ impl Debug for EmbedderMsg {
/// the `String` content is expected to be extension (e.g, "doc", without the prefixing ".")
#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct FilterPattern(pub String);

// Status for prompting user for permission.

This comment has been minimized.

@jdm

jdm Jun 28, 2019

Member

Use /// here.

#[derive(Clone, Debug, Deserialize, Serialize)]
pub enum PermissionRequest {
Granted,
Denied,
}
@@ -28,9 +28,6 @@ phf_codegen = "0.7"
phf_shared = "0.7"
serde_json = "1.0"

[target.'cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))'.dependencies]
tinyfiledialogs = "3.0"

[dependencies]
app_units = "0.7"
backtrace = {version = "0.3", optional = true}
@@ -4,6 +4,7 @@

use crate::dom::bindings::cell::DomRefCell;
use crate::dom::bindings::codegen::Bindings::EventSourceBinding::EventSourceBinding::EventSourceMethods;
use crate::dom::bindings::codegen::Bindings::PermissionStatusBinding::PermissionState;
use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
use crate::dom::bindings::codegen::Bindings::WorkerGlobalScopeBinding::WorkerGlobalScopeMethods;
use crate::dom::bindings::conversions::{root_from_object, root_from_object_static};
@@ -40,6 +41,7 @@ use crate::timers::{IsInterval, OneshotTimerCallback, OneshotTimerHandle};
use crate::timers::{OneshotTimers, TimerCallback};
use devtools_traits::{ScriptToDevtoolsControlMsg, WorkerId};
use dom_struct::dom_struct;
use embedder_traits::EmbedderMsg;
use ipc_channel::ipc::IpcSender;
use js::glue::{IsWrapper, UnwrapObjectDynamic};
use js::jsapi::JSObject;
@@ -55,7 +57,7 @@ use msg::constellation_msg::PipelineId;
use net_traits::image_cache::ImageCache;
use net_traits::{CoreResourceThread, IpcSend, ResourceThreads};
use profile_traits::{mem as profile_mem, time as profile_time};
use script_traits::{MsDuration, ScriptToConstellationChan, TimerEvent};
use script_traits::{MsDuration, ScriptMsg, ScriptToConstellationChan, TimerEvent};
use script_traits::{TimerEventId, TimerSchedulerMsg, TimerSource};
use servo_url::{MutableOrigin, ServoUrl};
use std::borrow::Cow;
@@ -124,6 +126,9 @@ pub struct GlobalScope {
/// The origin of the globalscope
origin: MutableOrigin,

/// A map for storing the previous permission state read results.
permission_state_invocation_results: DomRefCell<HashMap<String, PermissionState>>,

/// The microtask queue associated with this global.
///
/// It is refcounted because windows in the same script thread share the
@@ -197,6 +202,7 @@ impl GlobalScope {
resource_threads,
timers: OneshotTimers::new(timer_event_chan, scheduler_chan),
origin,
permission_state_invocation_results: Default::default(),
microtask_queue,
list_auto_close_worker: Default::default(),
event_source_tracker: DOMTracker::new(),
@@ -207,6 +213,12 @@ impl GlobalScope {
}
}

pub fn permission_state_invocation_results(
&self,
) -> &DomRefCell<HashMap<String, PermissionState>> {
&self.permission_state_invocation_results
}

pub fn track_worker(&self, closing_worker: Arc<AtomicBool>) {
self.list_auto_close_worker
.borrow_mut()
@@ -378,6 +390,14 @@ impl GlobalScope {
&self.script_to_constellation_chan
}

pub fn send_to_embedder(&self, msg: EmbedderMsg) {
self.send_to_constellation(ScriptMsg::ForwardToEmbedder(msg));
}

pub fn send_to_constellation(&self, msg: ScriptMsg) {
self.script_to_constellation_chan().send(msg).unwrap();
}

pub fn scheduler_chan(&self) -> &IpcSender<TimerSchedulerMsg> {
&self.scheduler_chan
}
@@ -18,15 +18,14 @@ use crate::dom::globalscope::GlobalScope;
use crate::dom::permissionstatus::PermissionStatus;
use crate::dom::promise::Promise;
use dom_struct::dom_struct;
use embedder_traits::{EmbedderMsg, PermissionRequest};
use ipc_channel::ipc;
use js::conversions::ConversionResult;
use js::jsapi::{JSContext, JSObject};
use js::jsval::{ObjectValue, UndefinedValue};
use servo_config::pref;
use std::rc::Rc;
#[cfg(target_os = "linux")]
use tinyfiledialogs::{self, MessageBoxIcon, YesNo};

#[cfg(target_os = "linux")]
const DIALOG_TITLE: &'static str = "Permission request dialog";
const NONSECURE_DIALOG_MESSAGE: &'static str = "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?";
@@ -146,7 +145,6 @@ impl Permissions {
// (Revoke) Step 3.
let globalscope = self.global();
globalscope
.as_window()
.permission_state_invocation_results()
.borrow_mut()
.remove(&root_desc.name.to_string());
@@ -179,7 +177,6 @@ impl Permissions {
// (Revoke) Step 3.
let globalscope = self.global();
globalscope
.as_window()
.permission_state_invocation_results()
.borrow_mut()
.remove(&root_desc.name.to_string());
@@ -268,16 +265,11 @@ impl PermissionAlgorithm for Permissions {
PermissionState::Prompt => {
let perm_name = status.get_query();

let globalscope = GlobalScope::current().expect("No current global object");

// https://w3c.github.io/permissions/#request-permission-to-use (Step 3 - 4)
let state = prompt_user(
&format!("{} {} ?", REQUEST_DIALOG_MESSAGE, perm_name.clone()),
globalscope.is_headless(),
);

let globalscope = GlobalScope::current().expect("No current global object");
let prompt = format!("{} {} ?", REQUEST_DIALOG_MESSAGE, perm_name.clone());

This comment has been minimized.

@jdm

jdm Jun 28, 2019

Member

Rather than doing string formatting for dialogs in this code, I think we should delegate that to the embedder and only send an enum value that represents the kind of prompt required.

let state = prompt_user_from_embedder(prompt, &globalscope);
globalscope
.as_window()
.permission_state_invocation_results()
.borrow_mut()
.insert(perm_name.to_string(), state);
@@ -300,7 +292,7 @@ pub fn get_descriptor_permission_state(
env_settings_obj: Option<&GlobalScope>,
) -> PermissionState {
// Step 1.
let settings = match env_settings_obj {
let globalscope = match env_settings_obj {
Some(env_settings_obj) => DomRoot::from_ref(env_settings_obj),
None => GlobalScope::current().expect("No current global object"),
};
@@ -316,22 +308,18 @@ pub fn get_descriptor_permission_state(
if pref!(dom.permissions.testing.allowed_in_nonsecure_contexts) {
PermissionState::Granted
} else {
settings
.as_window()
globalscope
.permission_state_invocation_results()
.borrow_mut()
.remove(&permission_name.to_string());

prompt_user(
&format!("The {} {}", permission_name, NONSECURE_DIALOG_MESSAGE),
settings.is_headless(),
)
let prompt = format!("The {} {}", permission_name, NONSECURE_DIALOG_MESSAGE);
prompt_user_from_embedder(prompt, &globalscope)
}
};

// Step 3.
if let Some(prev_result) = settings
.as_window()
if let Some(prev_result) = globalscope
.permission_state_invocation_results()
.borrow()
.get(&permission_name.to_string())
@@ -340,8 +328,7 @@ pub fn get_descriptor_permission_state(
}

// Store the invocation result
settings
.as_window()
globalscope
.permission_state_invocation_results()
.borrow_mut()
.insert(permission_name.to_string(), state);
@@ -350,28 +337,6 @@ pub fn get_descriptor_permission_state(
state
}

#[cfg(target_os = "linux")]
fn prompt_user(message: &str, headless: bool) -> PermissionState {
if headless {
return PermissionState::Denied;
}
match tinyfiledialogs::message_box_yes_no(
DIALOG_TITLE,
message,
MessageBoxIcon::Question,
YesNo::No,
) {
YesNo::Yes => PermissionState::Granted,
YesNo::No => PermissionState::Denied,
}
}

#[cfg(not(target_os = "linux"))]
fn prompt_user(_message: &str, _headless: bool) -> PermissionState {
// TODO popup only supported on linux
PermissionState::Denied
}

// https://w3c.github.io/permissions/#allowed-in-non-secure-contexts
fn allowed_in_nonsecure_contexts(permission_name: &PermissionName) -> bool {
match *permission_name {
@@ -399,3 +364,21 @@ fn allowed_in_nonsecure_contexts(permission_name: &PermissionName) -> bool {
PermissionName::Persistent_storage => false,
}
}

fn prompt_user_from_embedder(prompt: String, gs: &GlobalScope) -> PermissionState {
let (sender, receiver) = ipc::channel().expect("Failed to create IPC channel!");
gs.send_to_embedder(EmbedderMsg::PromptPermission(
prompt,
DIALOG_TITLE.to_string(),
sender,
));

match receiver.recv() {
Ok(PermissionRequest::Granted) => PermissionState::Granted,
Ok(PermissionRequest::Denied) => PermissionState::Denied,
Err(e) => {
warn!("Failed to receive permission state from embedder ({}).", e);
PermissionState::Denied
},
}
}
@@ -10,7 +10,6 @@ use crate::dom::bindings::codegen::Bindings::DocumentBinding::{
use crate::dom::bindings::codegen::Bindings::FunctionBinding::Function;
use crate::dom::bindings::codegen::Bindings::HistoryBinding::HistoryBinding::HistoryMethods;
use crate::dom::bindings::codegen::Bindings::MediaQueryListBinding::MediaQueryListBinding::MediaQueryListMethods;
use crate::dom::bindings::codegen::Bindings::PermissionStatusBinding::PermissionState;
use crate::dom::bindings::codegen::Bindings::RequestBinding::RequestInit;
use crate::dom::bindings::codegen::Bindings::WindowBinding::{
self, FrameRequestCallback, WindowMethods,
@@ -265,9 +264,6 @@ pub struct Window {
#[ignore_malloc_size_of = "channels are hard"]
webvr_chan: Option<IpcSender<WebVRMsg>>,

/// A map for storing the previous permission state read results.
permission_state_invocation_results: DomRefCell<HashMap<String, PermissionState>>,

/// All of the elements that have an outstanding image request that was
/// initiated by layout during a reflow. They are stored in the script thread
/// to ensure that the element can be marked dirty when the image data becomes
@@ -435,12 +431,6 @@ impl Window {
Worklet::new(self, WorkletGlobalScopeType::Paint)
}

pub fn permission_state_invocation_results(
&self,
) -> &DomRefCell<HashMap<String, PermissionState>> {
&self.permission_state_invocation_results
}

pub fn pending_image_notification(&self, response: PendingImageResponse) {
//XXXjdm could be more efficient to send the responses to the layout thread,
// rather than making the layout thread talk to the image cache to
@@ -2141,7 +2131,6 @@ impl Window {
test_runner: Default::default(),
webgl_chan,
webvr_chan,
permission_state_invocation_results: Default::default(),
pending_layout_images: Default::default(),
unminified_js_dir: Default::default(),
test_worklet: Default::default(),
@@ -7,7 +7,7 @@ use crate::window_trait::{WindowPortsMethods, LINE_HEIGHT};
use euclid::{TypedPoint2D, TypedVector2D};
use keyboard_types::{Key, KeyboardEvent, Modifiers, ShortcutMatcher};
use servo::compositing::windowing::{WebRenderDebugOption, WindowEvent};
use servo::embedder_traits::{EmbedderMsg, FilterPattern};
use servo::embedder_traits::{EmbedderMsg, FilterPattern, PermissionRequest};
use servo::msg::constellation_msg::TopLevelBrowsingContextId as BrowserId;
use servo::msg::constellation_msg::TraversalDirection;
use servo::net_traits::pub_domains::is_reg_domain;
@@ -23,7 +23,7 @@ use std::mem;
use std::rc::Rc;
use std::thread;
use std::time::Duration;
use tinyfiledialogs::{self, MessageBoxIcon};
use tinyfiledialogs::{self, MessageBoxIcon, YesNo};

pub struct Browser<Window: WindowPortsMethods + ?Sized> {
current_url: Option<ServoUrl>,
@@ -401,6 +401,10 @@ where
self.event_queue.push(WindowEvent::SendError(None, reason));
};
},
EmbedderMsg::PromptPermission(message, dialog_title, sender) => {

This comment has been minimized.

@jdm

jdm Jun 28, 2019

Member

If you build ./mach build -d --libsimpleservo, you will see code in ports/libsimpleservo/api and ports/libsimpleservo/capi that need to handle this (by invoking a new method in HostTrait), and you can add stub implementations to ports/libsimpleservo/jniapi and ports/libmlservo for this new method as well.

let permission_state = prompt_user(&message, &dialog_title);
let _ = sender.send(permission_state);
}
EmbedderMsg::ShowIME(_kind) => {
debug!("ShowIME received");
},
@@ -419,6 +423,28 @@ where
}
}

#[cfg(target_os = "linux")]
fn prompt_user(prompt: &str, dialog_title: &str) -> PermissionRequest {
if opts::get().headless {
return PermissionRequest::Denied;
}
match tinyfiledialogs::message_box_yes_no(
dialog_title,
prompt,
MessageBoxIcon::Question,
YesNo::No,
) {
YesNo::Yes => PermissionRequest::Granted,
YesNo::No => PermissionRequest::Denied,
}
}

#[cfg(not(target_os = "linux"))]
fn prompt_user(_prompt: &str, _dialog_title: &str) -> PermissionRequest {
// TODO popup only supported on linux
PermissionRequest::Denied
}

#[cfg(target_os = "linux")]
fn platform_get_selected_devices(devices: Vec<String>) -> Option<String> {
let picker_name = "Choose a device";
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.