[webkit-reviews] review granted: [Bug 177102] Allow modern decoding of Vectors : [Attachment 321137] Patch

[bugzilla-daemon at webkit.org]

Andy Estes <aestes at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 177102: Allow modern decoding of Vectors
https://bugs.webkit.org/show_bug.cgi?id=177102

Attachment 321137: Patch

https://bugs.webkit.org/attachment.cgi?id=321137&action=review




--- Comment #6 from Andy Estes <aestes at apple.com> ---
Comment on attachment 321137
  --> https://bugs.webkit.org/attachment.cgi?id=321137
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=321137&action=review

> Source/WebCore/Modules/indexeddb/IDBDatabaseIdentifier.cpp:38
> -IDBDatabaseIdentifier::IDBDatabaseIdentifier(const String& databaseName,
const SecurityOrigin& openingOrigin, const SecurityOrigin& mainFrameOrigin)
> +IDBDatabaseIdentifier::IDBDatabaseIdentifier(const String& databaseName,
const SecurityOriginData& openingOrigin, const SecurityOriginData&
mainFrameOrigin)

Can the SecurityOriginDatas be rvalue references?

> Source/WebKit/Platform/IPC/ArgumentCoders.h:274
> +    static std::optional<Vector<T, inlineCapacity>> decode(Decoder& decoder)

It would be nice if we implemented the non-optional version of this function in
terms of the optional-returning version.

> Source/WebKit/Platform/IPC/ArgumentCoders.h:280
> +	   Vector<T, inlineCapacity> vector;

We should use reserveInitialCapacity() here since we know the size.

> Source/WebKit/Platform/IPC/ArgumentCoders.h:286
> +	       vector.append(WTFMove(*element));

We should use uncheckedAppend().

> Source/WebKit/Platform/IPC/ArgumentCoders.h:324
> +    static std::optional<Vector<T, inlineCapacity>> decode(Decoder& decoder)

It would be nice if we implemented the non-optional version of this function in
terms of the optional-returning version.

> Source/WebKit/Shared/RTCNetwork.cpp:143
> +	   return result;

WTFMove?

> Source/WebKit/Shared/RTCNetwork.cpp:150
> +    return result;

Ditto.

> Source/WebKit/Shared/RTCNetwork.cpp:213
> +    return result;

Ditto.

> Source/WebKit/Shared/WebCompiledContentRuleListData.cpp:79
> +    return compiledContentRuleListData;

Ditto.

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:660
> +    return rect;

Ditto.

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:815
> +    return recentSearch;

Ditto.

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:944
> +    return pluginInfo;

Ditto.

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:2258
> +    return blobPart;

Ditto.

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:2508
> +    return statistics;

Ditto.

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:2566
> +    return device;

Ditto.

> Source/WebKit/Shared/WebPopupItem.cpp:123
> +    return item;

Ditto.

> Source/WebKit/Shared/Gamepad/GamepadData.cpp:86
> +    return data;

Ditto.

> Source/WebKit/Shared/Plugins/NPIdentifierData.cpp:93
> +    return result;

Ditto.

> Source/WebKit/Shared/Plugins/NPVariantData.cpp:171
> +	   return result;

Buncha dittos.