Auto merge of #25777 - kunalmohan:24720-ImageCache, r=jdm

Take origin from current window instead of creating a new one in event 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.

<!-- Please describe your changes on the following line: -->
r?@jdm

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #24720  (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

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 {

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);
                 }

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);
                 }

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,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(),
@@ -688,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,
@@ -734,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)
@@ -808,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);
             },
@@ -836,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();
@@ -1017,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);
         }
     }
 
@@ -1341,6 +1345,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 +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);
+        let mut layout_context = self.build_layout_context(guards.clone(), true, &map, origin);
 
         let pool;
         let (thread_pool, num_threads) = if self.parallel_flag {
@@ -1710,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",
@@ -1738,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);
+            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.

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,

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();
     }
 
@@ -1622,6 +1626,7 @@ impl Window {
             document: self.Document().upcast::<Node>().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(),

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 {

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;
@@ -47,13 +47,13 @@ pub enum Msg {
     GetRPC(Sender<Box<dyn LayoutRPC + Send>>),
 
     /// 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.
     ///
@@ -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 {

components/script_traits/lib.rs

@@ -124,7 +124,7 @@ pub enum LayoutControlMsg {
     /// Requests the current epoch (layout counter) from this layout.
     GetCurrentEpoch(IpcSender<Epoch>),
     /// 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<ScrollState>),
     /// Requests the current load state of Web fonts. `true` is returned if fonts are still loading