Bug 145069 – [Content Extensions] Relax restrictions on triggers that match everything.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alex Christensen]

Right now we have restrictions on rules with triggers that match everything such as .*
We do not allow them after ignore-previous-rules.
We do not allow them with if-domain or unless-domain.

There are simple workarounds for these restrictions such as using [a-zA-Z]* which would have effectively the same behavior but kill performance by matching on almost every character.  We should just compile the right thing and let people be able to use .* everywhere and encourage them to put their css at the beginning.

[Alex Christensen]

Created attachment 253219 [details]
Patch

[Alex Christensen]

Comment on attachment 253219 [details]
Patch

I need to increment the version number and rebase this after https://bugs.webkit.org/show_bug.cgi?id=145027 but please review.

[Alex Christensen]

Created attachment 253221 [details]
Patch

[Alex Christensen]

Comment on attachment 253221 [details]
Patch

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

> Source/WebCore/contentextensions/DFABytecodeInterpreter.cpp:122
>                  if (instruction == DFABytecodeInstruction::AppendAction)

this should be else if

[Benjamin Poulain]

Comment on attachment 253221 [details]
Patch

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

> Source/WebCore/contentextensions/DFABytecodeCompiler.cpp:53
> +    // High bits are used to store flags. Booleans are stored in the 48th and 49th bit. See compileRuleList.

Describing the format here guarantees your comment will get out of sync.

> Source/WebCore/contentextensions/DFABytecodeCompiler.cpp:66
> +        if (action & DisplayNoneStyleSheetFlag) {
> +            RELEASE_ASSERT(!(action & IfDomainFlag));
> +            append<DFABytecodeInstruction>(m_bytecode, DFABytecodeInstruction::AppendActionDefaultStylesheet);
> +        } else if (action & IfDomainFlag)

How do you get into those cases?

not(action & 0xFFFF00000000)  is a superset of those two tests.

[Alex Christensen]

(In reply to comment #5)
> not(action & 0xFFFF00000000)  is a superset of those two tests.
That code is correct. not(action & 0x3000000000000) is a superset of those two tests.

[Alex Christensen]

I added a test using backend5 and fixed the else if.
http://trac.webkit.org/changeset/184644

[Benjamin Poulain]

(In reply to comment #6)
> (In reply to comment #5)
> > not(action & 0xFFFF00000000)  is a superset of those two tests.
> That code is correct. not(action & 0x3000000000000) is a superset of those
> two tests.

ok

[Alex Christensen]

rdar://problem/20937620