servo: Merge #18578 - Window should own Location, Document shouldn't (from asajeffrey:script-window-owns-location); r=KiChjang
authorAlan Jeffrey <ajeffrey@mozilla.com>
Wed, 20 Sep 2017 14:38:31 -0500
changeset 431601 6a151f43cb3f70917bde1ace58a9d118776e70b9
parent 431600 e740ebb21e460f2882d72829258fdb0e9075e67b
child 431602 40af6e459538e579da800aebb6520cdaa9b30e40
push id7785
push userryanvm@gmail.com
push dateThu, 21 Sep 2017 13:39:55 +0000
treeherdermozilla-beta@06d4034a8a03 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersKiChjang
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #18578 - Window should own Location, Document shouldn't (from asajeffrey:script-window-owns-location); r=KiChjang <!-- Please describe your changes on the following line: --> Document shouldn't own location, Window should. --- <!-- 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 #18438 - [X] These changes do not require tests because it's an intermittent <!-- 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. --> Source-Repo: https://github.com/servo/servo Source-Revision: 6a791cd7f26b42a6688099bea203c21fb3c9cc12
servo/components/script/dom/document.rs
servo/components/script/dom/window.rs
--- a/servo/components/script/dom/document.rs
+++ b/servo/components/script/dom/document.rs
@@ -221,17 +221,16 @@ impl ::style::stylesheets::StylesheetInD
 }
 
 /// https://dom.spec.whatwg.org/#document
 #[dom_struct]
 pub struct Document {
     node: Node,
     window: JS<Window>,
     implementation: MutNullableJS<DOMImplementation>,
-    location: MutNullableJS<Location>,
     content_type: DOMString,
     last_modified: Option<String>,
     encoding: Cell<EncodingRef>,
     has_browsing_context: bool,
     is_html_document: bool,
     activity: Cell<DocumentActivity>,
     url: DOMRefCell<ServoUrl>,
     #[ignore_heap_size_of = "defined in selectors"]
@@ -2217,17 +2216,16 @@ impl Document {
             (DocumentReadyState::Complete, true)
         };
 
         Document {
             node: Node::new_document_node(),
             window: JS::from_ref(window),
             has_browsing_context: has_browsing_context == HasBrowsingContext::Yes,
             implementation: Default::default(),
-            location: Default::default(),
             content_type: match content_type {
                 Some(string) => string,
                 None => DOMString::from(match is_html_document {
                     // https://dom.spec.whatwg.org/#dom-domimplementation-createhtmldocument
                     IsHTMLDocument::HTMLDocument => "text/html",
                     // https://dom.spec.whatwg.org/#concept-document-content-type
                     IsHTMLDocument::NonHTMLDocument => "application/xml",
                 }),
@@ -3433,17 +3431,21 @@ impl DocumentMethods for Document {
         self.applets.or_init(|| {
             let filter = box AppletsFilter;
             HTMLCollection::create(&self.window, self.upcast(), filter)
         })
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-document-location
     fn GetLocation(&self) -> Option<Root<Location>> {
-        self.browsing_context().map(|_| self.location.or_init(|| Location::new(&self.window)))
+        if self.is_fully_active() {
+            Some(self.window.Location())
+        } else {
+            None
+        }
     }
 
     // https://dom.spec.whatwg.org/#dom-parentnode-children
     fn Children(&self) -> Root<HTMLCollection> {
         HTMLCollection::children(&self.window, self.upcast())
     }
 
     // https://dom.spec.whatwg.org/#dom-parentnode-firstelementchild
@@ -3810,17 +3812,16 @@ impl DocumentMethods for Document {
         Node::replace_all(None, self.upcast::<Node>());
 
         // Steps 16-18.
         // Let's not?
         // TODO: https://github.com/whatwg/html/issues/1698
 
         // Step 19.
         self.implementation.set(None);
-        self.location.set(None);
         self.images.set(None);
         self.embeds.set(None);
         self.links.set(None);
         self.forms.set(None);
         self.scripts.set(None);
         self.anchors.set(None);
         self.applets.set(None);
         *self.stylesheets.borrow_mut() = StylesheetSet::new();
--- a/servo/components/script/dom/window.rs
+++ b/servo/components/script/dom/window.rs
@@ -178,16 +178,17 @@ pub struct Window {
     performance_timeline_task_source: PerformanceTimelineTaskSource,
     navigator: MutNullableJS<Navigator>,
     #[ignore_heap_size_of = "Arc"]
     image_cache: Arc<ImageCache>,
     #[ignore_heap_size_of = "channels are hard"]
     image_cache_chan: Sender<ImageCacheMsg>,
     window_proxy: MutNullableJS<WindowProxy>,
     document: MutNullableJS<Document>,
+    location: MutNullableJS<Location>,
     history: MutNullableJS<History>,
     custom_element_registry: MutNullableJS<CustomElementRegistry>,
     performance: MutNullableJS<Performance>,
     navigation_start: Cell<u64>,
     navigation_start_precise: Cell<f64>,
     screen: MutNullableJS<Screen>,
     session_storage: MutNullableJS<Storage>,
     local_storage: MutNullableJS<Storage>,
@@ -563,17 +564,17 @@ impl WindowMethods for Window {
 
     // https://html.spec.whatwg.org/multipage/#dom-window-customelements
     fn CustomElements(&self) -> Root<CustomElementRegistry> {
         self.custom_element_registry.or_init(|| CustomElementRegistry::new(self))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location
     fn Location(&self) -> Root<Location> {
-        self.Document().GetLocation().unwrap()
+        self.location.or_init(|| Location::new(self))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-sessionstorage
     fn SessionStorage(&self) -> Root<Storage> {
         self.session_storage.or_init(|| Storage::new(self, StorageType::Session))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-localstorage
@@ -1849,16 +1850,17 @@ impl Window {
             user_interaction_task_source,
             networking_task_source,
             history_traversal_task_source,
             file_reading_task_source,
             performance_timeline_task_source,
             image_cache_chan,
             image_cache,
             navigator: Default::default(),
+            location: Default::default(),
             history: Default::default(),
             custom_element_registry: Default::default(),
             window_proxy: Default::default(),
             document: Default::default(),
             performance: Default::default(),
             navigation_start: Cell::new(navigation_start),
             navigation_start_precise: Cell::new(navigation_start_precise),
             screen: Default::default(),