Bug 128877 – Move document life time management from TreeScope to Document

• Status: NEW
• Component: DOM

[Antti Koivisto]

Also devirtualize TreeScope

[Antti Koivisto]

Created attachment 224311 [details]
patch

[Ryosuke Niwa]

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]

> > 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]

Created attachment 224312 [details]
build fix

[Andreas Kling]

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.