Bug 128877

Summary: Move document life time management from TreeScope to Document
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: cmarcelo, commit-queue, darin, esprehn+autocc, glenn, gyuyoung.kim, kangil.han, kling, macpherson, menard, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
build fix kling: review+

Antti Koivisto
Reported 2014-02-15 18:43:04 PST
Also devirtualize TreeScope
Attachments
patch (29.60 KB, patch)
2014-02-15 19:29 PST, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
build fix (29.75 KB, patch)
2014-02-15 19:57 PST, Antti Koivisto 		kling: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Antti Koivisto
Comment 1 2014-02-15 19:29:11 PST
Created attachment 224311 [details] patch
Ryosuke Niwa
Comment 2 2014-02-15 19:48:21 PST
Comment on attachment 224311 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=224311&action=review > Source/WebCore/css/ElementRuleCollector.cpp:158 > - if (!MatchingUARulesScope::isMatchingUARules() > - && !m_element.treeScope().applyAuthorStyles()) > + // Only match UA rules in shadow tree. > + if (!MatchingUARulesScope::isMatchingUARules() && m_element.treeScope().rootNode()->isShadowRoot()) It seems like we can put this in a separate patch. > Source/WebCore/dom/TreeScope.cpp:-54 > - int ints[1]; Don't we still have m_resetStyleInheritance and m_type as flags here?
Antti Koivisto
Comment 3 2014-02-15 19:57:30 PST
> > Source/WebCore/dom/TreeScope.cpp:-54 > > - int ints[1]; > > Don't we still have m_resetStyleInheritance and m_type as flags here? You are thinking ShadowRoot.
Antti Koivisto
Comment 4 2014-02-15 19:57:58 PST
Created attachment 224312 [details] build fix
Andreas Kling
Comment 5 2014-02-16 11:27:37 PST
Comment on attachment 224312 [details] build fix View in context: https://bugs.webkit.org/attachment.cgi?id=224312&action=review r=me. This feels like a good step forward. > Source/WebCore/dom/Document.h:1714 > - , m_treeScope(document ? document : &TreeScope::noDocumentInstance()) > + , m_treeScope(document) Nice! > Source/WebCore/dom/DocumentOrderedMap.cpp:94 > + UNUSED_PARAM(treeScope); > ASSERT_WITH_SECURITY_IMPLICATION(element.isInTreeScope()); > ASSERT_WITH_SECURITY_IMPLICATION(treeScope.rootNode()->containsIncludingShadowDOM(&element)); Oh, there's no ASSERT_WITH_SECURITY_IMPLICATION_UNUSED so we have to use UNUSED_PARAM. Not ideal but whatever. > Source/WebCore/dom/Node.cpp:306 > if (hasRareData()) > clearRareData(); This is also done by ~ShadowRoot which no longer seems necessary. > Source/WebCore/dom/TreeScope.h:98 > + ContainerNode* rootNode() const { return &m_rootNode; } I suppose we can make this return ContainerNode& later on.
Note You need to log in before you can comment on or make changes to this bug.