[MediaStream] DeviceIdHashSaltStorage should use iframe and top level documents
authoreric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Nov 2018 03:15:03 +0000 (03:15 +0000)
committereric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Nov 2018 03:15:03 +0000 (03:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192182

Reviewed by Youenn Fablet.

Source/WebKit:

* UIProcess/DeviceIdHashSaltStorage.cpp:
(WebKit::DeviceIdHashSaltStorage::deviceIdHashSaltForOrigin): Key off of request and top
level documents.
(WebKit::DeviceIdHashSaltStorage::deleteDeviceIdHashSaltForOrigins): Ditto.
* UIProcess/DeviceIdHashSaltStorage.h:
(WebKit::DeviceIdHashSaltStorage::HashSaltForOrigin::HashSaltForOrigin):

* UIProcess/UserMediaPermissionRequestManagerProxy.cpp:
(WebKit::UserMediaPermissionRequestManagerProxy::userMediaAccessWasGranted): Pass both documents.
(WebKit::UserMediaPermissionRequestManagerProxy::requestUserMediaPermissionForFrame): Ditto.
(WebKit::UserMediaPermissionRequestManagerProxy::enumerateMediaDevicesForFrame): Ditto.

LayoutTests:

* http/tests/media/media-stream/enumerate-devices-source-id-expected.txt:
* http/tests/media/media-stream/enumerate-devices-source-id.html:
* http/tests/media/media-stream/resources/enumerate-devices-source-id-frame.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238724 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/media/media-stream/enumerate-devices-source-id-expected.txt
LayoutTests/http/tests/media/media-stream/enumerate-devices-source-id.html
LayoutTests/http/tests/media/media-stream/resources/enumerate-devices-source-id-frame.html
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/DeviceIdHashSaltStorage.cpp
Source/WebKit/UIProcess/DeviceIdHashSaltStorage.h
Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp

index 1584d32..12eddfe 100644 (file)
@@ -1,3 +1,14 @@
+2018-11-29  Eric Carlson  <eric.carlson@apple.com>
+
+        [MediaStream] DeviceIdHashSaltStorage should use iframe and top level documents
+        https://bugs.webkit.org/show_bug.cgi?id=192182
+
+        Reviewed by Youenn Fablet.
+
+        * http/tests/media/media-stream/enumerate-devices-source-id-expected.txt:
+        * http/tests/media/media-stream/enumerate-devices-source-id.html:
+        * http/tests/media/media-stream/resources/enumerate-devices-source-id-frame.html:
+
 2018-11-29  Christopher Reid  <chris.reid@sony.com>
 
         [Win] listDirectory in FileSystemWin.cpp should not skip all directories
index 9acc1ba..dbe8b09 100644 (file)
@@ -1,25 +1,11 @@
  
  
 
-Tests that mediaDevices.enumerateDevices returns the same value for a device ID in all subframes.
+Tests that mediaDevices.enumerateDevices returns the same ids for devices in the same frame/subframe .
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS : device id 1 is not unique
-PASS : device id 2 is not unique
-PASS : device id 3 is not unique
-PASS : device id 4 is not unique
-PASS : device id 5 is not unique
-PASS : device id 6 is not unique
-PASS : device id 7 is not unique
-PASS : device id 8 is not unique
-PASS : device id 9 is not unique
-PASS : device id 10 is not unique
-PASS : device id 11 is not unique
-PASS : device id 12 is not unique
-
-PASS : device IDs are not unique
 
 PASS successfullyParsed is true
 
index 48e7dfe..250084a 100644 (file)
@@ -6,8 +6,6 @@
         </style>
         <script src="../../../../resources/js-test-pre.js"></script>
         <script>
-            var frameInfos = [];
-            var idCounts = { };
             window.jsTestIsAsync = true;
 
             if (window.testRunner)
 
             function setup()
             {
-                description("Tests that mediaDevices.enumerateDevices returns the same value for a device ID in all subframes.");
+                description("Tests that mediaDevices.enumerateDevices returns the same ids for devices in the same frame/subframe .");
             }
 
-            function handler(event) 
+            let map = { };
+            function checkID(origin, device)
             {
-                event.data.deviceIds.forEach(function(id) {
-                    frameInfos.push({origin : event.data.origin, deviceId : id});
-                    let count = idCounts[id] || 0;
-                    idCounts[id] = ++count;
-                });
+                let origins = `${document.origin}-${origin}`;
 
-                if (frameInfos.length != 12)
-                    return;
+                if (!map[origins])
+                    map[origins] = { };
 
-                var success = true;
-                for (var i = 0; i < frameInfos.length; i++) {
-                    var deviceId = frameInfos[i].deviceId;
-                    let count = idCounts[deviceId] || 0;
-                    if (!count) {
-                        testFailed(`: device id ${i + 1} is unique`);
-                        success = false;
-                    } else
-                        testPassed(`: device id ${i + 1} is not unique`);
-                }
+                let originInfo = map[origins];
+                if (!originInfo[device.deviceId])
+                    originInfo[device.deviceId] = device.kind;
+                
+                if (originInfo[device.deviceId] != device.kind)
+                    testFailed(`: duplicate device IDs for ${device.kind} and ${originInfo[device.deviceId]} in ${origin}/${document.origin}`);
 
-                debug('');
-                if (success)
-                    testPassed(`: device IDs are not unique`);
+                if (Object.keys(originInfo).length > 4)
+                    testFailed(`: more then four unique device IDs in ${origin}/${document.origin}`);
+            }
+
+            let eventCount = 0;
+            async function handler(event) 
+            {
+                event.data.devices.forEach(device => { checkID(event.data.origin, device); });
+
+                if (++eventCount != 6)
+                    return;
+
+                await navigator.mediaDevices.getUserMedia({video: true, audio: true });
+                let devices = await navigator.mediaDevices.enumerateDevices();
+                devices.forEach(device => checkID(document.origin, device));
 
                 debug('');
                 finishJSTest();
         </script> 
     </head>
     <body onload="setup()">
-        <iframe allow="camera;microphone" src="http://localhost:8000/media/media-stream/resources/enumerate-devices-source-id-frame.html"></iframe>
+        <iframe allow="camera;microphone" src="http://localhost:8000/media/media-stream/resources/enumerate-devices-source-id-frame.html?1"></iframe>
         <br>
-        <iframe allow="camera;microphone" src="http://127.0.0.1:8000/media/media-stream/resources/enumerate-devices-source-id-frame.html"></iframe>
+        <iframe allow="camera;microphone" src="http://127.0.0.1:8000/media/media-stream/resources/enumerate-devices-source-id-frame.html?2"></iframe>
         <br>
-        <iframe allow="camera;microphone" src="http://localhost:8000/media/media-stream/resources/enumerate-devices-source-id-frame.html"></iframe>
+        <iframe allow="camera;microphone" src="http://localhost:8000/media/media-stream/resources/enumerate-devices-source-id-frame.html?3"></iframe>
         <div id="console"></div>
         <script src="../../../../resources/js-test-post.js"></script>
     </body>
index cbee8c9..f1c8c95 100644 (file)
                 document.getElementById("console").innerHTML += msg + "<br>";
             }
 
-            function test(event)
+            async function test(event)
             {
-                navigator.mediaDevices.enumerateDevices()
-                    .then(function(devices) {
-                        var result = {
-                            origin: document.origin, 
-                            deviceIds: [devices[0].deviceId, devices[1].deviceId]
-                        };
+                await navigator.mediaDevices.getUserMedia({video: true, audio: true });
+                let devices = await navigator.mediaDevices.enumerateDevices();
+                let result = {
+                    origin: document.origin, 
+                    devices: devices.map(device => { return {deviceId: device.deviceId, kind: device.kind} }),
+                };
 
-                        parent.postMessage(result, "*");
-                    })
-                    .catch(function(err) {
-                        log(`enumerateDevices failed: ${err.name} - ${err.message}`);
-                    });
+                parent.postMessage(result, "*");
                 
-                var parentPage = document.referrer.split('/').pop();
+                let parentPage = document.referrer.split('/').pop();
                 if (parentPage != "enumerate-devices-source-id.html")
                     return;
 
-                var iframe = document.createElement('iframe');
+                let iframe = document.createElement('iframe');
                 iframe.src = location.href;
                 document.body.appendChild(iframe);
             }
 
             function handler(event) 
             {
-                var result = {
-                    origin: `${event.data.origin} in ${document.origin}`, 
-                    deviceIds: event.data.deviceIds
+                let result = {
+                    origin: `${event.data.origin}`, 
+                    devices: event.data.devices
                 };
 
                 parent.postMessage(result, "*");
index 45c5dda..b191b25 100644 (file)
@@ -1,3 +1,22 @@
+2018-11-29  Eric Carlson  <eric.carlson@apple.com>
+
+        [MediaStream] DeviceIdHashSaltStorage should use iframe and top level documents
+        https://bugs.webkit.org/show_bug.cgi?id=192182
+
+        Reviewed by Youenn Fablet.
+
+        * UIProcess/DeviceIdHashSaltStorage.cpp:
+        (WebKit::DeviceIdHashSaltStorage::deviceIdHashSaltForOrigin): Key off of request and top 
+        level documents.
+        (WebKit::DeviceIdHashSaltStorage::deleteDeviceIdHashSaltForOrigins): Ditto.
+        * UIProcess/DeviceIdHashSaltStorage.h:
+        (WebKit::DeviceIdHashSaltStorage::HashSaltForOrigin::HashSaltForOrigin):
+
+        * UIProcess/UserMediaPermissionRequestManagerProxy.cpp:
+        (WebKit::UserMediaPermissionRequestManagerProxy::userMediaAccessWasGranted): Pass both documents.
+        (WebKit::UserMediaPermissionRequestManagerProxy::requestUserMediaPermissionForFrame): Ditto.
+        (WebKit::UserMediaPermissionRequestManagerProxy::enumerateMediaDevicesForFrame): Ditto.
+
 2018-11-29  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r238713.
index 2638a07..fc8582e 100644 (file)
@@ -46,20 +46,21 @@ Ref<DeviceIdHashSaltStorage> DeviceIdHashSaltStorage::create()
     return adoptRef(*new DeviceIdHashSaltStorage());
 }
 
-const String& DeviceIdHashSaltStorage::deviceIdHashSaltForOrigin(const SecurityOrigin& documentOrigin)
+const String& DeviceIdHashSaltStorage::deviceIdHashSaltForOrigin(const SecurityOrigin& documentOrigin, const SecurityOrigin& parentOrigin)
 {
-    auto& deviceIdHashSalt = m_deviceIdHashSaltForOrigins.ensure(documentOrigin.toRawString(), [&documentOrigin] () {
+    auto origins = makeString(documentOrigin.toRawString(), parentOrigin.toRawString());
+    auto& deviceIdHashSalt = m_deviceIdHashSaltForOrigins.ensure(origins, [&documentOrigin, &parentOrigin] () {
         uint64_t randomData[randomDataSize];
         cryptographicallyRandomValues(reinterpret_cast<unsigned char*>(randomData), sizeof(randomData));
 
         StringBuilder builder;
         builder.reserveCapacity(hashSaltSize);
         for (int i = 0; i < randomDataSize; i++)
-            appendUnsigned64AsHex(randomData[0], builder);
+            appendUnsigned64AsHex(randomData[i], builder);
 
         String deviceIdHashSalt = builder.toString();
 
-        return std::make_unique<HashSaltForOrigin>(documentOrigin.data().isolatedCopy(), WTFMove(deviceIdHashSalt));
+        return std::make_unique<HashSaltForOrigin>(documentOrigin.data().isolatedCopy(), parentOrigin.data().isolatedCopy(), WTFMove(deviceIdHashSalt));
     }).iterator->value;
 
     deviceIdHashSalt->lastTimeUsed = WallTime::now();
@@ -71,8 +72,10 @@ void DeviceIdHashSaltStorage::getDeviceIdHashSaltOrigins(CompletionHandler<void(
 {
     HashSet<SecurityOriginData> origins;
 
-    for (auto& hashSaltForOrigin : m_deviceIdHashSaltForOrigins)
+    for (auto& hashSaltForOrigin : m_deviceIdHashSaltForOrigins) {
         origins.add(hashSaltForOrigin.value->documentOrigin);
+        origins.add(hashSaltForOrigin.value->parentOrigin);
+    }
 
     RunLoop::main().dispatch([origins = WTFMove(origins), completionHandler = WTFMove(completionHandler)]() mutable {
         completionHandler(WTFMove(origins));
@@ -82,7 +85,7 @@ void DeviceIdHashSaltStorage::getDeviceIdHashSaltOrigins(CompletionHandler<void(
 void DeviceIdHashSaltStorage::deleteDeviceIdHashSaltForOrigins(const Vector<SecurityOriginData>& origins, CompletionHandler<void()>&& completionHandler)
 {
     m_deviceIdHashSaltForOrigins.removeIf([&origins](auto& keyAndValue) {
-        return origins.contains(keyAndValue.value->documentOrigin);
+        return origins.contains(keyAndValue.value->documentOrigin) || origins.contains(keyAndValue.value->parentOrigin);
     });
 
     RunLoop::main().dispatch(WTFMove(completionHandler));
index b3945f5..5fe2a86 100644 (file)
@@ -38,7 +38,7 @@ public:
     static Ref<DeviceIdHashSaltStorage> create();
     ~DeviceIdHashSaltStorage() = default;
 
-    const String& deviceIdHashSaltForOrigin(const WebCore::SecurityOrigin&);
+    const String& deviceIdHashSaltForOrigin(const WebCore::SecurityOrigin& documentOrigin, const WebCore::SecurityOrigin& parentOrigin);
 
     void getDeviceIdHashSaltOrigins(CompletionHandler<void(HashSet<WebCore::SecurityOriginData>&&)>&&);
     void deleteDeviceIdHashSaltForOrigins(const Vector<WebCore::SecurityOriginData>&, CompletionHandler<void()>&&);
@@ -46,13 +46,15 @@ public:
 
 private:
     struct HashSaltForOrigin {
-        HashSaltForOrigin(WebCore::SecurityOriginData&& securityOrigin, String&& deviceIdHashSalt)
-            : documentOrigin(securityOrigin)
+        HashSaltForOrigin(WebCore::SecurityOriginData&& documentOrigin, WebCore::SecurityOriginData&& parentOrigin, String&& deviceIdHashSalt)
+            : documentOrigin(WTFMove(documentOrigin))
+            , parentOrigin(WTFMove(parentOrigin))
             , deviceIdHashSalt(WTFMove(deviceIdHashSalt))
             , lastTimeUsed(WallTime::now())
         { };
 
         WebCore::SecurityOriginData documentOrigin;
+        WebCore::SecurityOriginData parentOrigin;
         String deviceIdHashSalt;
         WallTime lastTimeUsed;
     };
index e642f74..56d3588 100644 (file)
@@ -163,7 +163,7 @@ void UserMediaPermissionRequestManagerProxy::userMediaAccessWasGranted(uint64_t
     if (!request)
         return;
 
-    auto deviceIDHashSalt = m_page.websiteDataStore().deviceIdHashSaltStorage()->deviceIdHashSaltForOrigin(request->topLevelDocumentSecurityOrigin());
+    auto deviceIDHashSalt = m_page.websiteDataStore().deviceIdHashSaltStorage()->deviceIdHashSaltForOrigin(request->userMediaDocumentSecurityOrigin(), request->topLevelDocumentSecurityOrigin());
     if (grantAccess(userMediaID, WTFMove(audioDevice), WTFMove(videoDevice), WTFMove(deviceIDHashSalt)))
         m_grantedRequests.append(request.releaseNonNull());
 #else
@@ -362,7 +362,7 @@ void UserMediaPermissionRequestManagerProxy::requestUserMediaPermissionForFrame(
 
         syncWithWebCorePrefs();
 
-        auto deviceIDHashSalt = m_page.websiteDataStore().deviceIdHashSaltStorage()->deviceIdHashSaltForOrigin(pendingRequest.value()->topLevelDocumentSecurityOrigin());
+        auto deviceIDHashSalt = m_page.websiteDataStore().deviceIdHashSaltStorage()->deviceIdHashSaltForOrigin(pendingRequest.value()->userMediaDocumentSecurityOrigin(), pendingRequest.value()->topLevelDocumentSecurityOrigin());
         RealtimeMediaSourceCenter::singleton().validateRequestConstraints(WTFMove(validHandler), WTFMove(invalidHandler), WTFMove(localUserRequest), WTFMove(deviceIDHashSalt));
     };
 
@@ -399,7 +399,7 @@ void UserMediaPermissionRequestManagerProxy::getUserMediaPermissionInfo(uint64_t
 void UserMediaPermissionRequestManagerProxy::enumerateMediaDevicesForFrame(uint64_t userMediaID, uint64_t frameID, Ref<SecurityOrigin>&& userMediaDocumentOrigin, Ref<SecurityOrigin>&& topLevelDocumentOrigin)
 {
 #if ENABLE(MEDIA_STREAM)
-    auto completionHandler = [this, topOrigin = topLevelDocumentOrigin.copyRef()](uint64_t userMediaID, bool originHasPersistentAccess) {
+    auto completionHandler = [this, requestOrigin = userMediaDocumentOrigin.copyRef(), topOrigin = topLevelDocumentOrigin.copyRef()](uint64_t userMediaID, bool originHasPersistentAccess) {
         auto pendingRequest = m_pendingDeviceRequests.take(userMediaID);
         if (!pendingRequest)
             return;
@@ -414,7 +414,7 @@ void UserMediaPermissionRequestManagerProxy::enumerateMediaDevicesForFrame(uint6
             return !device.enabled() || (device.type() != WebCore::CaptureDevice::DeviceType::Camera && device.type() != WebCore::CaptureDevice::DeviceType::Microphone);
         });
 
-        auto deviceIDHashSalt = m_page.websiteDataStore().deviceIdHashSaltStorage()->deviceIdHashSaltForOrigin(topOrigin.get());
+        auto deviceIDHashSalt = m_page.websiteDataStore().deviceIdHashSaltStorage()->deviceIdHashSaltForOrigin(requestOrigin.get(), topOrigin.get());
         m_page.process().send(Messages::WebPage::DidCompleteMediaDeviceEnumeration(userMediaID, WTFMove(devices), WTFMove(deviceIDHashSalt), WTFMove(originHasPersistentAccess)), m_page.pageID());
     };