Changeset 254537 in webkit

Timestamp:

Jan 14, 2020 2:35:09 PM (4 years ago)

Author:

Alan Coon

Message:

Revert r254334. rdar://problem/58542040

Location:

branches/safari-610.1.1-branch

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/SharedBuffer.cpp (modified) (2 diffs)
• Source/WebCore/platform/SharedBuffer.h (modified) (1 diff)
• Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp (modified) (5 diffs)
• Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.h (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebCore/SharedBuffer.cpp (modified) (1 diff)

branches/safari-610.1.1-branch/Source/WebCore/ChangeLog

@@ +1 @@
+2020-01-14  Kocsen Chung  <kocsen_chung@apple.com>
+
+        Revert r254334. rdar://problem/58542040
+
 2020-01-11  Zalan Bujtas  <zalan@apple.com>
 

branches/safari-610.1.1-branch/Source/WebCore/platform/SharedBuffer.cpp

@@ -30 +30 @@
 
 #include <algorithm>
-#include <wtf/HexNumber.h>
 #include <wtf/persistence/PersistentCoders.h>
-#include <wtf/text/StringBuilder.h>
 #include <wtf/unicode/UTF8Conversion.h>
 
@@ -133 +131 @@
     element--; // std::upper_bound gives a pointer to the element that is greater than position. We want the element just before that.
     return { element->segment.copyRef(), position - element->beginPosition };
-}
-
-String SharedBuffer::toHexString() const
-{
-    StringBuilder stringBuilder;
-    for (unsigned byteOffset = 0; byteOffset < size(); byteOffset++) {
-        const uint8_t byte = data()[byteOffset];
-        stringBuilder.append(pad('0', 2, hex(byte)));
-    }
-    return stringBuilder.toString();
 }
 

branches/safari-610.1.1-branch/Source/WebCore/platform/SharedBuffer.h

@@ -199 +199 @@
     SharedBufferDataView getSomeData(size_t position) const;
 
-    String toHexString() const;
-
     void hintMemoryNotNeededSoon() const;
 

branches/safari-610.1.1-branch/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp

@@ -35 +35 @@
 #include "CDMRestrictions.h"
 #include "CDMSessionType.h"
-#include "Logging.h"
 #include "SharedBuffer.h"
 #include <wtf/JSONValues.h>
@@ -539 +538 @@
 }
 
-String CDMInstanceClearKey::Key::keyIDAsString() const
-{
-    return makeString("[", keyIDData->toHexString(), "]");
-}
-
-String CDMInstanceClearKey::Key::keyValueAsString() const
-{
-    return makeString("[", keyValueData->toHexString(), "]");
-}
-
-bool operator==(const CDMInstanceClearKey::Key& k1, const CDMInstanceClearKey::Key& k2)
-{
-    ASSERT(k1.keyIDData);
-    ASSERT(k2.keyIDData);
-
-    return *k1.keyIDData == *k2.keyIDData;
-}
-
-bool operator<(const CDMInstanceClearKey::Key& k1, const CDMInstanceClearKey::Key& k2)
-{
-    ASSERT(k1.keyIDData);
-    ASSERT(k2.keyIDData);
-
-    if (k1.keyIDData->size() != k2.keyIDData->size())
-        return k1.keyIDData->size() < k2.keyIDData->size();
-
-    return memcmp(k1.keyIDData->data(), k2.keyIDData->data(), k1.keyIDData->size());
-}
-
 void CDMInstanceSessionClearKey::requestLicense(LicenseType, const AtomString& initDataType, Ref<SharedBuffer>&& initData, LicenseCallback&& callback)
 {
@@ -603 +573 @@
         };
 
+    // Parse the response buffer as an JSON object.
     RefPtr<JSON::Object> root = parseJSONObject(response);
     if (!root) {
@@ -609 +580 @@
     }
 
-    LOG(EME, "EME - ClearKey - updating license for session %s", sessionId.utf8().data());
-
+    // Parse the response using 'license' formatting, if possible.
     if (auto decodedKeys = parseLicenseFormat(*root)) {
         // Retrieve the target Vector of Key objects for this session.
-        // FIXME: Refactor this state management code.
-        Vector<CDMInstanceClearKey::Key>& keyVector = ClearKeyState::singleton().keys().ensure(sessionId, [] { return Vector<CDMInstanceClearKey::Key> { }; }).iterator->value;
-
+        auto& keyVector = ClearKeyState::singleton().keys().ensure(sessionId, [] { return Vector<CDMInstanceClearKey::Key> { }; }).iterator->value;
+
+        // For each decoded key, find an existing item for the decoded key's ID. If none exist,
+        // the key is decoded. Otherwise, the key is updated in case there's a mismatch between
+        // the size or data of the existing and proposed key.
         bool keysChanged = false;
-        for (auto& decodedKey : *decodedKeys) {
-            LOG(EME, "EME - ClearKey - Decoded a key with ID %s and key data %s", decodedKey.keyIDAsString().utf8().data(), decodedKey.keyValueAsString().utf8().data());
-            auto keyWithMatchingKeyID = std::find_if(keyVector.begin(), keyVector.end(),
-                [&decodedKey] (const CDMInstanceClearKey::Key& containedKey) {
-                    return containedKey == decodedKey;
+        for (auto& key : *decodedKeys) {
+            auto it = std::find_if(keyVector.begin(), keyVector.end(),
+                [&key] (const CDMInstanceClearKey::Key& containedKey) {
+                    return containedKey.keyIDData->size() == key.keyIDData->size()
+                        && !std::memcmp(containedKey.keyIDData->data(), key.keyIDData->data(), containedKey.keyIDData->size());
                 });
-            if (keyWithMatchingKeyID != keyVector.end()) {
-                LOG(EME, "EME - ClearKey - Existing key found with data %s", keyWithMatchingKeyID->keyValueAsString().utf8().data());
-
-                if (!keyWithMatchingKeyID->hasSameKeyValue(decodedKey)) {
-                    LOG(EME, "EME - ClearKey - Updating key since the data are different");
-                    *keyWithMatchingKeyID = WTFMove(decodedKey);
+            if (it != keyVector.end()) {
+                auto& existingKey = it->keyValueData;
+                auto& proposedKey = key.keyValueData;
+
+                // Update the existing Key if it differs from the proposed key in key value.
+                if (existingKey->size() != proposedKey->size() || std::memcmp(existingKey->data(), proposedKey->data(), existingKey->size())) {
+                    *it = WTFMove(key);
                     keysChanged = true;
                 }
             } else {
-                LOG(EME, "EME - ClearKey - This is a new key");
-                keyVector.append(WTFMove(decodedKey));
+                // In case a Key for this key ID doesn't exist yet, append the new one to keyVector.
+                keyVector.append(WTFMove(key));
                 keysChanged = true;
             }
         }
 
-        LOG(EME, "EME - ClearKey - Update has provided %zu keys", keyVector.size());
-
+        // In case of changed keys, we have to provide a KeyStatusVector of all the keys for
+        // this session.
         Optional<KeyStatusVector> changedKeys;
         if (keysChanged) {
-            // Sort by key IDs.
-            std::sort(keyVector.begin(), keyVector.end());
-
+            // First a helper Vector is constructed, cotaining pairs of SharedBuffer RefPtrs
+            // representint key ID data, and the corresponding key statuses.
+            // We can't use KeyStatusVector here because this Vector has to be sorted, which
+            // is not possible to do on Ref<> objects.
+            Vector<std::pair<RefPtr<SharedBuffer>, KeyStatus>> keys;
+            keys.reserveInitialCapacity(keyVector.size());
+            for (auto& it : keyVector)
+                keys.uncheckedAppend(std::pair<RefPtr<SharedBuffer>, KeyStatus> { it.keyIDData, it.status });
+
+            // Sort first by size, second by data.
+            std::sort(keys.begin(), keys.end(),
+                [] (const auto& a, const auto& b) {
+                    if (a.first->size() != b.first->size())
+                        return a.first->size() < b.first->size();
+
+                    return std::memcmp(a.first->data(), b.first->data(), a.first->size()) < 0;
+                });
+
+            // Finally construct the mirroring KeyStatusVector object and move it into the
+            // Optional<> object that will be passed to the callback.
             KeyStatusVector keyStatusVector;
-            keyStatusVector.reserveInitialCapacity(keyVector.size());
-            for (auto& key : keyVector)
-                keyStatusVector.uncheckedAppend(std::pair<Ref<SharedBuffer>, KeyStatus> { *key.keyIDData, key.status });
+            keyStatusVector.reserveInitialCapacity(keys.size());
+            for (auto& it : keys)
+                keyStatusVector.uncheckedAppend(std::pair<Ref<SharedBuffer>, KeyStatus> { *it.first, it.second });
 
             changedKeys = WTFMove(keyStatusVector);
@@ -657 +647 @@
     }
 
+    // Parse the response using 'license release acknowledgement' formatting, if possible.
     if (parseLicenseReleaseAcknowledgementFormat(*root)) {
         // FIXME: Retrieve the key ID information and use it to validate the keys for this sessionId.

branches/safari-610.1.1-branch/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.h

@@ -111 +111 @@
         RefPtr<SharedBuffer> keyIDData;
         RefPtr<SharedBuffer> keyValueData;
-
-        String keyIDAsString() const;
-        String keyValueAsString() const;
-
-        bool hasSameKeyValue(const Key &other)
-        {
-            ASSERT(keyValueData);
-            ASSERT(other.keyValueData);
-            return *keyValueData == *other.keyValueData;
-        }
-
-        // Two keys are equal if they have the same ID, ignoring key value and status.
-        friend bool operator==(const Key &k1, const Key &k2);
-        // Key's are ordered by their IDs, first by size, then by contents.
-        friend bool operator<(const Key &k1, const Key &k2);
-
-        friend bool operator!=(const Key &k1, const Key &k2) { return !(operator==(k1, k2)); }
-        friend bool operator>(const Key &k1, const Key &k2) { return !operator==(k1, k2) && !operator<(k1, k2); }
-        friend bool operator<=(const Key &k1, const Key &k2) { return !operator>(k1, k2); }
-        friend bool operator>=(const Key &k1, const Key &k2) { return !operator<(k1, k2); }
     };
 

branches/safari-610.1.1-branch/Tools/ChangeLog

@@ +1 @@
+2020-01-14  Kocsen Chung  <kocsen_chung@apple.com>
+
+        Revert r254334. rdar://problem/58542040
+
 2020-01-11  Alex Christensen  <achristensen@webkit.org>
 

branches/safari-610.1.1-branch/Tools/TestWebKitAPI/Tests/WebCore/SharedBuffer.cpp

@@ -218 +218 @@
 }
 
-TEST_F(SharedBufferTest, toHexString)
-{
-    Vector<char> t1 = {0x11, 0x5, 0x12};
-    auto buffer = SharedBuffer::create();
-    buffer->append(WTFMove(t1));
-    String result = buffer->toHexString();
-    EXPECT_EQ(result, "110512");
-    buffer->clear();
-    EXPECT_EQ(buffer->toHexString(), "");
-}
-
-}
+}