[Webkit-unassigned] [Bug 88606] [Shadow DOM] Needs @host rule for ShadowDOM styling

[bugzilla-daemon at webkit.org]

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





--- Comment #13 from Takashi Sakamoto <tasak at google.com>  2012-08-01 03:21:36 PST ---
Thank you for reviewing my WIP patch.

(In reply to comment #9)
> (From update of attachment 155477 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=155477&action=review
> 
> There are several patches in here, to be landed separately. It's fine to work on this as one patch for now, however.
> 
> ProTip: don't fill out the ChangeLog until you're ready to start landing. Otherwise, you'll end up rewriting it many times :)

I see. I recreated ChangeLog and removed all details about my patch.

> > Source/WebCore/html/HTMLStyleElement.cpp:155
> > +    if (hasHostRule())
> > +        if (ShadowRoot* sr = shadowRoot())
> > +            if (Element* host = sr->host()) {
> > +                host->registerScopedHTMLStyleChild();
> > +                isHostRuleRegistered = true;
> > +            }
> 
> Wild to see this here. Why can't we do this when we're adding rules in StyleResolver, like all other machinery in CSS does?
> 
> > Source/WebCore/html/HTMLStyleElement.cpp:185
> > +        if (m_scopedStyleRegistrationState == RegisteredInShadowRootWithHostRule || m_scopedStyleRegistrationState == RegisteredAsScopedWithHostRule)
> > +            if (ShadowRoot* sr = shadowRoot())
> > +                if (Element* host = sr->host())
> > +                    host->unregisterScopedHTMLStyleChild();
> 
> You won't need any of these when you move @host accounting in styleresolver.
> 
> > Source/WebCore/html/HTMLStyleElement.cpp:200
> > +bool HTMLStyleElement::hasHostRule() const
> > +{
> > +    if (CSSStyleSheet* styleSheet = const_cast<HTMLStyleElement*>(this)->sheet())
> > +        return styleSheet->contents()->hasHostRule();
> > +    return false;
> > +}
> 
> Or these.

I see. As I decided to create the own matchHostRules or collectMatchingRules, I don't need to use m_scopedAuthorStyles now.
I removed these codes and modified StyleResolver, i.e. adding m_atHostRules to class StyleResolver, not class RuleSet.

> 
> > Source/WebCore/html/shadow/HTMLShadowElement.h:56
> > +    bool m_registeredWithShadowRoot;
> 
> It seems we're building an extra set of machinery for something that already exists. We already _know_ which shadow trees are rendered or not by the time we've created the composed tree, which happens before style resolution. Please ask morrita@/hayato@ for guidance here.

I asked hayato-san and he said that it is possible to find insertion points when distributed nodes are given. However, when shadow roots are given, there are no direct way to find whether there are any <shadow> in the shadow DOM subtrees.
He said, m_nodeToInsertionPoint's values() are insertion points. So, by using the following way:
(1) obtaining treescope of each m_nodeToInsertionPoint value and
(2) comparing the treescope with the given shadow root.
we can find active insertion points.

Is it better to try hayato-san's way and to file a new bug: "improve performance of finding <shadow>"?

Best regards,
Takashi Sakamoto

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