Title: [249712] trunk/Source/WebKit
Revision
249712
Author
you...@apple.com
Date
2019-09-10 06:52:37 -0700 (Tue, 10 Sep 2019)

Log Message

UserMediaProcessManager is revoking sandbox extensions too aggressively
https://bugs.webkit.org/show_bug.cgi?id=201638

Reviewed by Eric Carlson.

Sandbox revocation was sometimes happening when a page is being closed while another page from the same process is starting capture.
In that case, revocation might happen while it should not.
To prevent this, we do not revoke sandbox extensions if there are pending captures for a page of the process.
Whenever a page does not have any pending capture, sandbox extensions may be revoked.

Covered by OnDeviceChangeCrash API test in debug mode.

* UIProcess/UserMediaPermissionRequestManagerProxy.cpp:
(WebKit::UserMediaPermissionRequestManagerProxy::finishGrantingRequest):
* UIProcess/UserMediaPermissionRequestManagerProxy.h:
(WebKit::UserMediaPermissionRequestManagerProxy::hasPendingCapture const):
* UIProcess/UserMediaProcessManager.cpp:
(WebKit::UserMediaProcessManager::revokeSandboxExtensionsIfNeeded):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (249711 => 249712)


--- trunk/Source/WebKit/ChangeLog	2019-09-10 12:47:07 UTC (rev 249711)
+++ trunk/Source/WebKit/ChangeLog	2019-09-10 13:52:37 UTC (rev 249712)
@@ -1,3 +1,24 @@
+2019-09-10  Youenn Fablet  <you...@apple.com>
+
+        UserMediaProcessManager is revoking sandbox extensions too aggressively
+        https://bugs.webkit.org/show_bug.cgi?id=201638
+
+        Reviewed by Eric Carlson.
+
+        Sandbox revocation was sometimes happening when a page is being closed while another page from the same process is starting capture.
+        In that case, revocation might happen while it should not.
+        To prevent this, we do not revoke sandbox extensions if there are pending captures for a page of the process.
+        Whenever a page does not have any pending capture, sandbox extensions may be revoked.
+
+        Covered by OnDeviceChangeCrash API test in debug mode.
+
+        * UIProcess/UserMediaPermissionRequestManagerProxy.cpp:
+        (WebKit::UserMediaPermissionRequestManagerProxy::finishGrantingRequest):
+        * UIProcess/UserMediaPermissionRequestManagerProxy.h:
+        (WebKit::UserMediaPermissionRequestManagerProxy::hasPendingCapture const):
+        * UIProcess/UserMediaProcessManager.cpp:
+        (WebKit::UserMediaProcessManager::revokeSandboxExtensionsIfNeeded):
+
 2019-09-09  Chris Dumez  <cdu...@apple.com>
 
         [iOS] We sometimes attempt to use a terminated prewarmed WebContent process

Modified: trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp (249711 => 249712)


--- trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp	2019-09-10 12:47:07 UTC (rev 249711)
+++ trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp	2019-09-10 13:52:37 UTC (rev 249712)
@@ -241,7 +241,8 @@
     m_page.process().connection()->sendWithAsyncReply(Messages::WebPage::UserMediaAccessWasGranted { request.userMediaID(), request.audioDevice(), request.videoDevice(), request.deviceIdentifierHashSalt() }, [this, weakThis = makeWeakPtr(this)] {
         if (!weakThis)
             return;
-        --m_hasPendingCapture;
+        if (!--m_hasPendingCapture)
+            UserMediaProcessManager::singleton().revokeSandboxExtensionsIfNeeded(page().process());
     }, m_page.webPageID());
 
     processNextUserMediaRequestIfNeeded();

Modified: trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h (249711 => 249712)


--- trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h	2019-09-10 12:47:07 UTC (rev 249711)
+++ trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h	2019-09-10 13:52:37 UTC (rev 249712)
@@ -87,6 +87,7 @@
     };
 
     void setMockCaptureDevicesEnabledOverride(Optional<bool> enabled) { m_mockDevicesEnabledOverride = enabled; }
+    bool hasPendingCapture() const { return m_hasPendingCapture; }
 
 private:
 #if !RELEASE_LOG_DISABLED

Modified: trunk/Source/WebKit/UIProcess/UserMediaProcessManager.cpp (249711 => 249712)


--- trunk/Source/WebKit/UIProcess/UserMediaProcessManager.cpp	2019-09-10 12:47:07 UTC (rev 249711)
+++ trunk/Source/WebKit/UIProcess/UserMediaProcessManager.cpp	2019-09-10 13:52:37 UTC (rev 249712)
@@ -130,14 +130,19 @@
 #if ENABLE(SANDBOX_EXTENSIONS)
     bool hasAudioCapture = false;
     bool hasVideoCapture = false;
+    bool hasPendingCapture = false;
 
-    UserMediaPermissionRequestManagerProxy::forEach([&hasAudioCapture, &hasVideoCapture, &process](auto& managerProxy) {
+    UserMediaPermissionRequestManagerProxy::forEach([&hasAudioCapture, &hasVideoCapture, &hasPendingCapture, &process](auto& managerProxy) {
         if (&process != &managerProxy.page().process())
             return;
         hasAudioCapture |= managerProxy.page().isCapturingAudio();
         hasVideoCapture |= managerProxy.page().isCapturingVideo();
+        hasPendingCapture |= managerProxy.hasPendingCapture();
     });
 
+    if (hasPendingCapture)
+        return;
+
     if (hasAudioCapture && hasVideoCapture)
         return;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to