LayoutTests/webexposed - coverage for SecureContext (vs. not)

[Joshua Bell]

Background: We have a suite of tests in webexposed/ that enumerate all interfaces/methods/properties exposed to the web; when we add a new API it shows up there. Sign-off by the Blink API owners is needed for changes to the virtual/stable/webexposed so they can ensure that new APIs went through the "Intent to Ship" process. There's also a set of so-called "powerful features" that for various reasons we have decided to expose only in secure contexts (roughly: https:, file: or http://localhost), and this set is growing.

Problem: The default webexposed/global-interface-listing* tests run in a secure context (file:) so interfaces/methods/attributes marked with [SecureContext] always appear. Put another way, adding or removing [SecureContext] on an interface doesn't cause such tests to fail, which is a bit of a surprise.

We can have API-specific tests, either along with the rest of the API's tests, or in http/tests/security/powerfulFeatureRestrictions/ but it seems like we should have general coverage to prevent accidents.

Solution:  I have a CL up at https://chromium-review.googlesource.com/c/chromium/src/+/804400 that adds http/tests/webexposed/nonsecure which runs the tests in a non-secure context. 

Discussion: This adds work to (1) anyone shipping new APIs and (2) the Blink API owners as now there's Yet Another set of tests to update. We'll end up with:

webexposed/

http/tests/webexposed/nonsecure

virtual/stable/webexposed/

virtual/stable/http/tests/webexposed/nonsecure

(There are also variants of this for service workers and worklets, although since both of those implicitly require secure contexts we don't need two copies.)

As an aside: my sense of symmetry makes me want to relocate webexposed/ to http/tests/webexposed/secure (bonus: the tests would run under http: rather than file: contexts, which more accurately matches the web, although I'm not aware of any differences.)

Feedback?

[Hiroki Nakagawa]

Looks good from workers/worklets POV. I made a comment on the CL.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAD649j5cGfxvp7cAaDtyd%2Bsh18VBZM9QU1ZkNqa5dR1KimLRPg%40mail.gmail.com.

[Rick Byers]

+feature-control team who own these tests.

My argument in the past has been that there's diminishing returns from expanding the coverage of these tests.  They're just designed to help catch mistakes, not be exhaustive (eg. they don't try to catch changes in argument counts, types, etc.).  Doubling the number of expectation files can be a non-trivial cost in practice.

In this case the value would just be in catching cases where a new API was somehow added to INSECURE contexts only, or an existing API accidentally made secure-only.  Can you find an example  when such a  mistake occurred which could have been caught? Those seem unlikely to me, on the order of other bugs that could occur (eg. an API made to always throw).  Personally I don't see the value in adding complexity to the tests for this.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABj5diPntf6XCiQLwiTq8cd7_wxZztcTTpJRuPzDN0hgh_Nvrg%40mail.gmail.com.