Expanding OWNERS review of web-exposed surface area?

[cha...@chromium.org]

From an older thread (OWNERS enforcement of launch process), there seemed to be consensus that there is value in API owners approving changes to the web-exposed surface area. The primary mechanism being API owners review of changes to /blink/web_tests/virtual/stable/webexposed/ (and similar for service workers).

As in crbug.com/813087, the same setup is not in place for exposed surface area in Worklets. That seems like a gap we should close.  The initial engineering effort to implement doesn't seem significant.

The bigger question is whether the ongoing cost (API owners review of file changes, Chrome engineers making changes) is worth it? I don't know if we've had any accidental exposure in worklets to date, or how many APIs are worklet-only (i.e. wouldn't be caught by the existing webexposed tests for window/workers).

I assume the ongoing cost would be worth it to put the process in place. I'd guess the overhead should be minimal (we don't ship an exposed change that often), and the risk of accidental exposure is hard to quantify so we'd want a safeguard in place.

What do the owners think?

Thanks,

Jason

[Alex Russell]

Sorry for the slow follow-up. Was there resolution on this point?

If there wasn't, having a safeguard in place for this seems reasonable to me.

--
You received this message because you are subscribed to the Google Groups "blink-api-owners-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-api-owners-d...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-api-owners-discuss/52c12257-5214-4d69-8a47-b2498314b2dc%40chromium.org.

[Chris Harrelson]

I agree, this safeguard should be added. 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-api-owners-discuss/CANr5HFW91M4sp0D__9XPM_ffNj2CJJLvZ9-SCfyMo6icYveGJA%40mail.gmail.com.

[Rick Byers]

I've seen a couple accidental exposures over the years, and had at least a few cases where reviewing webexposed changes have prevented accidental exposure / misunderstanding of the process. But I haven't personally seen a very high rate (probably still well below the more inherent problem of just shipping bugs that accidentally break websites). So I think we should keep the process overhead to the bare minimum of what's easy to reliably check and quickly review. As long as API owners are doing these reviews relatively quickly, then the existing model of webexposed/ (which I'd argue SHOULD include Worklets - that's likely just an oversight) is a good balance.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-api-owners-discuss/CA%2BN6QZtNAQB%2BG91Re8%2B5X21szL9uEi6tdytbWUsnKpGc7Ygmjg%40mail.gmail.com.

[cha...@chromium.org]

Ok, that sounds like quorum that it's worth extending the existing process to Worklets. The Feature Control team will keep crbug.com/813087 on our backlog. It's not prioritized for this quarter. From this thread, I'd say it's worth doing, but not urgent.

Thanks,

Jason

On Thursday, October 31, 2019 at 11:25:46 AM UTC-4, Rick Byers wrote:

I've seen a couple accidental exposures over the years, and had at least a few cases where reviewing webexposed changes have prevented accidental exposure / misunderstanding of the process. But I haven't personally seen a very high rate (probably still well below the more inherent problem of just shipping bugs that accidentally break websites). So I think we should keep the process overhead to the bare minimum of what's easy to reliably check and quickly review. As long as API owners are doing these reviews relatively quickly, then the existing model of webexposed/ (which I'd argue SHOULD include Worklets - that's likely just an oversight) is a good balance.

On Wed, Oct 30, 2019 at 10:37 PM Chris Harrelson <chri...@chromium.org> wrote:

I agree, this safeguard should be added. 

On Thu, Oct 24, 2019 at 11:50 AM 'Alex Russell' via blink-api-owners-discuss <blink-api-owners-discuss@chromium.org> wrote:

Sorry for the slow follow-up. Was there resolution on this point?

If there wasn't, having a safeguard in place for this seems reasonable to me.

On Fri, Aug 23, 2019 at 9:15 AM <cha...@chromium.org> wrote:

From an older thread (OWNERS enforcement of launch process), there seemed to be consensus that there is value in API owners approving changes to the web-exposed surface area. The primary mechanism being API owners review of changes to /blink/web_tests/virtual/stable/webexposed/ (and similar for service workers).

As in crbug.com/813087, the same setup is not in place for exposed surface area in Worklets. That seems like a gap we should close.  The initial engineering effort to implement doesn't seem significant.

The bigger question is whether the ongoing cost (API owners review of file changes, Chrome engineers making changes) is worth it? I don't know if we've had any accidental exposure in worklets to date, or how many APIs are worklet-only (i.e. wouldn't be caught by the existing webexposed tests for window/workers).

I assume the ongoing cost would be worth it to put the process in place. I'd guess the overhead should be minimal (we don't ship an exposed change that often), and the risk of accidental exposure is hard to quantify so we'd want a safeguard in place.

What do the owners think?

Thanks,

Jason

--
You received this message because you are subscribed to the Google Groups "blink-api-owners-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-api-owners-discuss+unsub...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-api-owners-discuss/52c12257-5214-4d69-8a47-b2498314b2dc%40chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-api-owners-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-api-owners-discuss+unsub...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-api-owners-discuss/CANr5HFW91M4sp0D__9XPM_ffNj2CJJLvZ9-SCfyMo6icYveGJA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-api-owners-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-api-owners-discuss+unsub...@chromium.org.

[Johann Hofmann]

FYI I've recently landed the final piece of work to resolve crbug.com/813087 and we should now have full webexposed coverage for Worklets including API owners review for stable, and some mechanisms to ensure that we also catch new Worklets being exposed. Thank you Jason and Domenic for the help/reviews!

Johann

On Thursday, October 31, 2019 at 9:05:22 PM UTC+1 Jason Chase wrote:

Ok, that sounds like quorum that it's worth extending the existing process to Worklets. The Feature Control team will keep crbug.com/813087 on our backlog. It's not prioritized for this quarter. From this thread, I'd say it's worth doing, but not urgent.

Thanks,

Jason

On Thursday, October 31, 2019 at 11:25:46 AM UTC-4, Rick Byers wrote:

I've seen a couple accidental exposures over the years, and had at least a few cases where reviewing webexposed changes have prevented accidental exposure / misunderstanding of the process. But I haven't personally seen a very high rate (probably still well below the more inherent problem of just shipping bugs that accidentally break websites). So I think we should keep the process overhead to the bare minimum of what's easy to reliably check and quickly review. As long as API owners are doing these reviews relatively quickly, then the existing model of webexposed/ (which I'd argue SHOULD include Worklets - that's likely just an oversight) is a good balance.

On Wed, Oct 30, 2019 at 10:37 PM Chris Harrelson <chri...@chromium.org> wrote:

I agree, this safeguard should be added.