[Webkit-unassigned] [Bug 84048] ShadowRoot needs resetStyleInheritance

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=84048





--- Comment #25 from Takashi Sakamoto <tasak at google.com>  2012-05-16 03:50:33 PST ---
(In reply to comment #23)
> (From update of attachment 141840 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=141840&action=review
> 
> I think we don't need to compute m_parentNodeForRenderingAndStyleAtShadowBoundary at the constructor
> but we can compute it on the fly.
> 
> How about to have NodeRenderingContext::resetStyleInheritance() ?

Do you mean NodeRenderingContext also should look up ShadowRoot's resetStyleInheritance?
> 
> >> Source/WebCore/css/StyleResolver.cpp:1642
> >> +    bool resetStyleInheritance = element && element->treeScope()->resetStyleInheritance() && element->parentNodeForRenderingAndStyleAtShadowBoundary();
> > 
> > in this statement, the first two clauses are cheap, and the third one is expensive. What impact will this have on this hot function? Perhaps we could get rid of it somehow?
> 
> in initElement(), we call parentNodeForRendering().
> That means we compute NodeRenderingContext behind that. 
> Instead of calling parentNodeForRendering() there, we can use NodeRenderingContext directly to retrieve what we need.
> Both finding style reset boundary and finding the parent for rendering are (or should be) parts of same computation process.

I added "NodeRenderingContext context(e);" to obtain parentNode and information about whether parentNode is in the same tree as the element:e or not.
However I'm not sure which is the better way to obtain NodeRenderingContext, creating a new method in class Node, i.e. Node::parentNodeForRendering() or just adding "NodeRenderingContext context(e)".
Is there any policy? I mean, Node should create NodeRenderingContext and StyleResolver shouldn't or something.

> 
> > Source/WebCore/dom/NodeRenderingContext.cpp:58
> > +    , m_parentNodeForRenderingAndStyleAtShadowBoundary(0)
> 
> Should be false.

Done.

> 
> > Source/WebCore/dom/NodeRenderingContext.cpp:86
> > +                    m_parentNodeForRenderingAndStyleAtShadowBoundary = NodeRenderingContext(m_insertionPoint).parentNodeForRenderingAndStyleAtShadowBoundary();
> 
> Please don't create NodeRenderingContext twice. It's expensive.

I see. Done.

> 
> > Source/WebCore/dom/NodeRenderingContext.cpp:93
> > +            m_parentNodeForRenderingAndStyleAtShadowBoundary = false;
> 
> It looks we don't need this because it's the fault value.

Done.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.