Issue 13668 - Add disable WebRTC to Shields

[jumde]

Fix for: #13668

Test Plan:

1. Visit https://www.expressvpn.com/webrtc-leak-test to verify IP address is not leaked.
2. Toggle Site Shields and WebRTC preferences in about:preferences to verify WebRTC works when WebRTC is enabled

[codecov-io]

Codecov Report

Merging #13674 into master will increase coverage by <.01%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master   #13674      +/-   ##
==========================================
+ Coverage   56.65%   56.65%   +<.01%     
==========================================
  Files         285      285              
  Lines       28917    28919       +2     
  Branches     4781     4781              
==========================================
+ Hits        16383    16385       +2     
  Misses      12534    12534

Flag	Coverage Δ
#unittest	56.65% <66.66%> (ø)	⬆️

Impacted Files	Coverage Δ
js/constants/appConfig.js	100% <ø> (ø)	⬆️
app/browser/reducers/tabsReducer.js	40.27% <100%> (+0.27%)	⬆️
app/renderer/components/preferences/shieldsTab.js	47.69% <50%> (+0.07%)	⬆️

[diracdeltas]

the original issue is for disabling webrtc in preferences: #13668. i think it makes more sense to do it there instead of in site settings to avoid adding more switches to site settings. cc @bradleyrichter

[tildelowengrimm]

I'm not sure which way I lean on this. I think it more likely that folks generally don't use WebRTC, and definitely don't want it usable as a background tracking vector. But if there are a couple of sites where I do use WebRTC, I probably don't want to leave it on in the background for everyone? Could we do it as a permission request rather than a shields toggle?

[diracdeltas]

But if there are a couple of sites where I do use WebRTC, I probably don't want to leave it on in the background for everyone?

You wouldn't need to do that if we add webRTC as a global toggle but keep the current behavior where it's also tied to fingerprinting protection. If webrtc is globally disabled but you allow fingerprinting on a site, the latter should win so webrtc works on only that site

[tildelowengrimm]

Yeah, that works pretty well. Without any sort of prompt, it relies on the user behavior of "This site is broken somehow…" → try toggling fingerprinting protection. That pushes us in the direction of erring on the side of breaking more sites rather than risking unexpected tracking, but I'm okay with that.

[diracdeltas]

@flamsmark actually my proposal doesn't work for Brendan's use case (user needs to disable FP on a site to use a captcha slider, but they don't want to allow webrtc) if the user changes FP to 'allow all' on a site. another option is we could add a 4th setting to the FP select menu that is Allow all fingerprinting except webrtc

[tildelowengrimm]

@yan This UX that we're armchairing is getting pretty gnarly. Might benefit from some pro design work.

[bradleyrichter]

Over time as we reach a broader audience, we need to be careful to decrease the amount of default site breakage. Providing shield switches for advanced users that are off by default may be OK on going. We are going to roll-up the advanced choices in a near-future release. This will guide most users to use the shields master switch to fix a broken site.

we have more advanced toggles on deck as well:

• block all images
• block (and unblock) popups

That being said, we should still carefully consider adding per-site switches and assume people will not be able to understand any switch dependancies.

[diracdeltas]

discussed in slack; we are just going to add a global option to never enable webrtc for now.