Attachment 360804 Details for Bug 194122 – Patch

[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:

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; }

Actions: View | Formatted Diff | Diff

Attachments on bug 194122: 360804 | 360874 | 360875 | 360885 | 360891 | 360895 | 360896 | 360906