Skip to content

Commit

Permalink
Auto merge of #18578 - asajeffrey:script-window-owns-location, r=KiCh…
Browse files Browse the repository at this point in the history
…jang

Window should own Location, Document shouldn't

<!-- 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. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18578)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Sep 20, 2017
2 parents 46ae11b + 3d00b0e commit 6a791cd
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
9 changes: 5 additions & 4 deletions components/script/dom/document.rs
Expand Up @@ -226,7 +226,6 @@ pub struct Document {
node: Node,
window: JS<Window>,
implementation: MutNullableJS<DOMImplementation>,
location: MutNullableJS<Location>,
content_type: DOMString,
last_modified: Option<String>,
encoding: Cell<EncodingRef>,
Expand Down Expand Up @@ -2222,7 +2221,6 @@ impl Document {
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 {
Expand Down Expand Up @@ -3438,7 +3436,11 @@ impl DocumentMethods for Document {

// 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
Expand Down Expand Up @@ -3815,7 +3817,6 @@ impl DocumentMethods for Document {

// Step 19.
self.implementation.set(None);
self.location.set(None);
self.images.set(None);
self.embeds.set(None);
self.links.set(None);
Expand Down
4 changes: 3 additions & 1 deletion components/script/dom/window.rs
Expand Up @@ -183,6 +183,7 @@ pub struct Window {
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>,
Expand Down Expand Up @@ -568,7 +569,7 @@ impl WindowMethods for Window {

// 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
Expand Down Expand Up @@ -1854,6 +1855,7 @@ impl Window {
image_cache_chan,
image_cache,
navigator: Default::default(),
location: Default::default(),
history: Default::default(),
custom_element_registry: Default::default(),
window_proxy: Default::default(),
Expand Down

0 comments on commit 6a791cd

Please sign in to comment.