Skip to content

Commit

Permalink
Auto merge of #27691 - ghostd:stylesheet-owner-node, r=jdm
Browse files Browse the repository at this point in the history
Implements Stylesheet.ownerNode

<!-- Please describe your changes on the following line: -->

---
<!-- 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 #23082 (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- 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. -->
  • Loading branch information
bors-servo committed Oct 10, 2020
2 parents 406d159 + e7199c0 commit 7f12ee6
Show file tree
Hide file tree
Showing 14 changed files with 72 additions and 39 deletions.
12 changes: 7 additions & 5 deletions components/script/dom/cssrulelist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
use crate::dom::bindings::cell::DomRefCell;
use crate::dom::bindings::codegen::Bindings::CSSRuleListBinding::CSSRuleListMethods;
use crate::dom::bindings::error::{Error, ErrorResult, Fallible};
use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::reflector::{reflect_dom_object, DomObject, Reflector};
use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom};
use crate::dom::csskeyframerule::CSSKeyframeRule;
use crate::dom::cssrule::CSSRule;
use crate::dom::cssstylesheet::CSSStyleSheet;
use crate::dom::htmlelement::HTMLElement;
use crate::dom::window::Window;
use crate::style::stylesheets::StylesheetLoader as StyleStylesheetLoader;
use crate::stylesheet_loader::StylesheetLoader;
use dom_struct::dom_struct;
use servo_arc::Arc;
Expand Down Expand Up @@ -107,17 +107,19 @@ impl CSSRuleList {
let owner = self
.parent_stylesheet
.get_owner()
.downcast::<HTMLElement>()
.unwrap();
let loader = StylesheetLoader::for_element(owner);
.map(DomRoot::downcast::<HTMLElement>)
.flatten();
let loader = owner
.as_ref()
.map(|element| StylesheetLoader::for_element(&**element));
let new_rule = css_rules.with_raw_offset_arc(|arc| {
arc.insert_rule(
&parent_stylesheet.shared_lock,
rule,
&parent_stylesheet.contents,
index,
nested,
Some(&loader),
loader.as_ref().map(|l| l as &dyn StyleStylesheetLoader),
AllowImportRules::Yes,
)
})?;
Expand Down
6 changes: 4 additions & 2 deletions components/script/dom/cssstyledeclaration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,10 @@ impl CSSStyleOwner {
if changed {
// If this is changed, see also
// CSSStyleRule::SetSelectorText, which does the same thing.
stylesheets_owner_from_node(rule.parent_stylesheet().owner().upcast::<Node>())
.invalidate_stylesheets();
if let Some(owner) = rule.parent_stylesheet().get_owner() {
stylesheets_owner_from_node(owner.upcast::<Node>())
.invalidate_stylesheets();
}
}
result
},
Expand Down
5 changes: 3 additions & 2 deletions components/script/dom/cssstylerule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,9 @@ impl CSSStyleRuleMethods for CSSStyleRule {
let mut guard = self.cssrule.shared_lock().write();
let stylerule = self.stylerule.write_with(&mut guard);
mem::swap(&mut stylerule.selectors, &mut s);
stylesheets_owner_from_node(self.cssrule.parent_stylesheet().owner().upcast::<Node>())
.invalidate_stylesheets();
if let Some(owner) = self.cssrule.parent_stylesheet().get_owner() {
stylesheets_owner_from_node(owner.upcast::<Node>()).invalidate_stylesheets();
}
}
}
}
23 changes: 12 additions & 11 deletions components/script/dom/cssstylesheet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::dom::bindings::codegen::Bindings::CSSStyleSheetBinding::CSSStyleSheet
use crate::dom::bindings::error::{Error, ErrorResult, Fallible};
use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::reflector::{reflect_dom_object, DomObject};
use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom};
use crate::dom::bindings::root::{DomRoot, MutNullableDom};
use crate::dom::bindings::str::DOMString;
use crate::dom::cssrulelist::{CSSRuleList, RulesSource};
use crate::dom::element::Element;
Expand All @@ -22,7 +22,7 @@ use style::stylesheets::Stylesheet as StyleStyleSheet;
#[dom_struct]
pub struct CSSStyleSheet {
stylesheet: StyleSheet,
owner: Dom<Element>,
owner: MutNullableDom<Element>,
rulelist: MutNullableDom<CSSRuleList>,
#[ignore_malloc_size_of = "Arc"]
style_stylesheet: Arc<StyleStyleSheet>,
Expand All @@ -39,7 +39,7 @@ impl CSSStyleSheet {
) -> CSSStyleSheet {
CSSStyleSheet {
stylesheet: StyleSheet::new_inherited(type_, href, title),
owner: Dom::from_ref(owner),
owner: MutNullableDom::new(Some(owner)),
rulelist: MutNullableDom::new(None),
style_stylesheet: stylesheet,
origin_clean: Cell::new(true),
Expand All @@ -63,10 +63,6 @@ impl CSSStyleSheet {
)
}

pub fn owner(&self) -> DomRoot<Element> {
DomRoot::from_ref(&*self.owner)
}

fn rulelist(&self) -> DomRoot<CSSRuleList> {
self.rulelist.or_init(|| {
let rules = self.style_stylesheet.contents.rules.clone();
Expand All @@ -78,16 +74,21 @@ impl CSSStyleSheet {
self.style_stylesheet.disabled()
}

pub fn get_owner(&self) -> &Element {
&*self.owner
pub fn get_owner(&self) -> Option<DomRoot<Element>> {
self.owner.get()
}

pub fn set_disabled(&self, disabled: bool) {
if self.style_stylesheet.set_disabled(disabled) {
stylesheets_owner_from_node(self.owner().upcast::<Node>()).invalidate_stylesheets();
if self.style_stylesheet.set_disabled(disabled) && self.get_owner().is_some() {
stylesheets_owner_from_node(self.get_owner().unwrap().upcast::<Node>())
.invalidate_stylesheets();
}
}

pub fn set_owner(&self, value: Option<&Element>) {
self.owner.set(value);
}

pub fn shared_lock(&self) -> &SharedRwLock {
&self.style_stylesheet.shared_lock
}
Expand Down
10 changes: 9 additions & 1 deletion components/script/dom/htmllinkelement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ impl HTMLLinkElement {
stylesheets_owner.remove_stylesheet(self.upcast(), s)
}
*self.stylesheet.borrow_mut() = Some(s.clone());
self.cssom_stylesheet.set(None);
self.clean_stylesheet_ownership();
stylesheets_owner.add_stylesheet(self.upcast(), s);
}

Expand Down Expand Up @@ -148,6 +148,13 @@ impl HTMLLinkElement {
None => false,
}
}

fn clean_stylesheet_ownership(&self) {
if let Some(cssom_stylesheet) = self.cssom_stylesheet.get() {
cssom_stylesheet.set_owner(None);
}
self.cssom_stylesheet.set(None);
}
}

fn get_attr(element: &Element, local_name: &LocalName) -> Option<String> {
Expand Down Expand Up @@ -255,6 +262,7 @@ impl VirtualMethods for HTMLLinkElement {
}

if let Some(s) = self.stylesheet.borrow_mut().take() {
self.clean_stylesheet_ownership();
stylesheets_owner_from_node(self).remove_stylesheet(self.upcast(), &s);
}
}
Expand Down
10 changes: 9 additions & 1 deletion components/script/dom/htmlstyleelement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl HTMLStyleElement {
stylesheets_owner.remove_stylesheet(self.upcast(), s)
}
*self.stylesheet.borrow_mut() = Some(s.clone());
self.cssom_stylesheet.set(None);
self.clean_stylesheet_ownership();
stylesheets_owner.add_stylesheet(self.upcast(), s);
}

Expand All @@ -166,6 +166,13 @@ impl HTMLStyleElement {
})
})
}

fn clean_stylesheet_ownership(&self) {
if let Some(cssom_stylesheet) = self.cssom_stylesheet.get() {
cssom_stylesheet.set_owner(None);
}
self.cssom_stylesheet.set(None);
}
}

impl VirtualMethods for HTMLStyleElement {
Expand Down Expand Up @@ -217,6 +224,7 @@ impl VirtualMethods for HTMLStyleElement {

if context.tree_connected {
if let Some(s) = self.stylesheet.borrow_mut().take() {
self.clean_stylesheet_ownership();
stylesheets_owner_from_node(self).remove_stylesheet(self.upcast(), &s)
}
}
Expand Down
7 changes: 7 additions & 0 deletions components/script/dom/stylesheet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
use crate::dom::bindings::codegen::Bindings::StyleSheetBinding::StyleSheetMethods;
use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::reflector::Reflector;
use crate::dom::bindings::root::DomRoot;
use crate::dom::bindings::str::DOMString;
use crate::dom::cssstylesheet::CSSStyleSheet;
use crate::dom::element::Element;
use dom_struct::dom_struct;

#[dom_struct]
Expand Down Expand Up @@ -44,6 +46,11 @@ impl StyleSheetMethods for StyleSheet {
self.href.clone()
}

// https://drafts.csswg.org/cssom/#dom-stylesheet-ownernode
fn GetOwnerNode(&self) -> Option<DomRoot<Element>> {
self.downcast::<CSSStyleSheet>().and_then(|s| s.get_owner())
}

// https://drafts.csswg.org/cssom/#dom-stylesheet-title
fn GetTitle(&self) -> Option<DOMString> {
self.title.clone()
Expand Down
2 changes: 1 addition & 1 deletion components/script/dom/webidls/StyleSheet.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ interface StyleSheet {
readonly attribute DOMString type_;
readonly attribute DOMString? href;

// readonly attribute (Element or ProcessingInstruction)? ownerNode;
readonly attribute Element? ownerNode;
// readonly attribute StyleSheet? parentStyleSheet;
readonly attribute DOMString? title;

Expand Down
10 changes: 9 additions & 1 deletion tests/wpt/metadata-layout-2020/css/cssom/MediaList2.xhtml.ini
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
[MediaList2.xhtml]
[MediaList]
[MediaList.mediaText]
expected: FAIL

[MediaList.length]
expected: FAIL

[MediaList getter]
expected: FAIL

[MediaList.item]
expected: FAIL
6 changes: 0 additions & 6 deletions tests/wpt/metadata-layout-2020/css/cssom/idlharness.html.ini
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@
[CSSStyleDeclaration interface: calling getPropertyPriority(CSSOMString) on sheet.cssRules[2\].cssRules[0\].style with too few arguments must throw TypeError]
expected: FAIL

[StyleSheet interface: attribute ownerNode]
expected: FAIL

[CSSRule interface: sheet.cssRules[2\].cssRules[0\] must inherit property "STYLE_RULE" with the proper type]
expected: FAIL

Expand Down Expand Up @@ -338,9 +335,6 @@
[StyleSheet interface: sheet must inherit property "type" with the proper type]
expected: FAIL
[StyleSheet interface: sheet must inherit property "ownerNode" with the proper type]
expected: FAIL
[CSSStyleDeclaration interface: sheet.cssRules[2\].cssRules[0\].style must inherit property "cssText" with the proper type]
expected: FAIL
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[ttwf-cssom-doc-ext-load-count.html]
[stylesheet.css should be unloaded and styleSheets.length === 0]
[stylesheet-1.css should be loaded and styleSheets.length === 1]
expected: FAIL

10 changes: 9 additions & 1 deletion tests/wpt/metadata/css/cssom/MediaList2.xhtml.ini
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
[MediaList2.xhtml]
[MediaList]
[MediaList.mediaText]
expected: FAIL

[MediaList.length]
expected: FAIL

[MediaList getter]
expected: FAIL

[MediaList.item]
expected: FAIL
6 changes: 0 additions & 6 deletions tests/wpt/metadata/css/cssom/idlharness.html.ini
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@
[CSSStyleDeclaration interface: calling getPropertyPriority(CSSOMString) on sheet.cssRules[2\].cssRules[0\].style with too few arguments must throw TypeError]
expected: FAIL

[StyleSheet interface: attribute ownerNode]
expected: FAIL

[CSSRule interface: sheet.cssRules[2\].cssRules[0\] must inherit property "STYLE_RULE" with the proper type]
expected: FAIL

Expand Down Expand Up @@ -350,9 +347,6 @@
[StyleSheet interface: sheet must inherit property "type" with the proper type]
expected: FAIL
[StyleSheet interface: sheet must inherit property "ownerNode" with the proper type]
expected: FAIL
[CSSStyleDeclaration interface: sheet.cssRules[2\].cssRules[0\].style must inherit property "cssText" with the proper type]
expected: FAIL
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[ttwf-cssom-doc-ext-load-count.html]
type: testharness
[stylesheet.css should be unloaded and styleSheets.length === 0]
[stylesheet-1.css should be loaded and styleSheets.length === 1]
expected: FAIL

0 comments on commit 7f12ee6

Please sign in to comment.