[EME] Implement CDMInstanceClearKey::updateLicense()
Created attachment 320556 [details] Patch
Attachment 320556 [details] did not pass style-queue: ERROR: Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:357: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 320556 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320556&action=review Really nice! > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:47 > + using KeyStore = HashMap<String, Vector<CDMInstanceClearKey::Key>>; I would leave blank lines before and after this line. > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:345 > + auto dispatchCallback = > + [this, &callback](bool sessionWasClosed, std::optional<KeyStatusVector>&& changedKeys, SuccessValue succeeded) { > + callOnMainThread( > + [weakThis = m_weakPtrFactory.createWeakPtr(), callback = WTFMove(callback), sessionWasClosed, changedKeys = WTFMove(changedKeys), succeeded] () mutable { > + if (!weakThis) > + return; > + > + callback(sessionWasClosed, WTFMove(changedKeys), std::nullopt, std::nullopt, succeeded); > + }); > + }; Two lambdas in the same variable declaration cause you can! :))) I'd add a comment explaining that the inner lambda is the one executed in the main thread and the outter one is a way to avoid having that line duplicated several times in the following code. > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:373 > + if (existingKey->size() != proposedKey->size() || std::memcmp(existingKey->data(), proposedKey->data(), existingKey->size())) { I'd add a comment here cause we actually try to move the memory if sizes are equal but we do it in the if clause, and then if that fails, we move the key. I think this way is the most elegant way of doing it, but I think it still requires some clarification.
Comment on attachment 320556 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320556&action=review >> Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:47 >> + using KeyStore = HashMap<String, Vector<CDMInstanceClearKey::Key>>; > > I would leave blank lines before and after this line. I'd say only after, leaving an empty line before would look odd. >> Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:345 >> + }; > > Two lambdas in the same variable declaration cause you can! :))) I'd add a comment explaining that the inner lambda is the one executed in the main thread and the outter one is a way to avoid having that line duplicated several times in the following code. I'll add the comment. >> Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:373 >> + if (existingKey->size() != proposedKey->size() || std::memcmp(existingKey->data(), proposedKey->data(), existingKey->size())) { > > I'd add a comment here cause we actually try to move the memory if sizes are equal but we do it in the if clause, and then if that fails, we move the key. I think this way is the most elegant way of doing it, but I think it still requires some clarification. Only the Key object is moved, in case there's difference between the existing and proposed keys' values, in either size or data.
Created attachment 320623 [details] Patch for landing
Attachment 320623 [details] did not pass style-queue: ERROR: Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:360: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 320623 [details] Patch for landing Clearing flags on attachment: 320623 Committed r221961: <http://trac.webkit.org/changeset/221961>
All reviewed patches have been landed. Closing bug.
<rdar://problem/34693406>