Clarify filters and versions for processing

[aselbie]

This PR fixes some edge case issues with version and filterType, and updates the code to make it more obvious how they interact to control processing of scripts.

Changes

Address currentFilterType edge case

Changes the currently global currentFilterType to filterTypeByVersion which is a map of filter type per version.

This hardens a potential fragile point of the code:

1. Page loads and we find both manifests, setting currentFilterType to BOTH.
2. Code from a new version is encountered.
3. We process one manifest, but not the other, but because we're still in BOTH mode, scripts that shouldn't be processed until both manifests for the new version are available get processed anyway, and we falsely mark the page is invalid.

Currently, this most likely cannot happen since our server side code injects new manifests into the page at the same time, however this should protect us from any future changes, both on the client and server side.

Assume script belong to latest version

In cases where we don't know what version a script belongs to, we now push to the latest version in FOUND_SCRIPTS rather than the first. For the initial page load where we are pushing scripts into the UNKNOWN version (for WhatsApp) this is unlikely to matter, but for scripts encountered later this should avoid false negatives.

More granular filter types

Until we started revisiting this code a few versions ago, the "empty string" filter type was doing a lot of heavy lifting. I've added several specific filter types:

• MANIFEST_NOT_LOADED: Only used as a placeholder for the initial setup of filterTypeByVersion.
• MANIFEST_LOADED: only used for WhatsApp where we don't have separate main and longtail manifests.
• BOTH_MANIFESTS_LOADED: Essentially the old BOTH behavior, but now only used for apps with both manifests.
• ANY_FILTER: Used for intercepted background scripts. This doesn't actually change any behavior, but rather clarifies that we're making an assumption that we can process the script that isn't guaranteed to be true.

Don't mutate scripts when transferring them to FOUND_SCRIPTS

For WhatsApp, we don't know the versions of scripts we encounter until we've processed the manifest so we store them in an unknown version queue. When we process the manifest we move those scripts with unknown versions into the queue of scripts for that manifest's version.

Previously when we did this we would mutate the ScriptDetails objects so that they would also have the filterType for that manifest. Rather than do this mutation, the new MANIFEST_LOADED filter will mean these scripts will always just wait until a manifest is loaded to start processing. This is probably cleaner in either chase, but was necessary because currentFilterType is no longer available globally.

Clarifications

• Renamed otherType to filterTypeRequiredToProcess everywhere to be more descriptive of how it is used.
• Renamed UNINITIALIZED to UNKNOWN since it's slightly more accurate to what's happening.

Testing

• Automated tests pass.
• Manually tested Chrome and got green checks for all apps.

[meta-cla]

Hi @aselbie!

Thank you for your pull request.

We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!