[Webkit-unassigned] [Bug 205264] Resolve dynamic media queries without reconstructing RuleSets

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 16 07:07:16 PST 2019


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

--- Comment #2 from Emilio Cobos Álvarez (:emilio) <emilio at crisal.io> ---
Comment on attachment 385758
  --> https://bugs.webkit.org/attachment.cgi?id=385758
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=385758&action=review

This looks fancy to me (I took the CC as an invitation to take a look, but please let me know if I took it wrong).

I can't r+ it myself as I'm not 100% sure of the above and I'm not technically a reviewer, but this looks good otherwise.

The only thing I haven't double-checked and would take me more to check is whether this causes WebKit to overinvalidate for class/id/attribute changes for stuff that doesn't apply. So like whether:

@media (width: 0) {
  .myclass * { ... }
}

Causes "myclass" changes to restyle the whole subtree even if the feature doesn't match. Seems it probably would, but it might be ok.

> Source/WebCore/css/MediaQueryEvaluator.cpp:149
>  {

Probably assert that if mode is AlwaysMatchDynamic, then dynamicResults is not null?

> Source/WebCore/style/RuleSet.cpp:308
> +            mediaQueryCollector.didMutateResolver();

There's nothing preventing you in the future to take the same approach for keyframes and font-face and such, right? (as in, track them in the dynamic results, and enable / disable them on demand).

But this seems fine for now, and it is certainly hard to handle.

> Source/WebCore/style/RuleSet.cpp:335
> +        auto mediaQueryCollector = MediaQueryCollector { evaluator, true };

nit: May be worth to make collectDynamic an enum class, so this reads like CollectDynamic::Yes or such. But if it makes the surrounding code too ugly then probably never mind.

> Source/WebCore/style/RuleSet.h:80
> +            Vector<size_t> affectedRulePositions { };

nit: I think you can just use Vector<size_t> affectedRulePositions; and same with the other vectors here.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20191216/17e136a8/attachment-0001.htm>


More information about the webkit-unassigned mailing list