Auto merge of #22396 - jdm:no-spam-iframe-size, r=asajeffrey

Reduce unnecessary iframe size messages

This should be an improvement on pages that include iframes, since we currently run two layout jobs for every display-oriented layout request. When building the display list, we send a message to the constellation that includes the sizes of all iframes present, and the constellation sends resize messages to the script thread. This results in a mouse event on the outer page causing all frames to be re-laid out even if no changes occurred to the iframe sizes, which is ridiculous.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22394
- [x] These changes do not require tests because there is no way to test this internal detail.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22396)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2019-01-14 19:12:59 -05:00 committed by GitHub
commit 2e439fa7cb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 76 additions and 21 deletions

View file

@ -146,9 +146,9 @@ use script_traits::{DocumentActivity, DocumentState, LayoutControlMsg, LoadData}
use script_traits::{ use script_traits::{
IFrameLoadInfo, IFrameLoadInfoWithData, IFrameSandboxState, TimerSchedulerMsg, IFrameLoadInfo, IFrameLoadInfoWithData, IFrameSandboxState, TimerSchedulerMsg,
}; };
use script_traits::{IFrameSizeMsg, WindowSizeData, WindowSizeType};
use script_traits::{LayoutMsg as FromLayoutMsg, ScriptMsg as FromScriptMsg, ScriptThreadFactory}; use script_traits::{LayoutMsg as FromLayoutMsg, ScriptMsg as FromScriptMsg, ScriptThreadFactory};
use script_traits::{SWManagerMsg, ScopeThings, UpdatePipelineIdReason, WebDriverCommandMsg}; use script_traits::{SWManagerMsg, ScopeThings, UpdatePipelineIdReason, WebDriverCommandMsg};
use script_traits::{WindowSizeData, WindowSizeType};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use servo_config::opts; use servo_config::opts;
use servo_config::prefs::PREFS; use servo_config::prefs::PREFS;
@ -1837,17 +1837,14 @@ where
} }
} }
fn handle_iframe_size_msg( fn handle_iframe_size_msg(&mut self, iframe_sizes: Vec<IFrameSizeMsg>) {
&mut self, for IFrameSizeMsg { data, type_ } in iframe_sizes {
iframe_sizes: Vec<(BrowsingContextId, TypedSize2D<f32, CSSPixel>)>,
) {
for (browsing_context_id, size) in iframe_sizes {
let window_size = WindowSizeData { let window_size = WindowSizeData {
initial_viewport: size, initial_viewport: data.size,
device_pixel_ratio: self.window_size.device_pixel_ratio, device_pixel_ratio: self.window_size.device_pixel_ratio,
}; };
self.resize_browsing_context(window_size, WindowSizeType::Initial, browsing_context_id); self.resize_browsing_context(window_size, type_, data.id);
} }
} }

View file

@ -37,9 +37,10 @@ use gfx::text::glyph::ByteIndex;
use gfx::text::TextRun; use gfx::text::TextRun;
use gfx_traits::{combine_id_with_fragment_type, FragmentType, StackingContextId}; use gfx_traits::{combine_id_with_fragment_type, FragmentType, StackingContextId};
use ipc_channel::ipc; use ipc_channel::ipc;
use msg::constellation_msg::{BrowsingContextId, PipelineId}; use msg::constellation_msg::PipelineId;
use net_traits::image_cache::UsePlaceholder; use net_traits::image_cache::UsePlaceholder;
use range::Range; use range::Range;
use script_traits::IFrameSize;
use servo_config::opts; use servo_config::opts;
use servo_geometry::MaxRect; use servo_geometry::MaxRect;
use std::default::Default; use std::default::Default;
@ -61,7 +62,6 @@ use style::values::generics::background::BackgroundSize;
use style::values::generics::image::{GradientKind, Image, PaintWorklet}; use style::values::generics::image::{GradientKind, Image, PaintWorklet};
use style::values::{Either, RGBA}; use style::values::{Either, RGBA};
use style_traits::cursor::CursorKind; use style_traits::cursor::CursorKind;
use style_traits::CSSPixel;
use style_traits::ToCss; use style_traits::ToCss;
use webrender_api::{ use webrender_api::{
self, BorderDetails, BorderRadius, BorderSide, BoxShadowClipMode, ColorF, ColorU, self, BorderDetails, BorderRadius, BorderSide, BoxShadowClipMode, ColorF, ColorU,
@ -318,7 +318,7 @@ pub struct DisplayListBuildState<'a> {
/// Vector containing iframe sizes, used to inform the constellation about /// Vector containing iframe sizes, used to inform the constellation about
/// new iframe sizes /// new iframe sizes
pub iframe_sizes: Vec<(BrowsingContextId, TypedSize2D<f32, CSSPixel>)>, pub iframe_sizes: Vec<IFrameSize>,
/// Stores text runs to answer text queries used to place a cursor inside text. /// Stores text runs to answer text queries used to place a cursor inside text.
pub indexable_text: IndexableText, pub indexable_text: IndexableText,
@ -1785,9 +1785,10 @@ impl Fragment {
})); }));
let size = Size2D::new(item.bounds().size.width, item.bounds().size.height); let size = Size2D::new(item.bounds().size.width, item.bounds().size.height);
state state.iframe_sizes.push(IFrameSize {
.iframe_sizes id: browsing_context_id,
.push((browsing_context_id, TypedSize2D::from_untyped(&size))); size: TypedSize2D::from_untyped(&size),
});
state.add_display_item(item); state.add_display_item(item);
} }

View file

@ -68,8 +68,8 @@ use metrics::{PaintTimeMetrics, ProfilerMetadataFactory, ProgressiveWebMetric};
use msg::constellation_msg::{ use msg::constellation_msg::{
BackgroundHangMonitor, BackgroundHangMonitorRegister, HangAnnotation, BackgroundHangMonitor, BackgroundHangMonitorRegister, HangAnnotation,
}; };
use msg::constellation_msg::{BrowsingContextId, MonitoredComponentId, TopLevelBrowsingContextId};
use msg::constellation_msg::{LayoutHangAnnotation, MonitoredComponentType, PipelineId}; use msg::constellation_msg::{LayoutHangAnnotation, MonitoredComponentType, PipelineId};
use msg::constellation_msg::{MonitoredComponentId, TopLevelBrowsingContextId};
use net_traits::image_cache::{ImageCache, UsePlaceholder}; use net_traits::image_cache::{ImageCache, UsePlaceholder};
use parking_lot::RwLock; use parking_lot::RwLock;
use profile_traits::mem::{self as profile_mem, Report, ReportKind, ReportsChan}; use profile_traits::mem::{self as profile_mem, Report, ReportKind, ReportsChan};
@ -82,7 +82,7 @@ use script_layout_interface::rpc::{LayoutRPC, OffsetParentResponse, StyleRespons
use script_layout_interface::wrapper_traits::LayoutNode; use script_layout_interface::wrapper_traits::LayoutNode;
use script_traits::Painter; use script_traits::Painter;
use script_traits::{ConstellationControlMsg, LayoutControlMsg, LayoutMsg as ConstellationMsg}; use script_traits::{ConstellationControlMsg, LayoutControlMsg, LayoutMsg as ConstellationMsg};
use script_traits::{DrawAPaintImageResult, PaintWorkletError}; use script_traits::{DrawAPaintImageResult, IFrameSizeMsg, PaintWorkletError, WindowSizeType};
use script_traits::{ScrollState, UntrustedNodeAddress}; use script_traits::{ScrollState, UntrustedNodeAddress};
use selectors::Element; use selectors::Element;
use servo_arc::Arc as ServoArc; use servo_arc::Arc as ServoArc;
@ -241,6 +241,9 @@ pub struct LayoutThread {
/// The time a layout query has waited before serviced by layout thread. /// The time a layout query has waited before serviced by layout thread.
layout_query_waiting_time: Histogram, layout_query_waiting_time: Histogram,
/// The sizes of all iframes encountered during the last layout operation.
last_iframe_sizes: RefCell<HashMap<BrowsingContextId, TypedSize2D<f32, CSSPixel>>>,
} }
impl LayoutThreadFactory for LayoutThread { impl LayoutThreadFactory for LayoutThread {
@ -544,6 +547,7 @@ impl LayoutThread {
}, },
paint_time_metrics: paint_time_metrics, paint_time_metrics: paint_time_metrics,
layout_query_waiting_time: Histogram::new(), layout_query_waiting_time: Histogram::new(),
last_iframe_sizes: Default::default(),
} }
} }
@ -1054,11 +1058,43 @@ impl LayoutThread {
// it with an empty vector // it with an empty vector
let iframe_sizes = let iframe_sizes =
std::mem::replace(&mut build_state.iframe_sizes, vec![]); std::mem::replace(&mut build_state.iframe_sizes, vec![]);
let msg = ConstellationMsg::IFrameSizes(iframe_sizes); // Collect the last frame's iframe sizes to compute any differences.
// Every frame starts with a fresh collection so that any removed
// iframes do not linger.
let last_iframe_sizes = std::mem::replace(
&mut *self.last_iframe_sizes.borrow_mut(),
HashMap::default(),
);
let mut size_messages = vec![];
for new_size in iframe_sizes {
// Only notify the constellation about existing iframes
// that have a new size, or iframes that did not previously
// exist.
if let Some(old_size) = last_iframe_sizes.get(&new_size.id) {
if *old_size != new_size.size {
size_messages.push(IFrameSizeMsg {
data: new_size,
type_: WindowSizeType::Resize,
});
}
} else {
size_messages.push(IFrameSizeMsg {
data: new_size,
type_: WindowSizeType::Initial,
});
}
self.last_iframe_sizes
.borrow_mut()
.insert(new_size.id, new_size.size);
}
if !size_messages.is_empty() {
let msg = ConstellationMsg::IFrameSizes(size_messages);
if let Err(e) = self.constellation_chan.send(msg) { if let Err(e) = self.constellation_chan.send(msg) {
warn!("Layout resize to constellation failed ({}).", e); warn!("Layout resize to constellation failed ({}).", e);
} }
} }
}
rw_data.indexable_text = std::mem::replace( rw_data.indexable_text = std::mem::replace(
&mut build_state.indexable_text, &mut build_state.indexable_text,

View file

@ -61,7 +61,9 @@ use webvr_traits::{WebVREvent, WebVRMsg};
pub use crate::script_msg::{ pub use crate::script_msg::{
DOMMessage, SWManagerMsg, SWManagerSenders, ScopeThings, ServiceWorkerMsg, DOMMessage, SWManagerMsg, SWManagerSenders, ScopeThings, ServiceWorkerMsg,
}; };
pub use crate::script_msg::{EventResult, LayoutMsg, LogEntry, ScriptMsg}; pub use crate::script_msg::{
EventResult, IFrameSize, IFrameSizeMsg, LayoutMsg, LogEntry, ScriptMsg,
};
/// The address of a node. Layout sends these back. They must be validated via /// The address of a node. Layout sends these back. They must be validated via
/// `from_untrusted_node_address` before they can be used, because we do not trust layout. /// `from_untrusted_node_address` before they can be used, because we do not trust layout.

View file

@ -9,6 +9,7 @@ use crate::IFrameLoadInfo;
use crate::IFrameLoadInfoWithData; use crate::IFrameLoadInfoWithData;
use crate::LayoutControlMsg; use crate::LayoutControlMsg;
use crate::LoadData; use crate::LoadData;
use crate::WindowSizeType;
use crate::WorkerGlobalScopeInit; use crate::WorkerGlobalScopeInit;
use crate::WorkerScriptLoadOrigin; use crate::WorkerScriptLoadOrigin;
use canvas_traits::canvas::{CanvasId, CanvasMsg}; use canvas_traits::canvas::{CanvasId, CanvasMsg};
@ -30,13 +31,31 @@ use style_traits::viewport::ViewportConstraints;
use style_traits::CSSPixel; use style_traits::CSSPixel;
use webrender_api::{DeviceIntPoint, DeviceIntSize}; use webrender_api::{DeviceIntPoint, DeviceIntSize};
/// A particular iframe's size, associated with a browsing context.
#[derive(Clone, Copy, Debug, Deserialize, Serialize)]
pub struct IFrameSize {
/// The child browsing context for this iframe.
pub id: BrowsingContextId,
/// The size of the iframe.
pub size: TypedSize2D<f32, CSSPixel>,
}
/// An iframe sizing operation.
#[derive(Clone, Copy, Debug, Deserialize, Serialize)]
pub struct IFrameSizeMsg {
/// The iframe sizing data.
pub data: IFrameSize,
/// The kind of sizing operation.
pub type_: WindowSizeType,
}
/// Messages from the layout to the constellation. /// Messages from the layout to the constellation.
#[derive(Deserialize, Serialize)] #[derive(Deserialize, Serialize)]
pub enum LayoutMsg { pub enum LayoutMsg {
/// Indicates whether this pipeline is currently running animations. /// Indicates whether this pipeline is currently running animations.
ChangeRunningAnimationsState(PipelineId, AnimationState), ChangeRunningAnimationsState(PipelineId, AnimationState),
/// Inform the constellation of the size of the iframe's viewport. /// Inform the constellation of the size of the iframe's viewport.
IFrameSizes(Vec<(BrowsingContextId, TypedSize2D<f32, CSSPixel>)>), IFrameSizes(Vec<IFrameSizeMsg>),
/// Requests that the constellation inform the compositor that it needs to record /// Requests that the constellation inform the compositor that it needs to record
/// the time when the frame with the given ID (epoch) is painted. /// the time when the frame with the given ID (epoch) is painted.
PendingPaintMetric(PipelineId, Epoch), PendingPaintMetric(PipelineId, Epoch),