Bug 145528 – [Content Extensions] resource-type and load-type should be independent

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

[Alex Christensen]

Right now we use the same uint16_t to store all the load-type and resource-type flags, then we just do a bitwise and to check both at the same time.  This results in a trigger with load-type and resource-type firing if either condition is met, not both conditions.  A trigger with both resource-type and load-type conditions should only fire if both conditions are met.

[Alex Christensen]

Created attachment 254015 [details]
Patch

[Benjamin Poulain]

Comment on attachment 254015 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Covered by existing tests and a new API test.

IMHO, you should explain the bug here. Having the explanation in bugzilla is not enough.

> Source/WebCore/contentextensions/DFABytecodeInterpreter.cpp:81
> +    bool shouldAppendAction = false;
> +    if (shouldCheckLoadType && shouldCheckResourceType)
> +        shouldAppendAction = (flagsToCheck & LoadTypeMask & flags) && (flagsToCheck & ResourceTypeMask & flags);
> +    else if (shouldCheckLoadType)
> +        shouldAppendAction = flagsToCheck & LoadTypeMask & flags;
> +    else {
> +        ASSERT(shouldCheckResourceType);
> +        shouldAppendAction = flagsToCheck & ResourceTypeMask & flags;
> +    }

What about:

uint16_t loadTypeFlags = flagsToCheck & LoadTypeMask;
uint16_t ressourceTypeFlags = flagsToCheck & ResourceTypeMask;

bool loadTypeMatches = loadTypeFlags ? (loadTypeFlags & flags) : true;
bool ressourceTypeMatches = ressourceTypeFlags ? (ressourceTypeFlags & flags) : true;

if (loadTypeMatches && ressourceTypeMatches)
   ...

[Alex Christensen]

rdar://problem/21190765

[Alex Christensen]

http://trac.webkit.org/changeset/185079