Warn instead of error when service_worker is combined with background.scripts/page

[willdurand]

Fixes #5152

┆Issue is synchronized with this Jira Task

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8eeab65) 98.73% compared to head (04435f7) 98.73%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5156   +/-   ##
=======================================
  Coverage   98.73%   98.73%           
=======================================
  Files          54       54           
  Lines        2927     2932    +5     
  Branches      891      893    +2     
=======================================
+ Hits         2890     2895    +5     
  Misses         37       37           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rpl]

@willdurand lgtm, in the comments below just a nit for an assertion currently left as commented + some details about that schema update file history (at least the amount of history that I could gather without spending too much time on it).

[rpl]

@willdurand as we were discussing earlier today this property was being merged only into the first entry of the background anyOf schema definitions and so it was allowing only background.page along with other unknown properties but not for background.scripts, which clearly looked odd and not what the original intention likely was and so I did some archeology to track back what happened and if there was any historical reason for only apply that change to the first entry.

Turned out that originally "additionalProperties": false was merged into both the anyOf entries and applied to both background.page and background.scripts entries as we were both expecting to be more likely the original intention, then we lost the background.scripts one long long time ago due to (pretty sure unvoluntary mistake) this patch 8e7f2bc#diff-00f4beb517c238ef6210e32350012fa0eec24b2d2834731494c108c4136a17ac

As for the "wait forcing it to be strict", I couldn't track that back, the original manifest.json update file was introduced by #1169, and I don't see that much comments I could use to track back the motivation quickly, but my guess would be that we were keeping the same strictness we were using on the Firefox side.

and so (even if non blocking for this specific PR), as we were considering earlier today, leaving a note about how these updates file should be maintained (and suggest to cover new ones with a unit test, at least one testing the resulting merged schema) to avoid the same kind of mistake going unnoticed may be useful, but I'm not sure were that note would better fit in this repo, maybe a README.md file in this src/schema/updates/ source dir?

[rpl]

@willdurand forgot to mention, there is a typo in the commit message (pesent -> present), I took care of fixing the one in the PR subject message derived from the commit message

[Rob--W]

LGTM, thanks for the fix.

Note: the commit title states that its purpose is to emit a warning. That's not the objective: instead, the goal is to get rid of the hard error. Downgrading the error to a warning meets that goal, so this patch is indeed a right fix.

I suggest to change the commit message to: Warn instead of error when service_worker is combined with background.scripts/page