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+

Description Antti Koivisto 2014-02-15 18:43:04 PST
Also devirtualize TreeScope
Comment 1 Antti Koivisto 2014-02-15 19:29:11 PST
Created attachment 224311 [details]
patch
Comment 2 Ryosuke Niwa 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?
Comment 3 Antti Koivisto 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.
Comment 4 Antti Koivisto 2014-02-15 19:57:58 PST
Created attachment 224312 [details]
build fix
Comment 5 Andreas Kling 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.