1440180 - stylo: Some computed values are not reported in memory report

Status: NEW

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Xidorn Quan [:xidorn] UTC+10]

See bug 1421374 comment 9, there seems to be non-trivial amount of ComputedValues and style structs unreported from the DMD report.

They are generated from normal traversal path, something like:
> #01: mozilla::dmd::AllocCallback (\memory\replace\dmd\dmd.cpp:1211)
> #02: replace_malloc (\memory\replace\dmd\dmd.cpp:1265)
> #03: std::heap::__default_lib_allocator::__rdl_alloc (C:\projects\rust\src\libstd\heap.rs:33)
> #04: alloc::heap::{{impl}}::alloc (C:\projects\rust\src\liballoc\heap.rs:84)
> #05: alloc::heap::exchange_malloc (C:\projects\rust\src\liballoc\heap.rs:249)
> #06: servo_arc::Arc<style::gecko_properties::ComputedValues>::new<style::gecko_properties::ComputedValues> (\servo\components\servo_arc\lib.rs:184)
> #07: style::properties::StyleBuilder::build (\objdir\toolkit\library\x86_64-pc-windows-msvc\debug\build\style-64f0ffe836833b2d\out\properties.rs:135653)
> #08: style::properties::apply_declarations<style::gecko::wrapper::GeckoElement,closure,core::iter::FlatMap<style::rule_tree::SelfAndAncestors, core::iter::FilterMap<core::iter::Rev<style::properties::declaration_block::DeclarationImportanceIterator>, closure>, (\objdir\toolkit\library\x86_64-pc-windows-msvc\debug\build\style-64f0ffe836833b2d\out\properties.rs:137123)
> #09: style::properties::cascade<style::gecko::wrapper::GeckoElement> (\objdir\toolkit\library\x86_64-pc-windows-msvc\debug\build\style-64f0ffe836833b2d\out\properties.rs:136693)
> #10: style::stylist::Stylist::cascade_style_and_visited<style::gecko::wrapper::GeckoElement> (\servo\components\style\stylist.rs:920)
> #11: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_style_and_visited<style::gecko::wrapper::GeckoElement> (\servo\components\style\style_resolver.rs:304)
> #12: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_primary_style<style::gecko::wrapper::GeckoElement> (\servo\components\style\style_resolver.rs:212)
> #13: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::resolve_primary_style<style::gecko::wrapper::GeckoElement> (\servo\components\style\style_resolver.rs:180)
> #14: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::resolve_style<style::gecko::wrapper::GeckoElement> (\servo\components\style\style_resolver.rs:232)
> #15: style::style_resolver::{{impl}}::resolve_style_with_default_parents::{{closure}}<style::gecko::wrapper::GeckoElement> (\servo\components\style\style_resolver.rs:269)
> #16: style::style_resolver::with_default_parent_styles<style::gecko::wrapper::GeckoElement,closure,style::style_resolver::ResolvedElementStyles> (\servo\components\style\style_resolver.rs:102)
> #17: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::resolve_style_with_default_parents<style::gecko::wrapper::GeckoElement> (\servo\components\style\style_resolver.rs:270)
> #18: style::traversal::compute_style<style::gecko::wrapper::GeckoElement> (\servo\components\style\traversal.rs:616)
> #19: style::traversal::recalc_style_at<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly,closure> (\servo\components\style\traversal.rs:421)
> #20: style::gecko::traversal::{{impl}}::process_preorder<closure> (\servo\components\style\gecko\traversal.rs:38)
> #21: style::driver::traverse_dom<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly> (\servo\components\style\driver.rs:81)
> #22: geckoservo::glue::traverse_subtree (\servo\ports\geckolib\glue.rs:288)
> #23: geckoservo::glue::Servo_TraverseSubtree (\servo\ports\geckolib\glue.rs:354)
> #24: mozilla::ServoStyleSet::StyleNewSubtree (\layout\style\servostyleset.cpp:1077)

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

I checked whether there is any element which is created but not reported. I did find one, which is a <br> element created by editor with the following stack:
> #01: mozilla::dmd::AllocCallback (\memory\replace\dmd\dmd.cpp:1211)
> #02: replace_malloc (\memory\replace\dmd\dmd.cpp:1265)
> #03: moz_xmalloc (\memory\mozalloc\mozalloc.cpp:71)
> #04: NS_NewHTMLBRElement (\dom\html\htmlbrelement.cpp:15)
> #05: CreateHTMLElement (\dom\html\nshtmlcontentsink.cpp:253)
> #06: nsContentUtils::NewXULOrHTMLElement (\dom\base\nscontentutils.cpp:10213)
> #07: NS_NewHTMLElement (\dom\html\nshtmlcontentsink.cpp:236)
> #08: NS_NewElement (\dom\base\nsnamespacemanager.cpp:183)
> #09: nsDocument::CreateElem (\dom\base\nsdocument.cpp:7971)
> #10: mozilla::EditorBase::CreateHTMLContent (\editor\libeditor\editorbase.cpp:5024)
> #11: mozilla::TextEditRules::CreateBogusNodeIfNeeded (\editor\libeditor\texteditrules.cpp:1455)
> #12: mozilla::TextEditRules::Init (\editor\libeditor\texteditrules.cpp:145)
> #13: mozilla::TextEditor::EndEditorInit (\editor\libeditor\texteditor.cpp:204)
> #14: mozilla::TextEditor::Init (\editor\libeditor\texteditor.cpp:138)

But a single <br> element cannot explain the amount of unreported ComputedValues, so there must be some other cases.

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

A opt build gives us a deeper stack:
> #01: mozilla::dmd::AllocCallback (\memory\replace\dmd\dmd.cpp:1211)
> #02: replace_malloc (\memory\replace\dmd\dmd.cpp:1264)
> #03: style::properties::StyleBuilder::build (\objdir\toolkit\library\x86_64-pc-windows-msvc\release\build\style-ac145e11eea874da\out\properties.rs:135610)
> #04: style::properties::cascade<style::gecko::wrapper::GeckoElement> (\objdir\toolkit\library\x86_64-pc-windows-msvc\release\build\style-ac145e11eea874da\out\properties.rs:136610)
> #05: style::stylist::Stylist::cascade_style_and_visited<style::gecko::wrapper::GeckoElement> (\servo\components\style\stylist.rs:920)
> #06: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_style_and_visited<style::gecko::wrapper::GeckoElement> (\servo\components\style\style_resolver.rs:304)
> #07: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_primary_style<style::gecko::wrapper::GeckoElement> (\servo\components\style\style_resolver.rs:212)
> #08: style::style_resolver::{{impl}}::cascade_styles_with_default_parents::{{closure}}<style::gecko::wrapper::GeckoElement> (\servo\components\style\style_resolver.rs:339)
> #09: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_styles_with_default_parents<style::gecko::wrapper::GeckoElement> (\servo\components\style\style_resolver.rs:332)
> #10: style::traversal::compute_style<style::gecko::wrapper::GeckoElement> (\servo\components\style\traversal.rs:674)
> #11: geckoservo::glue::traverse_subtree (\servo\ports\geckolib\glue.rs:286)
> #12: geckoservo::glue::Servo_TraverseSubtree (\servo\ports\geckolib\glue.rs:345)
> #13: mozilla::ServoStyleSet::StyleDocument (\layout\style\servostyleset.cpp:1008)
> #14: mozilla::ServoRestyleManager::DoProcessPendingRestyles (\layout\base\servorestylemanager.cpp:1158)
> #15: mozilla::PresShell::DoFlushPendingNotifications (\layout\base\presshell.cpp:4199)
> #16: mozilla::PresShell::DoFlushPendingNotifications (\layout\base\presshell.cpp:4069)
> #17: nsDocument::FlushPendingNotifications (\dom\base\nsdocument.cpp:7581)
> #18: nsComputedDOMStyle::UpdateCurrentStyleSources (\layout\style\nscomputeddomstyle.cpp:995)
> #19: nsComputedDOMStyle::GetPropertyCSSValue (\layout\style\nscomputeddomstyle.cpp:1212)
> #20: nsComputedDOMStyle::GetPropertyValue (\layout\style\nscomputeddomstyle.cpp:502)
> #21: nsComputedDOMStyle::GetPropertyValue (\layout\style\nscomputeddomstyle.cpp:437)
> #22: mozilla::dom::CSS2PropertiesBinding::get_color (\objdir\dom\bindings\css2propertiesbinding.cpp:8594)
> #23: mozilla::dom::GenericBindingGetter (\dom\bindings\bindingutils.cpp:2911)
> #24: js::InternalCallOrConstruct (\js\src\vm\interpreter.cpp:468)

So the Servo_TraverseSubtree is just called for a general style flush. Nothing looks specifically weird from the stack.

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

Attachment: patch for debugging

This is the patch I used to dump list of elements which unreported computed values are initially constructed. It helped finding why we fail to report the computed values.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

So after bug 1440221 and bug 1440222, we would have the following items left in the list, which is a lot so maybe we can well ignore them for now...
> None, Some(ButtonContent) 
> Some(<key id=key_toggleReaderMode> (0x2065f055b80)), None 
> Some(<button id=measureButton> (0x206687c2d60)), None 
> Some(<dropmarker class="urlbar-history-dropmarker urlbar-icon chromeclass-toolbar-additional"> (0x206609eadc0)), None 
> Some(<button id=measureButton> (0x206687c2d60)), Some(MozFocusInner) 
> None, Some(ScrolledContent) 
> Some(<toolbarseparator> (0x2065f128ee0)), None 
> None, Some(ScrolledContent) 
> Some(<hbox id=titlebar-buttonbox> (0x2065f1360d0)), None 
> Some(<richlistbox class="autocomplete-richlistbox"> (0x2065f7e4040)), None 
> Some(<button id=measureButton> (0x206687c2d60)), Some(MozFocusInner) 
> Some(<command id=Browser:ReloadOrDuplicate> (0x2065f051790)), None 
> Some(<thumb> (0x2065c0ebca0)), None

Comment 5

[Jet Villegas (inactive)]

[Triage 2018/03/23 - P3]