auto merge of #5217 : zslayton/servo/master, r=jdm

@jdm This initial version has a few outstanding issues that I wanted to invite input on. Specifically:

1. I had some difficulty finding a home for the `StorageType` enum. Structs defined outside of the `script` module don't seem to be able to use the `#[jstraceable]` annotation and the `net` module (where `StorageTask` lives) doesn't have access to `script`. Per Simon Sapin's suggestion, I worked around this temporarily by creating a `TraceableStorageType` stand-in struct that was traceable and which could be translated into a regular `StorageType` when being sent to the `StorageTask`. Unsure of the best way to resolve this hack. Thoughts?

2. Apart from the `Storage` constructor used in `Window::SessionStorage` and the new `Window::LocalStorage`, there's also a method called `Storage::Constructor`. I'm unclear on what (if anything) will actually invoke this, so I'm not sure which variant of `StorageType` to use here. I've temporarily created an `Unknown` variant of `StorageType` as a placeholder.

3. I discovered that the web platform tests directory's localStorage tests. Many of them now pass despite the configured expectation that they fail. However, several do not pass. Is there a good way for me to add debug logging or otherwise get a sense of which assertion failed / what went wrong?

Thanks for your continued help!

components/script/dom/bindings/trace.rs

@@ -48,6 +48,7 @@ use layout_interface::{LayoutRPC, LayoutChan};
 use libc;
 use msg::constellation_msg::{PipelineId, SubpageId, WindowSizeData};
 use net::image_cache_task::ImageCacheTask;
+use net::storage_task::StorageType;
 use script_traits::ScriptControlChan;
 use script_traits::UntrustedNodeAddress;
 use msg::compositor_msg::ScriptListener;
@@ -231,6 +232,7 @@ no_jsmanaged_fields!(UntrustedNodeAddress);
 no_jsmanaged_fields!(LengthOrPercentageOrAuto);
 no_jsmanaged_fields!(RGBA);
 no_jsmanaged_fields!(Matrix2D<T>);
+no_jsmanaged_fields!(StorageType);
 
 impl JSTraceable for Box<ScriptChan+Send> {
     #[inline]

components/script/dom/storage.rs

@@ -7,9 +7,9 @@ use dom::bindings::codegen::Bindings::StorageBinding::StorageMethods;
 use dom::bindings::global::{GlobalRef, GlobalField};
 use dom::bindings::js::{JSRef, Temporary};
 use dom::bindings::utils::{Reflector, reflect_dom_object};
-use dom::bindings::error::Fallible;
 use util::str::DOMString;
 use net::storage_task::StorageTask;
+use net::storage_task::StorageType;
 use net::storage_task::StorageTaskMsg;
 use std::sync::mpsc::channel;
 use url::Url;
@@ -18,22 +18,20 @@ use url::Url;
 pub struct Storage {
     reflector_: Reflector,
     global: GlobalField,
+    storage_type: StorageType
 }
 
 impl Storage {
-    fn new_inherited(global: &GlobalRef) -> Storage {
+    fn new_inherited(global: &GlobalRef, storage_type: StorageType) -> Storage {
         Storage {
             reflector_: Reflector::new(),
             global: GlobalField::from_rooted(global),
+            storage_type: storage_type
         }
     }
 
-    pub fn new(global: &GlobalRef) -> Temporary<Storage> {
-        reflect_dom_object(box Storage::new_inherited(global), *global, StorageBinding::Wrap)
-    }
-
-    pub fn Constructor(global: &GlobalRef) -> Fallible<Temporary<Storage>> {
-        Ok(Storage::new(global))
+    pub fn new(global: &GlobalRef, storage_type: StorageType) -> Temporary<Storage> {
+        reflect_dom_object(box Storage::new_inherited(global, storage_type), *global, StorageBinding::Wrap)
     }
 
     fn get_url(&self) -> Url {
@@ -54,21 +52,21 @@ impl<'a> StorageMethods for JSRef<'a, Storage> {
     fn Length(self) -> u32 {
         let (sender, receiver) = channel();
 
-        self.get_storage_task().send(StorageTaskMsg::Length(sender, self.get_url())).unwrap();
+        self.get_storage_task().send(StorageTaskMsg::Length(sender, self.get_url(), self.storage_type)).unwrap();
         receiver.recv().unwrap()
     }
 
     fn Key(self, index: u32) -> Option<DOMString> {
         let (sender, receiver) = channel();
 
-        self.get_storage_task().send(StorageTaskMsg::Key(sender, self.get_url(), index)).unwrap();
+        self.get_storage_task().send(StorageTaskMsg::Key(sender, self.get_url(), self.storage_type, index)).unwrap();
         receiver.recv().unwrap()
     }
 
     fn GetItem(self, name: DOMString) -> Option<DOMString> {
         let (sender, receiver) = channel();
 
-        self.get_storage_task().send(StorageTaskMsg::GetItem(sender, self.get_url(), name)).unwrap();
+        self.get_storage_task().send(StorageTaskMsg::GetItem(sender, self.get_url(), self.storage_type, name)).unwrap();
         receiver.recv().unwrap()
     }
 
@@ -81,7 +79,7 @@ impl<'a> StorageMethods for JSRef<'a, Storage> {
     fn SetItem(self, name: DOMString, value: DOMString) {
         let (sender, receiver) = channel();
 
-        self.get_storage_task().send(StorageTaskMsg::SetItem(sender, self.get_url(), name, value)).unwrap();
+        self.get_storage_task().send(StorageTaskMsg::SetItem(sender, self.get_url(), self.storage_type, name, value)).unwrap();
         if receiver.recv().unwrap() {
             //TODO send notification
         }
@@ -98,7 +96,7 @@ impl<'a> StorageMethods for JSRef<'a, Storage> {
     fn RemoveItem(self, name: DOMString) {
         let (sender, receiver) = channel();
 
-        self.get_storage_task().send(StorageTaskMsg::RemoveItem(sender, self.get_url(), name)).unwrap();
+        self.get_storage_task().send(StorageTaskMsg::RemoveItem(sender, self.get_url(), self.storage_type, name)).unwrap();
         if receiver.recv().unwrap() {
             //TODO send notification
         }
@@ -111,10 +109,9 @@ impl<'a> StorageMethods for JSRef<'a, Storage> {
     fn Clear(self) {
         let (sender, receiver) = channel();
 
-        self.get_storage_task().send(StorageTaskMsg::Clear(sender, self.get_url())).unwrap();
+        self.get_storage_task().send(StorageTaskMsg::Clear(sender, self.get_url(), self.storage_type)).unwrap();
         if receiver.recv().unwrap() {
             //TODO send notification
         }
     }
 }
-

components/script/dom/webidls/Window.webidl

@@ -134,3 +134,10 @@ interface WindowSessionStorage {
   readonly attribute Storage sessionStorage;
 };
 Window implements WindowSessionStorage;
+
+// https://html.spec.whatwg.org/multipage/webstorage.html#dom-localstorage
+[NoInterfaceObject]
+interface WindowLocalStorage {
+  readonly attribute Storage localStorage;
+};
+Window implements WindowLocalStorage;

components/script/dom/window.rs

@@ -39,7 +39,7 @@ use msg::compositor_msg::ScriptListener;
 use msg::constellation_msg::{LoadData, PipelineId, SubpageId, ConstellationChan, WindowSizeData};
 use net::image_cache_task::ImageCacheTask;
 use net::resource_task::ResourceTask;
-use net::storage_task::StorageTask;
+use net::storage_task::{StorageTask, StorageType};
 use util::geometry::{self, Au, MAX_RECT};
 use util::opts;
 use util::str::{DOMString,HTML_SPACE_CHARACTERS};
@@ -95,6 +95,7 @@ pub struct Window {
     navigation_start_precise: f64,
     screen: MutNullableJS<Screen>,
     session_storage: MutNullableJS<Storage>,
+    local_storage: MutNullableJS<Storage>,
     timers: TimerManager,
 
     /// For providing instructions to an optional devtools server.
@@ -288,7 +289,11 @@ impl<'a> WindowMethods for JSRef<'a, Window> {
     }
 
     fn SessionStorage(self) -> Temporary<Storage> {
-        self.session_storage.or_init(|| Storage::new(&GlobalRef::Window(self)))
+        self.session_storage.or_init(|| Storage::new(&GlobalRef::Window(self), StorageType::Session))
+    }
+
+    fn LocalStorage(self) -> Temporary<Storage> {
+        self.local_storage.or_init(|| Storage::new(&GlobalRef::Window(self), StorageType::Local))
     }
 
     fn Console(self) -> Temporary<Console> {
@@ -775,6 +780,7 @@ impl Window {
             navigation_start_precise: time::precise_time_ns() as f64,
             screen: Default::default(),
             session_storage: Default::default(),
+            local_storage: Default::default(),
             timers: TimerManager::new(),
             id: id,
             subpage_id: subpage_id,