WebKit Bugzilla
Attachment 360804 Details for
Bug 194122
: Capture state should be managed consistently when doing process swapping
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-194122-20190131164047.patch (text/plain), 12.06 KB, created by
youenn fablet
on 2019-01-31 16:40:48 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
youenn fablet
Created:
2019-01-31 16:40:48 PST
Size:
12.06 KB
patch
obsolete
>Subversion Revision: 240633 >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index 163bcae1cfb85699e34e98f57dadebfc54a8bc82..b6a527e119eb9865d78a089bce9f736ffacaf05e 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,35 @@ >+2019-01-31 Youenn Fablet <youenn@apple.com> >+ >+ Capture state should be managed consistently when doing process swapping >+ https://bugs.webkit.org/show_bug.cgi?id=194122 >+ <rdar://problem/47609293> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ When doing PSON, WebPageProxy::resetState is called. >+ It resets the media state, but does not call the client delegates. >+ Instead of directly update the media state, call the routine used to update it so that client delegates are called. >+ >+ Additionally, UserMediaPermissionRequestManagerProxy were kept in a map whose key was the process attached to the web page proxy. >+ Given the process might change, this may mess up with the map and lead to inconsistent state. >+ Instead, keep a HashSet of all manager proxies and iterate over them whenever needed. >+ >+ * UIProcess/UserMediaPermissionRequestManagerProxy.cpp: >+ (WebKit::UserMediaPermissionRequestManagerProxy::UserMediaPermissionRequestManagerProxy): >+ (WebKit::UserMediaPermissionRequestManagerProxy::~UserMediaPermissionRequestManagerProxy): >+ * UIProcess/UserMediaPermissionRequestManagerProxy.h: >+ * UIProcess/UserMediaProcessManager.cpp: >+ (WebKit::UserMediaProcessManager::muteCaptureMediaStreamsExceptIn): >+ (WebKit::UserMediaProcessManager::endedCaptureSession): >+ (WebKit::UserMediaProcessManager::setCaptureEnabled): >+ (WebKit::UserMediaProcessManager::captureDevicesChanged): >+ * UIProcess/UserMediaProcessManager.h: >+ * UIProcess/WebPageProxy.cpp: >+ (WebKit::WebPageProxy::resetState): >+ (WebKit::WebPageProxy::isPlayingMediaDidChange): >+ (WebKit::WebPageProxy::updatePlayingMediaDidChange): >+ * UIProcess/WebPageProxy.h: >+ > 2019-01-31 Youenn Fablet <youenn@apple.com> > > Add an API test to cover UIClient checkUserMediaPermissionForOrigin being nullptr >diff --git a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp >index d5a1c4692348a2407c013aa36fe841ec889a1732..010c17ddb4f46811a0579cca1999cdf323091fb0 100644 >--- a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp >+++ b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp >@@ -51,22 +51,27 @@ static uint64_t generateRequestID() > } > #endif > >+HashSet<UserMediaPermissionRequestManagerProxy*>& UserMediaPermissionRequestManagerProxy::allManagers() >+{ >+ static NeverDestroyed<HashSet<UserMediaPermissionRequestManagerProxy*>> managers; >+ return managers; >+} >+ > UserMediaPermissionRequestManagerProxy::UserMediaPermissionRequestManagerProxy(WebPageProxy& page) > : m_page(page) > , m_rejectionTimer(RunLoop::main(), this, &UserMediaPermissionRequestManagerProxy::rejectionTimerFired) > , m_watchdogTimer(RunLoop::main(), this, &UserMediaPermissionRequestManagerProxy::watchdogTimerFired) > { >-#if ENABLE(MEDIA_STREAM) >- UserMediaProcessManager::singleton().addUserMediaPermissionRequestManagerProxy(*this); >-#endif >+ allManagers().add(this); > } > > UserMediaPermissionRequestManagerProxy::~UserMediaPermissionRequestManagerProxy() > { > #if ENABLE(MEDIA_STREAM) >- UserMediaProcessManager::singleton().removeUserMediaPermissionRequestManagerProxy(*this); >+ UserMediaProcessManager::singleton().endedCaptureSession(*this); > #endif > invalidatePendingRequests(); >+ allManagers().remove(this); > } > > void UserMediaPermissionRequestManagerProxy::invalidatePendingRequests() >diff --git a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h >index eacce6f92b65ffcc29be4d13788da81d91590558..fe4bff8c23050b8935a7b1c1927f26d4f42bd501 100644 >--- a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h >+++ b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h >@@ -44,6 +44,8 @@ public: > explicit UserMediaPermissionRequestManagerProxy(WebPageProxy&); > ~UserMediaPermissionRequestManagerProxy(); > >+ static HashSet<UserMediaPermissionRequestManagerProxy*>& allManagers(); >+ > WebPageProxy& page() const { return m_page; } > > void invalidatePendingRequests(); >diff --git a/Source/WebKit/UIProcess/UserMediaProcessManager.cpp b/Source/WebKit/UIProcess/UserMediaProcessManager.cpp >index addb0351722dfc00ba52707d29929d95af5fa1cc..4b968558a0474e96d07037e9e25b1a1f6f7f6f72 100644 >--- a/Source/WebKit/UIProcess/UserMediaProcessManager.cpp >+++ b/Source/WebKit/UIProcess/UserMediaProcessManager.cpp >@@ -44,10 +44,6 @@ public: > ProcessState() { } > ProcessState(const ProcessState&) = delete; > >- void addRequestManager(UserMediaPermissionRequestManagerProxy&); >- void removeRequestManager(UserMediaPermissionRequestManagerProxy&); >- Vector<UserMediaPermissionRequestManagerProxy*>& managers() { return m_managers; } >- > enum SandboxExtensionType : uint32_t { > None = 0, > Video = 1 << 0, >@@ -64,8 +60,6 @@ public: > void revokeAudioExtension() { m_pageSandboxExtensionsGranted &= ~Audio; } > > private: >- >- Vector<UserMediaPermissionRequestManagerProxy*> m_managers; > SandboxExtensionsGranted m_pageSandboxExtensionsGranted { SandboxExtensionType::None }; > }; > >@@ -85,20 +79,6 @@ static ProcessState& processState(WebProcessProxy& process) > return *state; > } > >-void ProcessState::addRequestManager(UserMediaPermissionRequestManagerProxy& proxy) >-{ >- ASSERT(!m_managers.contains(&proxy)); >- m_managers.append(&proxy); >-} >- >-void ProcessState::removeRequestManager(UserMediaPermissionRequestManagerProxy& proxy) >-{ >- ASSERT(m_managers.contains(&proxy)); >- m_managers.removeFirstMatching([&proxy](auto other) { >- return other == &proxy; >- }); >-} >- > UserMediaProcessManager& UserMediaProcessManager::singleton() > { > static NeverDestroyed<UserMediaProcessManager> manager; >@@ -110,32 +90,13 @@ UserMediaProcessManager::UserMediaProcessManager() > { > } > >-void UserMediaProcessManager::addUserMediaPermissionRequestManagerProxy(UserMediaPermissionRequestManagerProxy& proxy) >-{ >- processState(proxy.page().process()).addRequestManager(proxy); >-} >- >-void UserMediaProcessManager::removeUserMediaPermissionRequestManagerProxy(UserMediaPermissionRequestManagerProxy& proxy) >-{ >- endedCaptureSession(proxy); >- >- auto& state = processState(proxy.page().process()); >- state.removeRequestManager(proxy); >- if (state.managers().isEmpty()) { >- auto it = stateMap().find(&proxy.page().process()); >- stateMap().remove(it); >- } >-} >- > void UserMediaProcessManager::muteCaptureMediaStreamsExceptIn(WebPageProxy& pageStartingCapture) > { > #if PLATFORM(COCOA) >- for (auto& state : stateMap()) { >- for (auto& manager : state.value->managers()) { >- if (&manager->page() == &pageStartingCapture) >- continue; >- manager->page().setMediaStreamCaptureMuted(true); >- } >+ for (auto manager : UserMediaPermissionRequestManagerProxy::allManagers()) { >+ if (&manager->page() == &pageStartingCapture) >+ continue; >+ manager->page().setMediaStreamCaptureMuted(true); > } > #else > UNUSED_PARAM(pageStartingCapture); >@@ -218,13 +179,12 @@ void UserMediaProcessManager::startedCaptureSession(UserMediaPermissionRequestMa > void UserMediaProcessManager::endedCaptureSession(UserMediaPermissionRequestManagerProxy& proxy) > { > #if ENABLE(SANDBOX_EXTENSIONS) >- ASSERT(stateMap().contains(&proxy.page().process())); >+ auto& process = proxy.page().process(); > >- auto& state = processState(proxy.page().process()); > bool hasAudioCapture = false; > bool hasVideoCapture = false; >- for (auto& manager : state.managers()) { >- if (manager == &proxy) >+ for (auto manager : UserMediaPermissionRequestManagerProxy::allManagers()) { >+ if (manager == &proxy || &manager->page().process() != &process) > continue; > if (manager->page().hasActiveAudioStream()) > hasAudioCapture = true; >@@ -235,6 +195,9 @@ void UserMediaProcessManager::endedCaptureSession(UserMediaPermissionRequestMana > if (hasAudioCapture && hasVideoCapture) > return; > >+ ASSERT(stateMap().contains(&process)); >+ auto& state = processState(process); >+ > Vector<String> params; > if (!hasAudioCapture && state.hasAudioExtension()) { > params.append(audioExtensionPath); >@@ -265,23 +228,14 @@ void UserMediaProcessManager::setCaptureEnabled(bool enabled) > if (enabled) > return; > >- for (auto& state : stateMap()) { >- for (auto& manager : state.value->managers()) >- manager->stopCapture(); >- } >+ for (auto manager : UserMediaPermissionRequestManagerProxy::allManagers()) >+ manager->stopCapture(); > } > > void UserMediaProcessManager::captureDevicesChanged() > { >- auto& map = stateMap(); >- for (auto& state : map) { >- auto* process = state.key; >- for (auto& manager : state.value->managers()) { >- if (map.find(process) == map.end()) >- break; >- manager->captureDevicesChanged(); >- } >- } >+ for (auto manager : UserMediaPermissionRequestManagerProxy::allManagers()) >+ manager->captureDevicesChanged(); > } > > void UserMediaProcessManager::beginMonitoringCaptureDevices() >diff --git a/Source/WebKit/UIProcess/UserMediaProcessManager.h b/Source/WebKit/UIProcess/UserMediaProcessManager.h >index ae16ebed85d80bfdcac05ec6b80c7643b8ef528b..04f1c2973d67d1d3b9be8122c8e26d968ab95623 100644 >--- a/Source/WebKit/UIProcess/UserMediaProcessManager.h >+++ b/Source/WebKit/UIProcess/UserMediaProcessManager.h >@@ -35,9 +35,6 @@ public: > > UserMediaProcessManager(); > >- void addUserMediaPermissionRequestManagerProxy(UserMediaPermissionRequestManagerProxy&); >- void removeUserMediaPermissionRequestManagerProxy(UserMediaPermissionRequestManagerProxy&); >- > bool willCreateMediaStream(UserMediaPermissionRequestManagerProxy&, bool withAudio, bool withVideo); > void muteCaptureMediaStreamsExceptIn(WebPageProxy&); > >diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp >index c2a4d6a1902ab364d021976036858b2de863c8a8..3c4c8649604cecce998b3598100871a54512ed25 100644 >--- a/Source/WebKit/UIProcess/WebPageProxy.cpp >+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp >@@ -6619,7 +6619,7 @@ void WebPageProxy::resetState(ResetStateReason resetStateReason) > editCommand->invalidate(); > > m_activePopupMenu = nullptr; >- m_mediaState = MediaProducer::IsNotPlaying; >+ updatePlayingMediaDidChange(MediaProducer::IsNotPlaying); > > #if ENABLE(POINTER_LOCK) > requestPointerUnlock(); >@@ -7677,7 +7677,11 @@ void WebPageProxy::isPlayingMediaDidChange(MediaProducer::MediaStateFlags newSta > ASSERT(focusManager); > focusManager->updatePlaybackAttributesFromMediaState(this, sourceElementID, newState); > #endif >+ updatePlayingMediaDidChange(newState); >+} > >+void WebPageProxy::updatePlayingMediaDidChange(MediaProducer::MediaStateFlags newState) >+{ > if (newState == m_mediaState) > return; > >diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h >index 6faacb86633a522055b067bb872a2be92dad91fa..5b90620bf94bef3e61792ca7bf194a14dfb72042 100644 >--- a/Source/WebKit/UIProcess/WebPageProxy.h >+++ b/Source/WebKit/UIProcess/WebPageProxy.h >@@ -1238,6 +1238,7 @@ public: > > bool isPlayingAudio() const { return !!(m_mediaState & WebCore::MediaProducer::IsPlayingAudio); } > void isPlayingMediaDidChange(WebCore::MediaProducer::MediaStateFlags, uint64_t); >+ void updatePlayingMediaDidChange(WebCore::MediaProducer::MediaStateFlags newState); > bool hasActiveAudioStream() const { return m_mediaState & WebCore::MediaProducer::HasActiveAudioCaptureDevice; } > bool hasActiveVideoStream() const { return m_mediaState & WebCore::MediaProducer::HasActiveVideoCaptureDevice; } > WebCore::MediaProducer::MediaStateFlags mediaStateFlags() const { return m_mediaState; }
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194122
:
360804
|
360874
|
360875
|
360885
|
360891
|
360895
|
360896
|
360906