[webkit-reviews] review denied: [Bug 88606] [Shadow DOM] Needs @host rule for ShadowDOM styling : [Attachment 159626] Patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Takashi Sakamoto
<tasak at google.com>'s request for review:
Bug 88606: [Shadow DOM] Needs @host rule for ShadowDOM styling
https://bugs.webkit.org/show_bug.cgi?id=88606

Attachment 159626: Patch
https://bugs.webkit.org/attachment.cgi?id=159626&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=159626&action=review


> Source/WebCore/css/StyleResolver.cpp:1046
> +    for (ShadowRoot* shadowRoot = shadow->youngestShadowRoot(); shadowRoot;
shadowRoot = shadowRoot->olderShadowRoot()) {
> +	   if (atHostRuleSetForScope(shadowRoot))
> +	       return true;
> +	   if (!shadowRoot->hasShadowInsertionPoint())
> +	       break;
> +    }

Can we cache this value on ElementShadow?

> Source/WebCore/css/StyleResolver.cpp:1069
> +    Vector<RuleSet*, 16> matchedRules; 
> +    for (ShadowRoot* shadowRoot = shadow->youngestShadowRoot(); shadowRoot;
shadowRoot = shadowRoot->olderShadowRoot()) { 
> +	   if (RuleSet* ruleSet = atHostRuleSetForScope(shadowRoot))
> +	       matchedRules.append(ruleSet);
> +	   if (!shadowRoot->hasShadowInsertionPoint())
> +	       break;
> +    }
> +    if (matchedRules.isEmpty())
> +	   return;

Ah, this could be on ElementShadow, just as I suggested above. Basically,
maintain a list of activeShadowRoots (the interesting bit is that you only need
to store one value, the last visible shadow root from the stack). This should
allow you to make the patch neatly splittable into the section where we first
expose the ElementShadow::activeShadowRoots along with window.internals tests
for it, and then a trivial patch of adding the support @host rules. WDYT?