From a5b43b7df10cc73d8203f7ee0c3a5e1a1bb1d9f5 Mon Sep 17 00:00:00 2001 From: Kunal Mohan Date: Sat, 15 Feb 2020 23:32:09 +0530 Subject: [PATCH 1/2] Take origin from window instead of creating a new one in case of reflow Everytime a new LayoutContext was created, it created a new origin which caused endless stream of image loads to occur in case of reflow. The reason for this was that the existing image, although cached successfully, was not used because the entry in hashmap did not match because of different(new) origin. This is solved by storing the origin of a window in enum ScriptReflow and used in creating new LayoutContext in case of reflow. --- components/layout/context.rs | 2 ++ components/layout_thread/lib.rs | 16 ++++++++++++---- components/script/dom/window.rs | 1 + components/script_layout_interface/lib.rs | 3 ++- components/script_layout_interface/message.rs | 4 +++- 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/components/layout/context.rs b/components/layout/context.rs index 90b427b0e15..1374ba175f9 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -140,6 +140,7 @@ impl<'a> LayoutContext<'a> { state: PendingImageState::Unrequested(url), node: node.to_untrusted_node_address(), id: id, + origin: self.origin.clone(), }; self.pending_images .as_ref() @@ -160,6 +161,7 @@ impl<'a> LayoutContext<'a> { state: PendingImageState::PendingResponse, node: node.to_untrusted_node_address(), id: id, + origin: self.origin.clone(), }; pending_images.lock().unwrap().push(image); } diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 10ff2f2569b..47b195c4884 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -93,7 +93,7 @@ use servo_atoms::Atom; use servo_config::opts; use servo_config::pref; use servo_geometry::MaxRect; -use servo_url::ServoUrl; +use servo_url::{ImmutableOrigin, ServoUrl}; use std::borrow::ToOwned; use std::cell::{Cell, RefCell}; use std::collections::HashMap; @@ -644,13 +644,18 @@ impl LayoutThread { guards: StylesheetGuards<'a>, script_initiated_layout: bool, snapshot_map: &'a SnapshotMap, + origin: Option, ) -> LayoutContext<'a> { let thread_local_style_context_creation_data = ThreadLocalStyleContextCreationInfo::new(self.new_animations_sender.clone()); LayoutContext { id: self.id, - origin: self.url.origin(), + origin: if let Some(origin) = origin { + origin + } else { + self.url.origin() + }, style_context: SharedStyleContext { stylist: &self.stylist, options: GLOBAL_STYLE_DATA.options.clone(), @@ -1341,6 +1346,8 @@ impl LayoutThread { Au::from_f32_px(initial_viewport.height), ); + let origin = data.origin.clone(); + // Calculate the actual viewport as per DEVICE-ADAPT § 6 // If the entire flow tree is invalid, then it will be reflowed anyhow. let document_shared_lock = document.style_shared_lock(); @@ -1482,7 +1489,8 @@ impl LayoutThread { self.stylist.flush(&guards, Some(element), Some(&map)); // Create a layout context for use throughout the following passes. - let mut layout_context = self.build_layout_context(guards.clone(), true, &map); + let mut layout_context = + self.build_layout_context(guards.clone(), true, &map, Some(origin)); let pool; let (thread_pool, num_threads) = if self.parallel_flag { @@ -1738,7 +1746,7 @@ impl LayoutThread { ua_or_user: &ua_or_user_guard, }; let snapshots = SnapshotMap::new(); - let mut layout_context = self.build_layout_context(guards, false, &snapshots); + let mut layout_context = self.build_layout_context(guards, false, &snapshots, None); let invalid_nodes = { // Perform an abbreviated style recalc that operates without access to the DOM. diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 65a60176559..1abd30ecc4a 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1622,6 +1622,7 @@ impl Window { document: self.Document().upcast::().to_trusted_node_address(), stylesheets_changed, window_size: self.window_size.get(), + origin: self.origin().immutable().clone(), reflow_goal, script_join_chan: join_chan, dom_count: self.Document().dom_count(), diff --git a/components/script_layout_interface/lib.rs b/components/script_layout_interface/lib.rs index ad333209033..ef9382378cb 100644 --- a/components/script_layout_interface/lib.rs +++ b/components/script_layout_interface/lib.rs @@ -23,7 +23,7 @@ use ipc_channel::ipc::IpcSender; use libc::c_void; use net_traits::image_cache::PendingImageId; use script_traits::UntrustedNodeAddress; -use servo_url::ServoUrl; +use servo_url::{ImmutableOrigin, ServoUrl}; use std::ptr::NonNull; use std::sync::atomic::AtomicIsize; use style::data::ElementData; @@ -138,6 +138,7 @@ pub struct PendingImage { pub state: PendingImageState, pub node: UntrustedNodeAddress, pub id: PendingImageId, + pub origin: ImmutableOrigin, } pub struct HTMLMediaData { diff --git a/components/script_layout_interface/message.rs b/components/script_layout_interface/message.rs index 9bfeddb2ea2..aeefadc7213 100644 --- a/components/script_layout_interface/message.rs +++ b/components/script_layout_interface/message.rs @@ -18,7 +18,7 @@ use script_traits::{ConstellationControlMsg, LayoutControlMsg, LayoutMsg as Cons use script_traits::{ScrollState, UntrustedNodeAddress, WindowSizeData}; use servo_arc::Arc as ServoArc; use servo_atoms::Atom; -use servo_url::ServoUrl; +use servo_url::{ImmutableOrigin, ServoUrl}; use std::sync::atomic::AtomicBool; use std::sync::Arc; use style::context::QuirksMode; @@ -216,6 +216,8 @@ pub struct ScriptReflow { pub reflow_goal: ReflowGoal, /// The number of objects in the dom #10110 pub dom_count: u32, + /// The current window origin + pub origin: ImmutableOrigin, } pub struct LayoutThreadInit { From 4a3bf52a7c408183d4c3eb7d29a6a528f180315d Mon Sep 17 00:00:00 2001 From: Kunal Mohan Date: Tue, 18 Feb 2020 00:57:33 +0530 Subject: [PATCH 2/2] remove option for origin and mirror changes to layout_thread_2020 --- components/constellation/constellation.rs | 16 ++++---- components/layout_2020/context.rs | 2 + components/layout_thread/lib.rs | 40 ++++++++++--------- components/layout_thread_2020/lib.rs | 38 +++++++++++------- components/script/dom/window.rs | 6 ++- components/script_layout_interface/message.rs | 4 +- components/script_traits/lib.rs | 2 +- 7 files changed, 62 insertions(+), 46 deletions(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 11dbd627847..4151c803599 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -3227,14 +3227,14 @@ where }, } }, - AnimationTickType::Layout => { - let msg = LayoutControlMsg::TickAnimations; - match self.pipelines.get(&pipeline_id) { - Some(pipeline) => pipeline.layout_chan.send(msg), - None => { - return warn!("Pipeline {:?} got layout tick after closure.", pipeline_id); - }, - } + AnimationTickType::Layout => match self.pipelines.get(&pipeline_id) { + Some(pipeline) => { + let msg = LayoutControlMsg::TickAnimations(pipeline.load_data.url.origin()); + pipeline.layout_chan.send(msg) + }, + None => { + return warn!("Pipeline {:?} got layout tick after closure.", pipeline_id); + }, }, }; if let Err(e) = result { diff --git a/components/layout_2020/context.rs b/components/layout_2020/context.rs index f642dab60e4..dc894e65766 100644 --- a/components/layout_2020/context.rs +++ b/components/layout_2020/context.rs @@ -88,6 +88,7 @@ impl<'a> LayoutContext<'a> { state: PendingImageState::Unrequested(url), node: node.into(), id: id, + origin: self.origin.clone(), }; self.pending_images .as_ref() @@ -108,6 +109,7 @@ impl<'a> LayoutContext<'a> { state: PendingImageState::PendingResponse, node: node.into(), id: id, + origin: self.origin.clone(), }; pending_images.lock().unwrap().push(image); } diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 47b195c4884..b5b45993c2b 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -644,18 +644,14 @@ impl LayoutThread { guards: StylesheetGuards<'a>, script_initiated_layout: bool, snapshot_map: &'a SnapshotMap, - origin: Option, + origin: ImmutableOrigin, ) -> LayoutContext<'a> { let thread_local_style_context_creation_data = ThreadLocalStyleContextCreationInfo::new(self.new_animations_sender.clone()); LayoutContext { id: self.id, - origin: if let Some(origin) = origin { - origin - } else { - self.url.origin() - }, + origin, style_context: SharedStyleContext { stylist: &self.stylist, options: GLOBAL_STYLE_DATA.options.clone(), @@ -693,7 +689,7 @@ impl LayoutThread { Msg::SetQuirksMode(..) => LayoutHangAnnotation::SetQuirksMode, Msg::Reflow(..) => LayoutHangAnnotation::Reflow, Msg::GetRPC(..) => LayoutHangAnnotation::GetRPC, - Msg::TickAnimations => LayoutHangAnnotation::TickAnimations, + Msg::TickAnimations(..) => LayoutHangAnnotation::TickAnimations, Msg::AdvanceClockMs(..) => LayoutHangAnnotation::AdvanceClockMs, Msg::ReapStyleAndLayoutData(..) => LayoutHangAnnotation::ReapStyleAndLayoutData, Msg::CollectReports(..) => LayoutHangAnnotation::CollectReports, @@ -739,8 +735,8 @@ impl LayoutThread { Msg::SetScrollStates(new_scroll_states), possibly_locked_rw_data, ), - Request::FromPipeline(LayoutControlMsg::TickAnimations) => { - self.handle_request_helper(Msg::TickAnimations, possibly_locked_rw_data) + Request::FromPipeline(LayoutControlMsg::TickAnimations(origin)) => { + self.handle_request_helper(Msg::TickAnimations(origin), possibly_locked_rw_data) }, Request::FromPipeline(LayoutControlMsg::GetCurrentEpoch(sender)) => { self.handle_request_helper(Msg::GetCurrentEpoch(sender), possibly_locked_rw_data) @@ -813,7 +809,9 @@ impl LayoutThread { || self.handle_reflow(&mut data, possibly_locked_rw_data), ); }, - Msg::TickAnimations => self.tick_all_animations(possibly_locked_rw_data), + Msg::TickAnimations(origin) => { + self.tick_all_animations(possibly_locked_rw_data, origin) + }, Msg::SetScrollStates(new_scroll_states) => { self.set_scroll_states(new_scroll_states, possibly_locked_rw_data); }, @@ -841,8 +839,8 @@ impl LayoutThread { let _rw_data = possibly_locked_rw_data.lock(); sender.send(self.epoch.get()).unwrap(); }, - Msg::AdvanceClockMs(how_many, do_tick) => { - self.handle_advance_clock_ms(how_many, possibly_locked_rw_data, do_tick); + Msg::AdvanceClockMs(how_many, do_tick, origin) => { + self.handle_advance_clock_ms(how_many, possibly_locked_rw_data, do_tick, origin); }, Msg::GetWebFontLoadState(sender) => { let _rw_data = possibly_locked_rw_data.lock(); @@ -1022,10 +1020,11 @@ impl LayoutThread { how_many_ms: i32, possibly_locked_rw_data: &mut RwData<'a, 'b>, tick_animations: bool, + origin: ImmutableOrigin, ) { self.timer.increment(how_many_ms as f64 / 1000.0); if tick_animations { - self.tick_all_animations(possibly_locked_rw_data); + self.tick_all_animations(possibly_locked_rw_data, origin); } } @@ -1489,8 +1488,7 @@ impl LayoutThread { self.stylist.flush(&guards, Some(element), Some(&map)); // Create a layout context for use throughout the following passes. - let mut layout_context = - self.build_layout_context(guards.clone(), true, &map, Some(origin)); + let mut layout_context = self.build_layout_context(guards.clone(), true, &map, origin); let pool; let (thread_pool, num_threads) = if self.parallel_flag { @@ -1718,12 +1716,16 @@ impl LayoutThread { rw_data.scroll_offsets = layout_scroll_states } - fn tick_all_animations<'a, 'b>(&mut self, possibly_locked_rw_data: &mut RwData<'a, 'b>) { + fn tick_all_animations<'a, 'b>( + &mut self, + possibly_locked_rw_data: &mut RwData<'a, 'b>, + origin: ImmutableOrigin, + ) { let mut rw_data = possibly_locked_rw_data.lock(); - self.tick_animations(&mut rw_data); + self.tick_animations(&mut rw_data, origin); } - fn tick_animations(&mut self, rw_data: &mut LayoutThreadData) { + fn tick_animations(&mut self, rw_data: &mut LayoutThreadData, origin: ImmutableOrigin) { if self.relayout_event { println!( "**** pipeline={}\tForDisplay\tSpecial\tAnimationTick", @@ -1746,7 +1748,7 @@ impl LayoutThread { ua_or_user: &ua_or_user_guard, }; let snapshots = SnapshotMap::new(); - let mut layout_context = self.build_layout_context(guards, false, &snapshots, None); + let mut layout_context = self.build_layout_context(guards, false, &snapshots, origin); let invalid_nodes = { // Perform an abbreviated style recalc that operates without access to the DOM. diff --git a/components/layout_thread_2020/lib.rs b/components/layout_thread_2020/lib.rs index 97d19762528..be24d768595 100644 --- a/components/layout_thread_2020/lib.rs +++ b/components/layout_thread_2020/lib.rs @@ -74,7 +74,7 @@ use servo_arc::Arc as ServoArc; use servo_atoms::Atom; use servo_config::opts; use servo_config::pref; -use servo_url::ServoUrl; +use servo_url::{ImmutableOrigin, ServoUrl}; use std::cell::{Cell, RefCell}; use std::collections::HashMap; use std::ops::{Deref, DerefMut}; @@ -589,13 +589,14 @@ impl LayoutThread { guards: StylesheetGuards<'a>, script_initiated_layout: bool, snapshot_map: &'a SnapshotMap, + origin: ImmutableOrigin, ) -> LayoutContext<'a> { let thread_local_style_context_creation_data = ThreadLocalStyleContextCreationInfo::new(self.new_animations_sender.clone()); LayoutContext { id: self.id, - origin: self.url.origin(), + origin, style_context: SharedStyleContext { stylist: &self.stylist, options: GLOBAL_STYLE_DATA.options.clone(), @@ -628,7 +629,7 @@ impl LayoutThread { Msg::SetQuirksMode(..) => LayoutHangAnnotation::SetQuirksMode, Msg::Reflow(..) => LayoutHangAnnotation::Reflow, Msg::GetRPC(..) => LayoutHangAnnotation::GetRPC, - Msg::TickAnimations => LayoutHangAnnotation::TickAnimations, + Msg::TickAnimations(..) => LayoutHangAnnotation::TickAnimations, Msg::AdvanceClockMs(..) => LayoutHangAnnotation::AdvanceClockMs, Msg::ReapStyleAndLayoutData(..) => LayoutHangAnnotation::ReapStyleAndLayoutData, Msg::CollectReports(..) => LayoutHangAnnotation::CollectReports, @@ -674,8 +675,8 @@ impl LayoutThread { Msg::SetScrollStates(new_scroll_states), possibly_locked_rw_data, ), - Request::FromPipeline(LayoutControlMsg::TickAnimations) => { - self.handle_request_helper(Msg::TickAnimations, possibly_locked_rw_data) + Request::FromPipeline(LayoutControlMsg::TickAnimations(origin)) => { + self.handle_request_helper(Msg::TickAnimations(origin), possibly_locked_rw_data) }, Request::FromPipeline(LayoutControlMsg::GetCurrentEpoch(sender)) => { self.handle_request_helper(Msg::GetCurrentEpoch(sender), possibly_locked_rw_data) @@ -748,7 +749,7 @@ impl LayoutThread { || self.handle_reflow(&mut data, possibly_locked_rw_data), ); }, - Msg::TickAnimations => self.tick_all_animations(), + Msg::TickAnimations(origin) => self.tick_all_animations(origin), Msg::SetScrollStates(new_scroll_states) => { self.set_scroll_states(new_scroll_states, possibly_locked_rw_data); }, @@ -776,8 +777,8 @@ impl LayoutThread { let _rw_data = possibly_locked_rw_data.lock(); sender.send(self.epoch.get()).unwrap(); }, - Msg::AdvanceClockMs(how_many, do_tick) => { - self.handle_advance_clock_ms(how_many, do_tick); + Msg::AdvanceClockMs(how_many, do_tick, origin) => { + self.handle_advance_clock_ms(how_many, do_tick, origin); }, Msg::GetWebFontLoadState(sender) => { let _rw_data = possibly_locked_rw_data.lock(); @@ -914,10 +915,15 @@ impl LayoutThread { } /// Advances the animation clock of the document. - fn handle_advance_clock_ms<'a, 'b>(&mut self, how_many_ms: i32, tick_animations: bool) { + fn handle_advance_clock_ms<'a, 'b>( + &mut self, + how_many_ms: i32, + tick_animations: bool, + origin: ImmutableOrigin, + ) { self.timer.increment(how_many_ms as f64 / 1000.0); if tick_animations { - self.tick_all_animations(); + self.tick_all_animations(origin); } } @@ -995,6 +1001,8 @@ impl LayoutThread { Au::from_f32_px(initial_viewport.height), ); + let origin = data.origin.clone(); + // Calculate the actual viewport as per DEVICE-ADAPT § 6 // If the entire flow tree is invalid, then it will be reflowed anyhow. let document_shared_lock = document.style_shared_lock(); @@ -1117,7 +1125,7 @@ impl LayoutThread { self.stylist.flush(&guards, Some(element), Some(&map)); // Create a layout context for use throughout the following passes. - let mut layout_context = self.build_layout_context(guards.clone(), true, &map); + let mut layout_context = self.build_layout_context(guards.clone(), true, &map, origin); let traversal = RecalcStyle::new(layout_context); let token = { @@ -1324,11 +1332,11 @@ impl LayoutThread { rw_data.scroll_offsets = layout_scroll_states } - fn tick_all_animations<'a, 'b>(&mut self) { - self.tick_animations(); + fn tick_all_animations<'a, 'b>(&mut self, origin: ImmutableOrigin) { + self.tick_animations(origin); } - fn tick_animations(&mut self) { + fn tick_animations(&mut self, origin: ImmutableOrigin) { if self.relayout_event { println!( "**** pipeline={}\tForDisplay\tSpecial\tAnimationTick", @@ -1347,7 +1355,7 @@ impl LayoutThread { ua_or_user: &ua_or_user_guard, }; let snapshots = SnapshotMap::new(); - let mut layout_context = self.build_layout_context(guards, false, &snapshots); + let mut layout_context = self.build_layout_context(guards, false, &snapshots, origin); self.perform_post_style_recalc_layout_passes( root, diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 1abd30ecc4a..227217a2734 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1548,7 +1548,11 @@ impl Window { /// forces a reflow if `tick` is true. pub fn advance_animation_clock(&self, delta: i32, tick: bool) { self.layout_chan - .send(Msg::AdvanceClockMs(delta, tick)) + .send(Msg::AdvanceClockMs( + delta, + tick, + self.origin().immutable().clone(), + )) .unwrap(); } diff --git a/components/script_layout_interface/message.rs b/components/script_layout_interface/message.rs index aeefadc7213..9e52083c9d4 100644 --- a/components/script_layout_interface/message.rs +++ b/components/script_layout_interface/message.rs @@ -47,13 +47,13 @@ pub enum Msg { GetRPC(Sender>), /// Requests that the layout thread render the next frame of all animations. - TickAnimations, + TickAnimations(ImmutableOrigin), /// Updates layout's timer for animation testing from script. /// /// The inner field is the number of *milliseconds* to advance, and the bool /// field is whether animations should be force-ticked. - AdvanceClockMs(i32, bool), + AdvanceClockMs(i32, bool, ImmutableOrigin), /// Destroys layout data associated with a DOM node. /// diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs index b85dffd790c..7f6ecece10b 100644 --- a/components/script_traits/lib.rs +++ b/components/script_traits/lib.rs @@ -124,7 +124,7 @@ pub enum LayoutControlMsg { /// Requests the current epoch (layout counter) from this layout. GetCurrentEpoch(IpcSender), /// Asks layout to run another step in its animation. - TickAnimations, + TickAnimations(ImmutableOrigin), /// Tells layout about the new scrolling offsets of each scrollable stacking context. SetScrollStates(Vec), /// Requests the current load state of Web fonts. `true` is returned if fonts are still loading