We should activate ICE candidate privacy policy by default and find a way to disable it when needed.
Created attachment 302956 [details] Patch
This patch is a second step towards implementing privacy policy. We should add tests and probably reactivate filtering in some cases. For instance when access is being revoked from the address bar.
Comment on attachment 302956 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=302956&action=review > Source/WebKit2/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:209 > +#if ENABLE(WEB_RTC) && USE(LIBWEBRTC) > + m_page.process().send(Messages::WebPage::DisableICECandidateFiltering(), m_page.pageID()); > +#endif I don't think this should be done here, because it will disable filtering if the user accepts one stream and denies another. Instead, I think it should be in UserMediaProcessManager::willCreateMediaStream and UserMediaProcessManager::endedCaptureSession.
Created attachment 303074 [details] Patch
(In reply to comment #3) > Comment on attachment 302956 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=302956&action=review > > > Source/WebKit2/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:209 > > +#if ENABLE(WEB_RTC) && USE(LIBWEBRTC) > > + m_page.process().send(Messages::WebPage::DisableICECandidateFiltering(), m_page.pageID()); > > +#endif > > I don't think this should be done here, because it will disable filtering if > the user accepts one stream and denies another. Instead, I think it should > be in UserMediaProcessManager::willCreateMediaStream and > UserMediaProcessManager::endedCaptureSession. Thanks for the feedback. I updated patch accordingly. I also added a layout test for it.
Comment on attachment 303074 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=303074&action=review > Source/WebKit2/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:205 > +#if ENABLE(WEB_RTC) && USE(LIBWEBRTC) > + m_page.process().send(Messages::WebPage::DisableICECandidateFiltering(), m_page.pageID()); > +#endif Don't you want to remove this? > Source/WebKit2/UIProcess/UserMediaProcessManager.cpp:159 > +#if ENABLE(WEB_RTC) && USE(LIBWEBRTC) > + if (currentExtensions == ProcessState::SandboxExtensionsGranted::None) > + proxy.page().process().send(Messages::WebPage::DisableICECandidateFiltering(), proxy.page().pageID()); > +#endif Shouldn't this be in UserMediaProcessManager::endedCaptureSession?
Created attachment 303143 [details] Patch
(In reply to comment #6) > Comment on attachment 303074 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=303074&action=review > > > Source/WebKit2/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:205 > > +#if ENABLE(WEB_RTC) && USE(LIBWEBRTC) > > + m_page.process().send(Messages::WebPage::DisableICECandidateFiltering(), m_page.pageID()); > > +#endif > > Don't you want to remove this? Right > > Source/WebKit2/UIProcess/UserMediaProcessManager.cpp:159 > > +#if ENABLE(WEB_RTC) && USE(LIBWEBRTC) > > + if (currentExtensions == ProcessState::SandboxExtensionsGranted::None) > > + proxy.page().process().send(Messages::WebPage::DisableICECandidateFiltering(), proxy.page().pageID()); > > +#endif > > Shouldn't this be in UserMediaProcessManager::endedCaptureSession? We want to disable filtering when capture is authorised, so I think this is ok like this. I reworked a bit the way this is handled to ensure we got good coverage here.
Eric, could you check that the granting/revocation goes well with the disable/enable filtering? Alex, one thing to consider is that we probably do not want these preferences to stay persistent WebKit launch after WebKit launch. Maybe more a Safari issue though but this may be the two-only preferences that act like this currently.
Comment on attachment 303143 [details] Patch Clearing flags on attachment: 303143 Committed r213283: <http://trac.webkit.org/changeset/213283>
All reviewed patches have been landed. Closing bug.