[webkit-reviews] review granted: [Bug 107095] Move parameters (firstRuleIndex and lastRuleIndex) into a parameter object, MatchRequest. : [Attachment 183124] factoring.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Hayato Ito <hayato at chromium.org>'s
request for review:
Bug 107095: Move parameters (firstRuleIndex and lastRuleIndex) into a parameter
object, MatchRequest.
https://bugs.webkit.org/show_bug.cgi?id=107095

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=183124&action=review


Seems OK, but I have a refinement I think would make this cleaner, so I am
going to say review+.

> Source/WebCore/css/StyleResolver.h:345
>      struct MatchRequest {

I think a better design would be to make the ruleSet and scope data members
const the way the includeEmptyRules member is, then make firstRuleIndex and
lastRuleIndex be plain old int members rather than references. Then we could
pass non-const MatchRequest& objects in and handle this in a more natural way.

That would also allow us to keep the default initial values for
includeEmptyRules and scope, and so be cleaner at the call sites.

It’s really strange for const MatchRequest& to “secretly” have some references
in it and so be partly an “out” argument. There’s no reason to be twisted like
that.