diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index ea22e68d993..e2ec2920d3d 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -129,7 +129,7 @@ use devtools_traits::{ use embedder_traits::resources::{self, Resource}; use embedder_traits::user_content_manager::UserContentManager; use embedder_traits::{ - AnimationState, CompositorHitTestResult, Cursor, EmbedderMsg, EmbedderProxy, + AnimationState, CompositorHitTestResult, Cursor, EmbedderMsg, EmbedderProxy, FocusId, FocusSequenceNumber, InputEvent, JSValue, JavaScriptEvaluationError, JavaScriptEvaluationId, KeyboardEvent, MediaSessionActionType, MediaSessionEvent, MediaSessionPlaybackState, MouseButton, MouseButtonAction, MouseButtonEvent, Theme, ViewportDetails, WebDriverCommandMsg, @@ -1347,8 +1347,8 @@ where } self.handle_panic(webview_id, error, None); }, - EmbedderToConstellationMessage::FocusWebView(webview_id, response_sender) => { - self.handle_focus_web_view(webview_id, response_sender); + EmbedderToConstellationMessage::FocusWebView(webview_id, focus_id) => { + self.handle_focus_web_view(webview_id, focus_id); }, EmbedderToConstellationMessage::BlurWebView => { self.webviews.unfocus(); @@ -2755,20 +2755,13 @@ where } #[servo_tracing::instrument(skip_all)] - fn handle_focus_web_view( - &mut self, - webview_id: WebViewId, - response_sender: Option>, - ) { - if self.webviews.get(webview_id).is_none() { - if let Some(response_sender) = response_sender { - let _ = response_sender.send(false); - } - return warn!("{webview_id}: FocusWebView on unknown top-level browsing context"); + fn handle_focus_web_view(&mut self, webview_id: WebViewId, focus_id: FocusId) { + let focused = self.webviews.focus(webview_id).is_ok(); + if !focused { + warn!("{webview_id}: FocusWebView on unknown top-level browsing context"); } - self.webviews.focus(webview_id); self.embedder_proxy - .send(EmbedderMsg::WebViewFocused(webview_id, response_sender)); + .send(EmbedderMsg::WebViewFocused(webview_id, focus_id, focused)); } #[servo_tracing::instrument(skip_all)] @@ -4121,9 +4114,12 @@ where } // Focus the top-level browsing context. - self.webviews.focus(webview_id); - self.embedder_proxy - .send(EmbedderMsg::WebViewFocused(webview_id, None)); + let focused = self.webviews.focus(webview_id); + self.embedder_proxy.send(EmbedderMsg::WebViewFocused( + webview_id, + FocusId::new(), + focused.is_ok(), + )); // If a container with a non-null nested browsing context is focused, // the nested browsing context's active document becomes the focused diff --git a/components/constellation/webview_manager.rs b/components/constellation/webview_manager.rs index 7d6e98a54ea..9b4a189a74a 100644 --- a/components/constellation/webview_manager.rs +++ b/components/constellation/webview_manager.rs @@ -67,11 +67,14 @@ impl WebViewManager { } } - pub fn focus(&mut self, webview_id: WebViewId) { - debug_assert!(self.webviews.contains_key(&webview_id)); + pub fn focus(&mut self, webview_id: WebViewId) -> Result<(), ()> { + if !self.webviews.contains_key(&webview_id) { + return Err(()); + } self.focus_order.retain(|b| *b != webview_id); self.focus_order.push(webview_id); self.is_focused = true; + Ok(()) } pub fn unfocus(&mut self) { diff --git a/components/servo/lib.rs b/components/servo/lib.rs index cae4dd3f9f1..a6ee7c6efda 100644 --- a/components/servo/lib.rs +++ b/components/servo/lib.rs @@ -732,15 +732,17 @@ impl Servo { webview.delegate().notify_closed(webview); } }, - EmbedderMsg::WebViewFocused(webview_id, response_sender) => { - for id in self.webviews.borrow().keys() { - if let Some(webview) = self.get_webview_handle(*id) { - let focused = webview.id() == webview_id; - webview.set_focused(focused); + EmbedderMsg::WebViewFocused(webview_id, focus_id, focus_result) => { + if focus_result { + for id in self.webviews.borrow().keys() { + if let Some(webview) = self.get_webview_handle(*id) { + let focused = webview.id() == webview_id; + webview.set_focused(focused); + } } } - if let Some(response_sender) = response_sender { - let _ = response_sender.send(true); + if let Some(webview) = self.get_webview_handle(webview_id) { + webview.complete_focus(focus_id); } }, EmbedderMsg::WebViewBlurred => { diff --git a/components/servo/webview.rs b/components/servo/webview.rs index 6d190f89e97..2c7714b9981 100644 --- a/components/servo/webview.rs +++ b/components/servo/webview.rs @@ -13,8 +13,8 @@ use compositing_traits::WebViewTrait; use constellation_traits::{EmbedderToConstellationMessage, TraversalDirection}; use dpi::PhysicalSize; use embedder_traits::{ - Cursor, InputEvent, JSValue, JavaScriptEvaluationError, LoadStatus, MediaSessionActionType, - ScreenGeometry, Theme, TraversalId, ViewportDetails, + Cursor, FocusId, InputEvent, JSValue, JavaScriptEvaluationError, LoadStatus, + MediaSessionActionType, ScreenGeometry, Theme, TraversalId, ViewportDetails, }; use euclid::{Point2D, Scale, Size2D}; use servo_geometry::DeviceIndependentPixel; @@ -25,7 +25,7 @@ use webrender_api::units::{DeviceIntPoint, DevicePixel, DeviceRect}; use crate::clipboard_delegate::{ClipboardDelegate, DefaultClipboardDelegate}; use crate::javascript_evaluator::JavaScriptEvaluator; use crate::webview_delegate::{DefaultWebViewDelegate, WebViewDelegate}; -use crate::{ConstellationProxy, IpcSender, Servo, WebRenderDebugOption}; +use crate::{ConstellationProxy, Servo, WebRenderDebugOption}; /// A handle to a Servo webview. If you clone this handle, it does not create a new webview, /// but instead creates a new handle to the webview. Once the last handle is dropped, Servo @@ -293,6 +293,10 @@ impl WebView { self.delegate().notify_focus_changed(self, new_value); } + pub(crate) fn complete_focus(self, focus_id: FocusId) { + self.delegate().notify_focus_complete(self, focus_id); + } + pub fn cursor(&self) -> Cursor { self.inner().cursor } @@ -305,22 +309,15 @@ impl WebView { self.delegate().notify_cursor_changed(self, new_value); } - pub fn focus(&self) { + pub fn focus(&self) -> FocusId { + let focus_id = FocusId::new(); self.inner() .constellation_proxy .send(EmbedderToConstellationMessage::FocusWebView( self.id(), - None, - )); - } - - pub fn focus_from_webdriver(&self, response_sender: IpcSender) { - self.inner() - .constellation_proxy - .send(EmbedderToConstellationMessage::FocusWebView( - self.id(), - Some(response_sender), + focus_id.clone(), )); + focus_id } pub fn blur(&self) { diff --git a/components/servo/webview_delegate.rs b/components/servo/webview_delegate.rs index 6e2290dec06..e55c0fb0134 100644 --- a/components/servo/webview_delegate.rs +++ b/components/servo/webview_delegate.rs @@ -7,7 +7,7 @@ use std::path::PathBuf; use base::id::PipelineId; use constellation_traits::EmbedderToConstellationMessage; use embedder_traits::{ - AllowOrDeny, AuthenticationResponse, ContextMenuResult, Cursor, FilterPattern, + AllowOrDeny, AuthenticationResponse, ContextMenuResult, Cursor, FilterPattern, FocusId, GamepadHapticEffectType, InputMethodType, KeyboardEvent, LoadStatus, MediaSessionEvent, Notification, PermissionFeature, RgbColor, ScreenGeometry, SelectElementOptionOrOptgroup, SimpleDialog, TraversalId, WebResourceRequest, WebResourceResponse, WebResourceResponseMsg, @@ -418,6 +418,9 @@ pub trait WebViewDelegate { /// This [`WebView`] has either become focused or lost focus. Whether or not the /// [`WebView`] is focused can be accessed via [`WebView::focused`]. fn notify_focus_changed(&self, _webview: WebView, _focused: bool) {} + /// A focus operation that was initiated by this webview has completed. + /// The current focus status of this [`WebView`] can be accessed via [`WebView::focused`]. + fn notify_focus_complete(&self, _webview: WebView, _focus_id: FocusId) {} /// This [`WebView`] has either started to animate or stopped animating. When a /// [`WebView`] is animating, it is up to the embedding application ensure that /// `Servo::spin_event_loop` is called at regular intervals in order to update the diff --git a/components/shared/constellation/lib.rs b/components/shared/constellation/lib.rs index fa724a5d286..99219d62c11 100644 --- a/components/shared/constellation/lib.rs +++ b/components/shared/constellation/lib.rs @@ -19,8 +19,9 @@ use base::Epoch; use base::cross_process_instant::CrossProcessInstant; use base::id::{MessagePortId, PipelineId, WebViewId}; use embedder_traits::{ - CompositorHitTestResult, Cursor, InputEvent, JavaScriptEvaluationId, MediaSessionActionType, - Theme, TraversalId, ViewportDetails, WebDriverCommandMsg, WebDriverCommandResponse, + CompositorHitTestResult, Cursor, FocusId, InputEvent, JavaScriptEvaluationId, + MediaSessionActionType, Theme, TraversalId, ViewportDetails, WebDriverCommandMsg, + WebDriverCommandResponse, }; pub use from_script_message::*; use ipc_channel::ipc::IpcSender; @@ -68,9 +69,9 @@ pub enum EmbedderToConstellationMessage { CloseWebView(WebViewId), /// Panic a top level browsing context. SendError(Option, String), - /// Make a webview focused. If sender is provided, it will be used to send back a - /// bool indicating whether the focus was successfully set in EmbedderMsg::WebViewFocused. - FocusWebView(WebViewId, Option>), + /// Make a webview focused. [EmbedderMsg::WebViewFocused] will be sent with + /// the result of this operation. + FocusWebView(WebViewId, FocusId), /// Make none of the webviews focused. BlurWebView, /// Forward an input event to an appropriate ScriptTask. diff --git a/components/shared/embedder/lib.rs b/components/shared/embedder/lib.rs index 62aef6a89b5..c7f0ca46984 100644 --- a/components/shared/embedder/lib.rs +++ b/components/shared/embedder/lib.rs @@ -327,8 +327,19 @@ pub struct ScreenMetrics { pub available_size: DeviceIndependentIntSize, } +/// An opaque identifier for a single webview focus operation. +#[derive(Clone, Deserialize, Eq, Hash, PartialEq, Serialize)] +pub struct FocusId(String); + +impl FocusId { + #[allow(clippy::new_without_default)] + pub fn new() -> Self { + Self(Uuid::new_v4().to_string()) + } +} + /// An opaque identifier for a single history traversal operation. -#[derive(Clone, Deserialize, PartialEq, Serialize)] +#[derive(Clone, Deserialize, Eq, Hash, PartialEq, Serialize)] pub struct TraversalId(String); impl TraversalId { @@ -372,10 +383,10 @@ pub enum EmbedderMsg { AllowOpeningWebView(WebViewId, IpcSender>), /// A webview was destroyed. WebViewClosed(WebViewId), - /// A webview gained focus for keyboard events - /// If sender is provided, it will be used to send back a - /// bool indicating whether the focus was successfully set. - WebViewFocused(WebViewId, Option>), + /// A webview potentially gained focus for keyboard events, as initiated + /// by the provided focus id. If the boolean value is false, the webiew + /// could not be focused. + WebViewFocused(WebViewId, FocusId, bool), /// All webviews lost focus for keyboard events. WebViewBlurred, /// Wether or not to unload a document diff --git a/ports/servoshell/desktop/app.rs b/ports/servoshell/desktop/app.rs index 10206c414d8..929c4ef1634 100644 --- a/ports/servoshell/desktop/app.rs +++ b/ports/servoshell/desktop/app.rs @@ -380,7 +380,8 @@ impl App { }, WebDriverCommandMsg::FocusWebView(webview_id, response_sender) => { if let Some(webview) = running_state.webview_by_id(webview_id) { - webview.focus_from_webdriver(response_sender); + let focus_id = webview.focus(); + running_state.set_pending_focus(focus_id, response_sender); } }, WebDriverCommandMsg::GetWindowRect(_webview_id, response_sender) => { @@ -478,21 +479,13 @@ impl App { WebDriverCommandMsg::GoBack(webview_id, load_status_sender) => { if let Some(webview) = running_state.webview_by_id(webview_id) { let traversal_id = webview.go_back(1); - running_state.set_pending_traversal( - webview_id, - traversal_id, - load_status_sender, - ); + running_state.set_pending_traversal(traversal_id, load_status_sender); } }, WebDriverCommandMsg::GoForward(webview_id, load_status_sender) => { if let Some(webview) = running_state.webview_by_id(webview_id) { let traversal_id = webview.go_forward(1); - running_state.set_pending_traversal( - webview_id, - traversal_id, - load_status_sender, - ); + running_state.set_pending_traversal(traversal_id, load_status_sender); } }, // Key events don't need hit test so can be forwarded to constellation for now diff --git a/ports/servoshell/desktop/app_state.rs b/ports/servoshell/desktop/app_state.rs index d09d1cbc5de..2f2a94aa63b 100644 --- a/ports/servoshell/desktop/app_state.rs +++ b/ports/servoshell/desktop/app_state.rs @@ -18,10 +18,11 @@ use servo::ipc_channel::ipc::IpcSender; use servo::webrender_api::ScrollLocation; use servo::webrender_api::units::{DeviceIntPoint, DeviceIntSize}; use servo::{ - AllowOrDenyRequest, AuthenticationRequest, FilterPattern, FormControl, GamepadHapticEffectType, - KeyboardEvent, LoadStatus, PermissionRequest, Servo, ServoDelegate, ServoError, SimpleDialog, - TraversalId, WebDriverCommandMsg, WebDriverJSResult, WebDriverJSValue, WebDriverLoadStatus, - WebDriverUserPrompt, WebView, WebViewBuilder, WebViewDelegate, + AllowOrDenyRequest, AuthenticationRequest, FilterPattern, FocusId, FormControl, + GamepadHapticEffectType, KeyboardEvent, LoadStatus, PermissionRequest, Servo, ServoDelegate, + ServoError, SimpleDialog, TraversalId, WebDriverCommandMsg, WebDriverJSResult, + WebDriverJSValue, WebDriverLoadStatus, WebDriverUserPrompt, WebView, WebViewBuilder, + WebViewDelegate, }; use url::Url; @@ -45,7 +46,8 @@ pub(crate) enum AppState { struct WebDriverSenders { pub load_status_senders: HashMap>, pub script_evaluation_interrupt_sender: Option>, - pub pending_traversals: HashMap)>, + pub pending_traversals: HashMap>, + pub pending_focus: HashMap>, } pub(crate) struct RunningAppState { @@ -267,7 +269,9 @@ impl RunningAppState { .and_then(|id| inner.webviews.get(id)); match last_created { - Some(last_created_webview) => last_created_webview.focus(), + Some(last_created_webview) => { + last_created_webview.focus(); + }, None => self.servo.start_shutting_down(), } } @@ -446,16 +450,22 @@ impl RunningAppState { }); } + pub(crate) fn set_pending_focus(&self, focus_id: FocusId, sender: IpcSender) { + self.webdriver_senders + .borrow_mut() + .pending_focus + .insert(focus_id, sender); + } + pub(crate) fn set_pending_traversal( &self, - webview_id: WebViewId, traversal_id: TraversalId, sender: IpcSender, ) { self.webdriver_senders .borrow_mut() .pending_traversals - .insert(webview_id, (traversal_id, sender)); + .insert(traversal_id, sender); } pub(crate) fn set_load_status_sender( @@ -539,13 +549,11 @@ impl WebViewDelegate for RunningAppState { } } - fn notify_traversal_complete(&self, webview: servo::WebView, traversal_id: TraversalId) { + fn notify_traversal_complete(&self, _webview: servo::WebView, traversal_id: TraversalId) { let mut webdriver_state = self.webdriver_senders.borrow_mut(); - if let Entry::Occupied(entry) = webdriver_state.pending_traversals.entry(webview.id()) { - if entry.get().0 == traversal_id { - let (_, sender) = entry.remove(); - let _ = sender.send(WebDriverLoadStatus::Complete); - } + if let Entry::Occupied(entry) = webdriver_state.pending_traversals.entry(traversal_id) { + let sender = entry.remove(); + let _ = sender.send(WebDriverLoadStatus::Complete); } } @@ -638,6 +646,14 @@ impl WebViewDelegate for RunningAppState { self.close_webview(webview.id()); } + fn notify_focus_complete(&self, webview: servo::WebView, focus_id: FocusId) { + let mut webdriver_state = self.webdriver_senders.borrow_mut(); + if let Entry::Occupied(entry) = webdriver_state.pending_focus.entry(focus_id) { + let sender = entry.remove(); + let _ = sender.send(webview.focused()); + } + } + fn notify_focus_changed(&self, webview: servo::WebView, focused: bool) { let mut inner_mut = self.inner_mut(); if focused {