[Bindings] Remove use of Dictionary/ArrayValue in CDMSessionClearKey
Created attachment 297328 [details] Patch
Comment on attachment 297328 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=297328&action=review > Source/WebCore/Modules/encryptedmedia/CDMSessionClearKey.cpp:120 > + auto keysDataObject = asObject(keysDataValue); > + > + auto keysArrayValue = keysDataObject->get(&state, Identifier::fromString(&state, "keys")); I think i’d omit the keysDataObject local variable since it’s used only this once. But not sure everyone would agree. > Source/WebCore/Modules/encryptedmedia/CDMSessionClearKey.cpp:127 > + size_t length = keysArray->length(); Shouldn’t this be unsigned rather than size_t? I don’t think getIndex takes a 64-bit value. > Source/WebCore/Modules/encryptedmedia/CDMSessionClearKey.cpp:134 > for (size_t i = 0; i < length; ++i) { Shouldn’t this be unsigned rather than size_t? > Source/WebCore/Modules/encryptedmedia/CDMSessionClearKey.cpp:137 > + if (scope.exception() || !keyValue || !keyValue.isObject()) { I don’t think the !keyValue check is needed here. Doesn’t isObject safely return false in that case? > Source/WebCore/Modules/encryptedmedia/CDMSessionClearKey.cpp:144 > + auto getStringProperty = [&](ExecState& state, JSObject* object, const char* name) -> String { Not sure we need to capture at all here. Would be nice to be explicit that we are not capturing. I’d probably also have just written this function in the more conventional way at the top level of the namespace rather than writing it this way, here. > Source/WebCore/Modules/encryptedmedia/CDMSessionClearKey.cpp:146 > + if (scope.exception() || !value || !value.isString()) Not sure the !value check is needed here. Same question as above. > Source/WebCore/Modules/encryptedmedia/CDMSessionClearKey.cpp:149 > + auto string = value.toWTFString(&state); This should be asString(value)->value(&state); we already checked isString, so we don’t need the rest of what toWTFString does.
Committed r209939: <http://trac.webkit.org/changeset/209939>