storage: Isolate sessionStorage per top-level browsing context and copy sessionStorage when creating a new auxiliary browsing context (#37803)

This pull request introduces changes to the storage subsystem to:
- Isolate sessionStorage per top-level browsing context (WebViewId), in
  addition to origin.
- Copy sessionStorage when creating a new auxiliary browsing context
without
  noopener, as required by the corresponding spec
 
These changes bring Servo closer to spec compliance, matching expected
browser
behavior.

Testing: This work affects observable behavior. As a result, some
previously
failing WPT tests now pass. No new tests are added, since the behavior
is
already covered by existing web-platform-tests.

Fixes: #21291

---------

Signed-off-by: Jan Varga <jan.varga@gmail.com>

components/net/storage_thread.rs

@@ -7,6 +7,7 @@ use std::collections::{BTreeMap, HashMap};
 use std::path::PathBuf;
 use std::thread;
 
+use base::id::WebViewId;
 use ipc_channel::ipc::{self, IpcReceiver, IpcSender};
 use malloc_size_of::MallocSizeOf;
 use net_traits::storage_thread::{StorageThreadMsg, StorageType};
@@ -47,10 +48,12 @@ impl StorageThreadFactory for IpcSender<StorageThreadMsg> {
     }
 }
 
+type OriginEntry = (usize, BTreeMap<String, String>);
+
 struct StorageManager {
     port: IpcReceiver<StorageThreadMsg>,
-    session_data: HashMap<String, (usize, BTreeMap<String, String>)>,
+    session_data: HashMap<WebViewId, HashMap<String, OriginEntry>>,
-    local_data: HashMap<String, (usize, BTreeMap<String, String>)>,
+    local_data: HashMap<String, OriginEntry>,
     config_dir: Option<PathBuf>,
 }
 
@@ -73,30 +76,38 @@ impl StorageManager {
     fn start(&mut self) {
         loop {
             match self.port.recv().unwrap() {
-                StorageThreadMsg::Length(sender, url, storage_type) => {
+                StorageThreadMsg::Length(sender, storage_type, webview_id, url) => {
-                    self.length(sender, url, storage_type)
+                    self.length(sender, storage_type, webview_id, url)
                 },
-                StorageThreadMsg::Key(sender, url, storage_type, index) => {
+                StorageThreadMsg::Key(sender, storage_type, webview_id, url, index) => {
-                    self.key(sender, url, storage_type, index)
+                    self.key(sender, storage_type, webview_id, url, index)
                 },
-                StorageThreadMsg::Keys(sender, url, storage_type) => {
+                StorageThreadMsg::Keys(sender, storage_type, webview_id, url) => {
-                    self.keys(sender, url, storage_type)
+                    self.keys(sender, storage_type, webview_id, url)
                 },
-                StorageThreadMsg::SetItem(sender, url, storage_type, name, value) => {
+                StorageThreadMsg::SetItem(sender, storage_type, webview_id, url, name, value) => {
-                    self.set_item(sender, url, storage_type, name, value);
+                    self.set_item(sender, storage_type, webview_id, url, name, value);
                     self.save_state()
                 },
-                StorageThreadMsg::GetItem(sender, url, storage_type, name) => {
+                StorageThreadMsg::GetItem(sender, storage_type, webview_id, url, name) => {
-                    self.request_item(sender, url, storage_type, name)
+                    self.request_item(sender, storage_type, webview_id, url, name)
                 },
-                StorageThreadMsg::RemoveItem(sender, url, storage_type, name) => {
+                StorageThreadMsg::RemoveItem(sender, storage_type, webview_id, url, name) => {
-                    self.remove_item(sender, url, storage_type, name);
+                    self.remove_item(sender, storage_type, webview_id, url, name);
                     self.save_state()
                 },
-                StorageThreadMsg::Clear(sender, url, storage_type) => {
+                StorageThreadMsg::Clear(sender, storage_type, webview_id, url) => {
-                    self.clear(sender, url, storage_type);
+                    self.clear(sender, storage_type, webview_id, url);
                     self.save_state()
                 },
+                StorageThreadMsg::Clone {
+                    sender,
+                    src: src_webview_id,
+                    dest: dest_webview_id,
+                } => {
+                    self.clone(src_webview_id, dest_webview_id);
+                    let _ = sender.send(());
+                },
                 StorageThreadMsg::CollectMemoryReport(sender) => {
                     let reports = self.collect_memory_reports();
                     sender.send(ProcessReports::new(reports));
@@ -137,53 +148,90 @@ impl StorageManager {
     fn select_data(
         &self,
         storage_type: StorageType,
-    ) -> &HashMap<String, (usize, BTreeMap<String, String>)> {
+        webview_id: WebViewId,
+        origin: &str,
+    ) -> Option<&OriginEntry> {
         match storage_type {
-            StorageType::Session => &self.session_data,
+            StorageType::Session => self
-            StorageType::Local => &self.local_data,
+                .session_data
+                .get(&webview_id)
+                .and_then(|origin_map| origin_map.get(origin)),
+            StorageType::Local => self.local_data.get(origin),
         }
     }
 
     fn select_data_mut(
         &mut self,
         storage_type: StorageType,
-    ) -> &mut HashMap<String, (usize, BTreeMap<String, String>)> {
+        webview_id: WebViewId,
+        origin: &str,
+    ) -> Option<&mut OriginEntry> {
         match storage_type {
-            StorageType::Session => &mut self.session_data,
+            StorageType::Session => self
-            StorageType::Local => &mut self.local_data,
+                .session_data
+                .get_mut(&webview_id)
+                .and_then(|origin_map| origin_map.get_mut(origin)),
+            StorageType::Local => self.local_data.get_mut(origin),
         }
     }
 
-    fn length(&self, sender: IpcSender<usize>, url: ServoUrl, storage_type: StorageType) {
+    fn ensure_data_mut(
+        &mut self,
+        storage_type: StorageType,
+        webview_id: WebViewId,
+        origin: &str,
+    ) -> &mut OriginEntry {
+        match storage_type {
+            StorageType::Session => self
+                .session_data
+                .entry(webview_id)
+                .or_default()
+                .entry(origin.to_string())
+                .or_default(),
+            StorageType::Local => self.local_data.entry(origin.to_string()).or_default(),
+        }
+    }
+
+    fn length(
+        &self,
+        sender: IpcSender<usize>,
+        storage_type: StorageType,
+        webview_id: WebViewId,
+        url: ServoUrl,
+    ) {
         let origin = self.origin_as_string(url);
-        let data = self.select_data(storage_type);
+        let data = self.select_data(storage_type, webview_id, &origin);
         sender
-            .send(data.get(&origin).map_or(0, |(_, entry)| entry.len()))
+            .send(data.map_or(0, |(_, entry)| entry.len()))
             .unwrap();
     }
 
     fn key(
         &self,
         sender: IpcSender<Option<String>>,
-        url: ServoUrl,
         storage_type: StorageType,
+        webview_id: WebViewId,
+        url: ServoUrl,
         index: u32,
     ) {
         let origin = self.origin_as_string(url);
-        let data = self.select_data(storage_type);
+        let data = self.select_data(storage_type, webview_id, &origin);
         let key = data
-            .get(&origin)
             .and_then(|(_, entry)| entry.keys().nth(index as usize))
             .cloned();
         sender.send(key).unwrap();
     }
 
-    fn keys(&self, sender: IpcSender<Vec<String>>, url: ServoUrl, storage_type: StorageType) {
+    fn keys(
+        &self,
+        sender: IpcSender<Vec<String>>,
+        storage_type: StorageType,
+        webview_id: WebViewId,
+        url: ServoUrl,
+    ) {
         let origin = self.origin_as_string(url);
-        let data = self.select_data(storage_type);
+        let data = self.select_data(storage_type, webview_id, &origin);
-        let keys = data
+        let keys = data.map_or(vec![], |(_, entry)| entry.keys().cloned().collect());
-            .get(&origin)
-            .map_or(vec![], |(_, entry)| entry.keys().cloned().collect());
 
         sender.send(keys).unwrap();
     }
@@ -195,32 +243,28 @@ impl StorageManager {
     fn set_item(
         &mut self,
         sender: IpcSender<Result<(bool, Option<String>), ()>>,
-        url: ServoUrl,
         storage_type: StorageType,
+        webview_id: WebViewId,
+        url: ServoUrl,
         name: String,
         value: String,
     ) {
         let origin = self.origin_as_string(url);
 
         let (this_storage_size, other_storage_size) = {
-            let local_data = self.select_data(StorageType::Local);
+            let local_data = self.select_data(StorageType::Local, webview_id, &origin);
-            let session_data = self.select_data(StorageType::Session);
+            let session_data = self.select_data(StorageType::Session, webview_id, &origin);
-            let local_data_size = local_data.get(&origin).map_or(0, |&(total, _)| total);
+            let local_data_size = local_data.map_or(0, |&(total, _)| total);
-            let session_data_size = session_data.get(&origin).map_or(0, |&(total, _)| total);
+            let session_data_size = session_data.map_or(0, |&(total, _)| total);
             match storage_type {
                 StorageType::Local => (local_data_size, session_data_size),
                 StorageType::Session => (session_data_size, local_data_size),
             }
         };
 
-        let data = self.select_data_mut(storage_type);
+        let &mut (ref mut total, ref mut entry) =
-        if !data.contains_key(&origin) {
+            self.ensure_data_mut(storage_type, webview_id, &origin);
-            data.insert(origin.clone(), (0, BTreeMap::new()));
-        }
 
-        let message = data
-            .get_mut(&origin)
-            .map(|&mut (ref mut total, ref mut entry)| {
         let mut new_total_size = this_storage_size + value.len();
         if let Some(old_value) = entry.get(&name) {
             new_total_size -= old_value.len();
@@ -228,11 +272,10 @@ impl StorageManager {
             new_total_size += name.len();
         }
 
-                if (new_total_size + other_storage_size) > QUOTA_SIZE_LIMIT {
+        let message = if (new_total_size + other_storage_size) > QUOTA_SIZE_LIMIT {
-                    return Err(());
+            Err(())
-                }
+        } else {
-
+            *total = new_total_size;
-                let message =
             entry
                 .insert(name.clone(), value.clone())
                 .map_or(Ok((true, None)), |old| {
@@ -241,29 +284,23 @@ impl StorageManager {
                     } else {
                         Ok((true, Some(old)))
                     }
-                        });
-                *total = new_total_size;
-                message
                 })
-            .unwrap();
+        };
         sender.send(message).unwrap();
     }
 
     fn request_item(
         &self,
         sender: IpcSender<Option<String>>,
-        url: ServoUrl,
         storage_type: StorageType,
+        webview_id: WebViewId,
+        url: ServoUrl,
         name: String,
     ) {
         let origin = self.origin_as_string(url);
-        let data = self.select_data(storage_type);
+        let data = self.select_data(storage_type, webview_id, &origin);
         sender
-            .send(
+            .send(data.and_then(|(_, entry)| entry.get(&name)).cloned())
-                data.get(&origin)
-                    .and_then(|(_, entry)| entry.get(&name))
-                    .cloned(),
-            )
             .unwrap();
     }
 
@@ -271,15 +308,14 @@ impl StorageManager {
     fn remove_item(
         &mut self,
         sender: IpcSender<Option<String>>,
-        url: ServoUrl,
         storage_type: StorageType,
+        webview_id: WebViewId,
+        url: ServoUrl,
         name: String,
     ) {
         let origin = self.origin_as_string(url);
-        let data = self.select_data_mut(storage_type);
+        let data = self.select_data_mut(storage_type, webview_id, &origin);
-        let old_value = data
+        let old_value = data.and_then(|&mut (ref mut total, ref mut entry)| {
-            .get_mut(&origin)
-            .and_then(|&mut (ref mut total, ref mut entry)| {
             entry.remove(&name).inspect(|old| {
                 *total -= name.len() + old.len();
             })
@@ -287,13 +323,17 @@ impl StorageManager {
         sender.send(old_value).unwrap();
     }
 
-    fn clear(&mut self, sender: IpcSender<bool>, url: ServoUrl, storage_type: StorageType) {
+    fn clear(
+        &mut self,
+        sender: IpcSender<bool>,
+        storage_type: StorageType,
+        webview_id: WebViewId,
+        url: ServoUrl,
+    ) {
         let origin = self.origin_as_string(url);
-        let data = self.select_data_mut(storage_type);
+        let data = self.select_data_mut(storage_type, webview_id, &origin);
         sender
-            .send(
+            .send(data.is_some_and(|&mut (ref mut total, ref mut entry)| {
-                data.get_mut(&origin)
-                    .is_some_and(|&mut (ref mut total, ref mut entry)| {
                 if !entry.is_empty() {
                     entry.clear();
                     *total = 0;
@@ -301,11 +341,20 @@ impl StorageManager {
                 } else {
                     false
                 }
-                    }),
+            }))
-            )
             .unwrap();
     }
 
+    fn clone(&mut self, src_webview_id: WebViewId, dest_webview_id: WebViewId) {
+        let Some(src_origin_entries) = self.session_data.get(&src_webview_id) else {
+            return;
+        };
+
+        let dest_origin_entries = src_origin_entries.clone();
+        self.session_data
+            .insert(dest_webview_id, dest_origin_entries);
+    }
+
     fn origin_as_string(&self, url: ServoUrl) -> String {
         url.origin().ascii_serialization()
     }

components/script/dom/storage.rs

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
+use base::id::WebViewId;
 use constellation_traits::ScriptToConstellationMessage;
 use dom_struct::dom_struct;
 use ipc_channel::ipc::IpcSender;
@@ -49,6 +50,10 @@ impl Storage {
         )
     }
 
+    fn webview_id(&self) -> WebViewId {
+        self.global().as_window().window_proxy().webview_id()
+    }
+
     fn get_url(&self) -> ServoUrl {
         self.global().get_url()
     }
@@ -66,8 +71,9 @@ impl StorageMethods<crate::DomTypeHolder> for Storage {
         self.get_storage_thread()
             .send(StorageThreadMsg::Length(
                 sender,
-                self.get_url(),
                 self.storage_type,
+                self.webview_id(),
+                self.get_url(),
             ))
             .unwrap();
         receiver.recv().unwrap() as u32
@@ -80,8 +86,9 @@ impl StorageMethods<crate::DomTypeHolder> for Storage {
         self.get_storage_thread()
             .send(StorageThreadMsg::Key(
                 sender,
-                self.get_url(),
                 self.storage_type,
+                self.webview_id(),
+                self.get_url(),
                 index,
             ))
             .unwrap();
@@ -93,7 +100,13 @@ impl StorageMethods<crate::DomTypeHolder> for Storage {
         let (sender, receiver) = ipc::channel(self.global().time_profiler_chan().clone()).unwrap();
         let name = String::from(name);
 
-        let msg = StorageThreadMsg::GetItem(sender, self.get_url(), self.storage_type, name);
+        let msg = StorageThreadMsg::GetItem(
+            sender,
+            self.storage_type,
+            self.webview_id(),
+            self.get_url(),
+            name,
+        );
         self.get_storage_thread().send(msg).unwrap();
         receiver.recv().unwrap().map(DOMString::from)
     }
@@ -106,8 +119,9 @@ impl StorageMethods<crate::DomTypeHolder> for Storage {
 
         let msg = StorageThreadMsg::SetItem(
             sender,
-            self.get_url(),
             self.storage_type,
+            self.webview_id(),
+            self.get_url(),
             name.clone(),
             value.clone(),
         );
@@ -128,8 +142,13 @@ impl StorageMethods<crate::DomTypeHolder> for Storage {
         let (sender, receiver) = ipc::channel(self.global().time_profiler_chan().clone()).unwrap();
         let name = String::from(name);
 
-        let msg =
+        let msg = StorageThreadMsg::RemoveItem(
-            StorageThreadMsg::RemoveItem(sender, self.get_url(), self.storage_type, name.clone());
+            sender,
+            self.storage_type,
+            self.webview_id(),
+            self.get_url(),
+            name.clone(),
+        );
         self.get_storage_thread().send(msg).unwrap();
         if let Some(old_value) = receiver.recv().unwrap() {
             self.broadcast_change_notification(Some(name), Some(old_value), None);
@@ -143,8 +162,9 @@ impl StorageMethods<crate::DomTypeHolder> for Storage {
         self.get_storage_thread()
             .send(StorageThreadMsg::Clear(
                 sender,
-                self.get_url(),
                 self.storage_type,
+                self.webview_id(),
+                self.get_url(),
             ))
             .unwrap();
         if receiver.recv().unwrap() {
@@ -159,8 +179,9 @@ impl StorageMethods<crate::DomTypeHolder> for Storage {
         self.get_storage_thread()
             .send(StorageThreadMsg::Keys(
                 sender,
-                self.get_url(),
                 self.storage_type,
+                self.webview_id(),
+                self.get_url(),
             ))
             .unwrap();
         receiver

components/script/dom/windowproxy.rs

@@ -32,7 +32,9 @@ use js::jsval::{NullValue, PrivateValue, UndefinedValue};
 use js::rust::wrappers::{JS_TransplantObject, NewWindowProxy, SetWindowProxy};
 use js::rust::{Handle, MutableHandle, MutableHandleValue, get_object_class};
 use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
+use net_traits::IpcSend;
 use net_traits::request::Referrer;
+use net_traits::storage_thread::StorageThreadMsg;
 use script_traits::NewLayoutInfo;
 use serde::{Deserialize, Serialize};
 use servo_url::{ImmutableOrigin, ServoUrl};
@@ -334,8 +336,6 @@ impl WindowProxy {
             theme: window.theme(),
         };
         ScriptThread::process_attach_layout(new_layout_info, document.origin().clone());
-        // TODO: if noopener is false, copy the sessionStorage storage area of the creator origin.
-        // See step 14 of https://html.spec.whatwg.org/multipage/#creating-a-new-browsing-context
         let new_window_proxy = ScriptThread::find_document(response.new_pipeline_id)
             .and_then(|doc| doc.browsing_context())?;
         if name.to_lowercase() != "_blank" {
@@ -343,6 +343,26 @@ impl WindowProxy {
         }
         if noopener {
             new_window_proxy.disown();
+        } else {
+            // After creating a new auxiliary browsing context and document,
+            // the session storage is copied over.
+            // See https://html.spec.whatwg.org/multipage/#the-sessionstorage-attribute
+
+            let (sender, receiver) = ipc::channel().unwrap();
+
+            let msg = StorageThreadMsg::Clone {
+                sender,
+                src: window.window_proxy().webview_id(),
+                dest: response.new_webview_id,
+            };
+
+            document
+                .global()
+                .resource_threads()
+                .sender()
+                .send(msg)
+                .unwrap();
+            receiver.recv().unwrap();
         }
         Some(new_window_proxy)
     }

components/shared/net/storage_thread.rs

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
+use base::id::WebViewId;
 use ipc_channel::ipc::IpcSender;
 use malloc_size_of_derive::MallocSizeOf;
 use profile_traits::mem::ReportsChan;
@@ -18,31 +19,58 @@ pub enum StorageType {
 #[derive(Debug, Deserialize, Serialize)]
 pub enum StorageThreadMsg {
     /// gets the number of key/value pairs present in the associated storage data
-    Length(IpcSender<usize>, ServoUrl, StorageType),
+    Length(IpcSender<usize>, StorageType, WebViewId, ServoUrl),
 
     /// gets the name of the key at the specified index in the associated storage data
-    Key(IpcSender<Option<String>>, ServoUrl, StorageType, u32),
+    Key(
+        IpcSender<Option<String>>,
+        StorageType,
+        WebViewId,
+        ServoUrl,
+        u32,
+    ),
 
     /// Gets the available keys in the associated storage data
-    Keys(IpcSender<Vec<String>>, ServoUrl, StorageType),
+    Keys(IpcSender<Vec<String>>, StorageType, WebViewId, ServoUrl),
 
     /// gets the value associated with the given key in the associated storage data
-    GetItem(IpcSender<Option<String>>, ServoUrl, StorageType, String),
+    GetItem(
+        IpcSender<Option<String>>,
+        StorageType,
+        WebViewId,
+        ServoUrl,
+        String,
+    ),
 
     /// sets the value of the given key in the associated storage data
     SetItem(
         IpcSender<Result<(bool, Option<String>), ()>>,
-        ServoUrl,
         StorageType,
+        WebViewId,
+        ServoUrl,
         String,
         String,
     ),
 
     /// removes the key/value pair for the given key in the associated storage data
-    RemoveItem(IpcSender<Option<String>>, ServoUrl, StorageType, String),
+    RemoveItem(
+        IpcSender<Option<String>>,
+        StorageType,
+        WebViewId,
+        ServoUrl,
+        String,
+    ),
 
     /// clears the associated storage data by removing all the key/value pairs
-    Clear(IpcSender<bool>, ServoUrl, StorageType),
+    Clear(IpcSender<bool>, StorageType, WebViewId, ServoUrl),
+
+    /// clones all storage data of the given top-level browsing context for a new browsing context.
+    /// should only be used for sessionStorage.
+    Clone {
+        sender: IpcSender<()>,
+        src: WebViewId,
+        dest: WebViewId,
+    },
 
     /// send a reply when done cleaning up thread resources and then shut it down
     Exit(IpcSender<()>),

tests/wpt/meta/webstorage/storage_session_window_noopener.window.js.ini

@@ -1,3 +0,0 @@
-[storage_session_window_noopener.window.html]
-  [A new noopener window to make sure there is a not copy of the previous window's sessionStorage]
-    expected: FAIL

tests/wpt/meta/webstorage/storage_session_window_open.window.js.ini

@@ -1,3 +0,0 @@
-[storage_session_window_open.window.html]
-  [A new window to make sure there is a copy of the previous window's sessionStorage, and that they diverge after a change]
-    expected: FAIL