diff --git a/components/servo/lib.rs b/components/servo/lib.rs index 58e9c97e8ed..490f24e5a9e 100644 --- a/components/servo/lib.rs +++ b/components/servo/lib.rs @@ -132,6 +132,7 @@ use crate::proxies::ConstellationProxy; use crate::responders::ServoErrorChannel; pub use crate::servo_delegate::{ServoDelegate, ServoError}; use crate::webrender_api::FrameReadyParams; +use crate::webview::MINIMUM_WEBVIEW_SIZE; pub use crate::webview::{WebView, WebViewBuilder}; pub use crate::webview_delegate::{ AllowOrDenyRequest, AuthenticationRequest, ColorPicker, FormControl, NavigationRequest, @@ -693,7 +694,9 @@ impl Servo { }, EmbedderMsg::ResizeTo(webview_id, size) => { if let Some(webview) = self.get_webview_handle(webview_id) { - webview.delegate().request_resize_to(webview, size); + webview + .delegate() + .request_resize_to(webview, size.max(MINIMUM_WEBVIEW_SIZE)); } }, EmbedderMsg::ShowSimpleDialog(webview_id, prompt_definition) => { diff --git a/components/servo/tests/common/mod.rs b/components/servo/tests/common/mod.rs index fcfc78b26ab..b2df4f13c1f 100644 --- a/components/servo/tests/common/mod.rs +++ b/components/servo/tests/common/mod.rs @@ -56,7 +56,7 @@ pub(crate) fn run_test( } pub struct ServoTest { - servo: Servo, + pub servo: Rc, #[allow(dead_code)] pub rendering_context: Rc, } @@ -101,7 +101,7 @@ impl ServoTest { let builder = ServoBuilder::new(rendering_context.clone()) .event_loop_waker(Box::new(EventLoopWakerImpl(user_event_triggered))); let builder = customize(builder); - let servo = builder.build(); + let servo = Rc::new(builder.build()); Self { servo, rendering_context, diff --git a/components/servo/tests/webview.rs b/components/servo/tests/webview.rs index 84f44c34ad8..8c59bfe7e6a 100644 --- a/components/servo/tests/webview.rs +++ b/components/servo/tests/webview.rs @@ -11,16 +11,19 @@ mod common; +use std::cell::{Cell, RefCell}; use std::rc::Rc; use anyhow::ensure; use common::{ServoTest, WebViewDelegateImpl, evaluate_javascript, run_api_tests}; -use euclid::Point2D; +use dpi::PhysicalSize; +use euclid::{Point2D, Size2D}; use servo::{ Cursor, InputEvent, JSValue, JavaScriptEvaluationError, LoadStatus, MouseLeftViewportEvent, - MouseMoveEvent, Theme, WebViewBuilder, + MouseMoveEvent, Servo, Theme, WebView, WebViewBuilder, WebViewDelegate, }; use url::Url; +use webrender_api::units::DeviceIntSize; fn test_create_webview(servo_test: &ServoTest) -> Result<(), anyhow::Error> { let delegate = Rc::new(WebViewDelegateImpl::default()); @@ -199,6 +202,94 @@ fn test_cursor_change(servo_test: &ServoTest) -> Result<(), anyhow::Error> { Ok(()) } +/// A test that ensure that negative resize requests do not get passed to the embedder. +fn test_negative_resize_to_request(servo_test: &ServoTest) -> Result<(), anyhow::Error> { + struct WebViewResizeTestDelegate { + servo: Rc, + popup: RefCell>, + resize_request: Cell>, + } + + impl WebViewDelegate for WebViewResizeTestDelegate { + fn request_open_auxiliary_webview(&self, parent_webview: WebView) -> Option { + let webview = WebViewBuilder::new_auxiliary(&self.servo) + .delegate(parent_webview.delegate()) + .build(); + self.popup.borrow_mut().replace(webview.clone()); + Some(webview) + } + + fn request_resize_to(&self, _: WebView, requested_outer_size: DeviceIntSize) { + self.resize_request.set(Some(requested_outer_size)); + } + } + + let delegate = Rc::new(WebViewResizeTestDelegate { + servo: servo_test.servo.clone(), + popup: None.into(), + resize_request: None.into(), + }); + + let webview = WebViewBuilder::new(servo_test.servo()) + .delegate(delegate.clone()) + .url( + Url::parse( + "data:text/html,", + ) + .unwrap(), + ) + .build(); + + let load_webview = webview.clone(); + let _ = servo_test.spin(move || Ok(load_webview.load_status() != LoadStatus::Complete)); + + let popup = delegate + .popup + .borrow() + .clone() + .expect("Should have created popup"); + + let load_webview = popup.clone(); + let _ = servo_test.spin(move || Ok(load_webview.load_status() != LoadStatus::Complete)); + + // Resize requests should be floored to 1. + ensure!(delegate.resize_request.get() == Some(DeviceIntSize::new(1, 1))); + + // Ensure that the popup WebView is released before the end of the test. + *delegate.popup.borrow_mut() = None; + + Ok(()) +} + +/// This test verifies that trying to set the WebView size to a negative value does +/// not crash Servo. +fn test_resize_webview_zero(servo_test: &ServoTest) -> Result<(), anyhow::Error> { + let delegate = Rc::new(WebViewDelegateImpl::default()); + let webview = WebViewBuilder::new(servo_test.servo()) + .delegate(delegate.clone()) + .url( + Url::parse( + "data:text/html,hello", + ) + .unwrap(), + ) + .build(); + + webview.focus(); + webview.show(true); + + webview.move_resize(Size2D::new(-100.0, -100.0).into()); + webview.resize(PhysicalSize::new(0, 0)); + + let load_webview = webview.clone(); + let _ = servo_test.spin(move || Ok(load_webview.load_status() != LoadStatus::Complete)); + + Ok(()) +} + fn main() { run_api_tests!( test_create_webview, @@ -206,6 +297,8 @@ fn main() { test_evaluate_javascript_basic, test_evaluate_javascript_panic, test_theme_change, + test_negative_resize_to_request, + test_resize_webview_zero, // This test needs to be last, as it tests creating and dropping // a WebView right before shutdown. test_create_webview_and_immediately_drop_webview_before_shutdown diff --git a/components/servo/webview.rs b/components/servo/webview.rs index cbf2caf9da9..8c72cb20e7b 100644 --- a/components/servo/webview.rs +++ b/components/servo/webview.rs @@ -27,6 +27,8 @@ use crate::javascript_evaluator::JavaScriptEvaluator; use crate::webview_delegate::{DefaultWebViewDelegate, WebViewDelegate}; use crate::{ConstellationProxy, Servo, WebRenderDebugOption}; +pub(crate) const MINIMUM_WEBVIEW_SIZE: Size2D = Size2D::new(1, 1); + /// 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 /// considers that the webview has closed and will clean up all associated resources related @@ -343,6 +345,9 @@ impl WebView { return; } + let rect = + DeviceRect::from_origin_and_size(rect.min, rect.size().max(Size2D::new(1.0, 1.0))); + self.inner_mut().rect = rect; self.inner() .compositor @@ -350,7 +355,15 @@ impl WebView { .move_resize_webview(self.id(), rect); } + /// Request that the given [`WebView`]'s rendering area be resized. Note that the + /// minimum size for a WebView is 1 pixel by 1 pixel so any requested size will be + /// clamped by that value. pub fn resize(&self, new_size: PhysicalSize) { + let new_size = PhysicalSize { + width: new_size.width.max(MINIMUM_WEBVIEW_SIZE.width as u32), + height: new_size.height.max(MINIMUM_WEBVIEW_SIZE.height as u32), + }; + self.inner() .compositor .borrow_mut() diff --git a/components/servo/webview_delegate.rs b/components/servo/webview_delegate.rs index 85e0b6d3a5c..d748a538b3b 100644 --- a/components/servo/webview_delegate.rs +++ b/components/servo/webview_delegate.rs @@ -474,7 +474,11 @@ pub trait WebViewDelegate { fn request_unload(&self, _webview: WebView, _unload_request: AllowOrDenyRequest) {} /// Move the window to a point. fn request_move_to(&self, _webview: WebView, _: DeviceIntPoint) {} - /// Try to resize the window that contains this [`WebView`] to the provided outer size. + /// Try to resize the window that contains this [`WebView`] to the provided outer + /// size. These resize requests can come from page content. Servo will ensure that the + /// values are greater than zero, but it is up to the embedder to limit the maximum + /// size. For instance, a reasonable limitation might be that the final size is no + /// larger than the screen size. fn request_resize_to(&self, _webview: WebView, _requested_outer_size: DeviceIntSize) {} /// Whether or not to allow script to open a new `WebView`. If not handled by the /// embedder, these requests are automatically denied. diff --git a/ports/servoshell/desktop/headed_window.rs b/ports/servoshell/desktop/headed_window.rs index e0aae54fbd5..544895b6226 100644 --- a/ports/servoshell/desktop/headed_window.rs +++ b/ports/servoshell/desktop/headed_window.rs @@ -49,12 +49,10 @@ use { use super::app_state::RunningAppState; use super::geometry::{winit_position_to_euclid_point, winit_size_to_euclid_size}; use super::keyutils::{CMD_OR_ALT, keyboard_event_from_winit}; -use super::window_trait::{ - LINE_HEIGHT, LINE_WIDTH, MIN_INNER_HEIGHT, MIN_INNER_WIDTH, PIXEL_DELTA_FACTOR, - WindowPortsMethods, -}; +use super::window_trait::{LINE_HEIGHT, LINE_WIDTH, PIXEL_DELTA_FACTOR, WindowPortsMethods}; use crate::desktop::accelerated_gl_media::setup_gl_accelerated_media; use crate::desktop::keyutils::CMD_OR_CONTROL; +use crate::desktop::window_trait::MIN_WINDOW_INNER_SIZE; use crate::prefs::ServoShellPreferences; pub struct Window { @@ -99,7 +97,10 @@ impl Window { .with_decorations(!no_native_titlebar) .with_transparent(no_native_titlebar) .with_inner_size(LogicalSize::new(inner_size.width, inner_size.height)) - .with_min_inner_size(LogicalSize::new(MIN_INNER_WIDTH, MIN_INNER_HEIGHT)) + .with_min_inner_size(LogicalSize::new( + MIN_WINDOW_INNER_SIZE.width, + MIN_WINDOW_INNER_SIZE.height, + )) // Must be invisible at startup; accesskit_winit setup needs to // happen before the window is shown for the first time. .with_visible(false); @@ -489,26 +490,36 @@ impl WindowPortsMethods for Window { } fn request_resize(&self, _: &WebView, new_outer_size: DeviceIntSize) -> Option { + // Allocate space for the window deocrations, but do not let the inner size get + // smaller than `MIN_WINDOW_INNER_SIZE` or larger than twice the screen size. + let inner_size = self.winit_window.inner_size(); let outer_size = self.winit_window.outer_size(); + let decoration_size: DeviceIntSize = Size2D::new( + outer_size.height - inner_size.height, + outer_size.width - inner_size.width, + ) + .cast(); + + let screen_size = (self.screen_size.to_f32() * self.hidpi_scale_factor()).to_i32(); + let new_outer_size = + new_outer_size.clamp(MIN_WINDOW_INNER_SIZE + decoration_size, screen_size * 2); + if outer_size.width == new_outer_size.width as u32 && outer_size.height == new_outer_size.height as u32 { return Some(new_outer_size); } - let inner_size = self.winit_window.inner_size(); - let decoration_height = outer_size.height - inner_size.height; - let decoration_width = outer_size.width - inner_size.width; - + let new_inner_size = new_outer_size - decoration_size; self.winit_window - .request_inner_size::>(PhysicalSize::new( - new_outer_size.width - decoration_width as i32, - new_outer_size.height - decoration_height as i32, + .request_inner_size(PhysicalSize::new( + new_inner_size.width, + new_inner_size.height, )) .map(|resulting_size| { DeviceIntSize::new( - (resulting_size.width + decoration_width) as i32, - (resulting_size.height + decoration_height) as i32, + resulting_size.width as i32 + decoration_size.width, + resulting_size.height as i32 + decoration_size.height, ) }) } @@ -777,12 +788,6 @@ impl WindowPortsMethods for Window { return; } self.toolbar_height.set(height); - // Prevent the inner area from being 0 pixels wide or tall - // this prevents a crash in the compositor due to invalid surface size - self.winit_window.set_min_inner_size(Some(PhysicalSize::new( - MIN_INNER_WIDTH, - MIN_INNER_HEIGHT.max((self.toolbar_height() * self.hidpi_scale_factor()).0 as i32), - ))); } fn rendering_context(&self) -> Rc { diff --git a/ports/servoshell/desktop/headless_window.rs b/ports/servoshell/desktop/headless_window.rs index e17e09a5218..92b661a88b6 100644 --- a/ports/servoshell/desktop/headless_window.rs +++ b/ports/servoshell/desktop/headless_window.rs @@ -20,7 +20,7 @@ use servo::{RenderingContext, ScreenGeometry, SoftwareRenderingContext, WebView} use winit::dpi::PhysicalSize; use super::app_state::RunningAppState; -use crate::desktop::window_trait::{MIN_INNER_HEIGHT, MIN_INNER_WIDTH, WindowPortsMethods}; +use crate::desktop::window_trait::{MIN_WINDOW_INNER_SIZE, WindowPortsMethods}; use crate::prefs::ServoShellPreferences; pub struct Window { @@ -87,15 +87,10 @@ impl WindowPortsMethods for Window { self.window_position.set(point); } - fn request_resize( - &self, - webview: &WebView, - outer_size: DeviceIntSize, - ) -> Option { - let new_size = DeviceIntSize::new( - outer_size.width.max(MIN_INNER_WIDTH), - outer_size.height.max(MIN_INNER_HEIGHT), - ); + fn request_resize(&self, webview: &WebView, new_size: DeviceIntSize) -> Option { + // Do not let the window size get smaller than `MIN_WINDOW_INNER_SIZE` or larger + // than twice the screen size. + let new_size = new_size.clamp(MIN_WINDOW_INNER_SIZE, self.screen_size * 2); if self.inner_size.get() == new_size { return Some(new_size); } @@ -105,10 +100,10 @@ impl WindowPortsMethods for Window { // Because we are managing the rendering surface ourselves, there will be no other // notification (such as from the display manager) that it has changed size, so we // must notify the compositor here. - webview.move_resize(outer_size.to_f32().into()); + webview.move_resize(new_size.to_f32().into()); webview.resize(PhysicalSize::new( - outer_size.width as u32, - outer_size.height as u32, + new_size.width as u32, + new_size.height as u32, )); Some(new_size) diff --git a/ports/servoshell/desktop/window_trait.rs b/ports/servoshell/desktop/window_trait.rs index cd27cf90b68..d3c7997fe69 100644 --- a/ports/servoshell/desktop/window_trait.rs +++ b/ports/servoshell/desktop/window_trait.rs @@ -24,8 +24,7 @@ pub(crate) const PIXEL_DELTA_FACTOR: f64 = 4.0; /// /// "A window size of 10x10px shouldn't be supported by any browser." -pub(crate) const MIN_INNER_WIDTH: i32 = 20; -pub(crate) const MIN_INNER_HEIGHT: i32 = 20; +pub(crate) const MIN_WINDOW_INNER_SIZE: DeviceIntSize = DeviceIntSize::new(100, 100); pub trait WindowPortsMethods { fn id(&self) -> winit::window::WindowId; diff --git a/tests/wpt/meta/css/cssom-view/resizeTo-negative.html.ini b/tests/wpt/meta/css/cssom-view/resizeTo-negative.html.ini deleted file mode 100644 index fc971e5c333..00000000000 --- a/tests/wpt/meta/css/cssom-view/resizeTo-negative.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[resizeTo-negative.html] - expected: CRASH