I am not a fan of listing every option. When the author of adblock add an option, we will need to update the code. What about just one field: hasUnsupportedOption set to true if any option is not used by the filter? Not to mention adblock already takes its share of memory. Don't forget those field will be allocated for each rule.
I am still not a fan of the patch. I think its complexity is high for what it adds, we can probably do better. I also don't like the idea of adding a branch because there is a rule, but it is invalid. It does not look very clean IMHO. Alternative ideas: -add a AdBlockRuleNullImpl that always return false for ::match(). And create that in AdBlockRule::AdBlockRule() if there are any unsupported options. -or add AdBlockRule::isValid(). And do not set the m_implementation if there are any unsupported option. The AdBlockManager would not add a rule if it is not valid, so they are never evaluated. I am also not a fan of comparing lots of string for each rules. What about keeping a QSet of unsupported options?
Review request changed
This new patch implements a null rule to match all unimplemented filters. Just for the records, we have this way implemented 3 different kind of rules: - Text: 1753 filters matched, - Fallback: 1305 filters, - Null: 1154 filters. So, this implementation saves sizeof(QString)*1154 bytes. It is anyway a bit more heavy on startup, having the extra null check added. Not so easy to measure how much heavier.. ;)
Revision 4 (+232 -2)