Changeset 249712 in webkit

Timestamp:

Sep 10, 2019 6:52:37 AM (5 years ago)

Author:

youenn@apple.com

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

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/UserMediaPermissionRequestManagerProxy.cpp (modified) (1 diff)
• UIProcess/UserMediaPermissionRequestManagerProxy.h (modified) (1 diff)
• UIProcess/UserMediaProcessManager.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-09-10  Youenn Fablet  <youenn@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  <cdumez@apple.com>
 

trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp

@@ -242 +242 @@
         if (!weakThis)
             return;
-        --m_hasPendingCapture;
+        if (!--m_hasPendingCapture)
+            UserMediaProcessManager::singleton().revokeSandboxExtensionsIfNeeded(page().process());
     }, m_page.webPageID());
 

trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h

@@ -88 +88 @@
 
     void setMockCaptureDevicesEnabledOverride(Optional<bool> enabled) { m_mockDevicesEnabledOverride = enabled; }
+    bool hasPendingCapture() const { return m_hasPendingCapture; }
 
 private:

trunk/Source/WebKit/UIProcess/UserMediaProcessManager.cpp

@@ -131 +131 @@
     bool hasAudioCapture = false;
     bool hasVideoCapture = false;
-
-    UserMediaPermissionRequestManagerProxy::forEach([&hasAudioCapture, &hasVideoCapture, &process](auto& managerProxy) {
+    bool hasPendingCapture = false;
+
+    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)