Bug 177102 – Allow modern decoding of Vectors

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Allow modern decoding of Vectors

[Alex Christensen]

Created attachment 321129 [details]
Patch

[Alex Christensen]

Created attachment 321130 [details]
Patch

[Alex Christensen]

Created attachment 321132 [details]
Patch

[Alex Christensen]

Created attachment 321135 [details]
Patch

[Alex Christensen]

Created attachment 321137 [details]
Patch

[Andy Estes]

Comment on attachment 321137 [details]
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.

[Alex Christensen]

Created attachment 321208 [details]
Patch

[Alex Christensen]

Created attachment 321210 [details]
Patch

[Alex Christensen]

Created attachment 321214 [details]
Patch

[Alex Christensen]

Created attachment 321217 [details]
Patch

[Alex Christensen]

Created attachment 321227 [details]
Patch

[Build Bot]

Comment on attachment 321227 [details]
Patch

Attachment 321227 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4596440

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 321240 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 321227 [details]
Patch

Attachment 321227 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4597191

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 321258 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Alex Christensen]

Created attachment 321259 [details]
Patch

[Anders Carlsson]

Comment on attachment 321259 [details]
Patch

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

> Source/WebKit/Platform/IPC/ArgumentCoders.h:271
> +        Vector<T, inlineCapacity> vector;
> +        vector.reserveInitialCapacity(size);

There's a reason why reserveInitialCapacity wasn't called here - a compromised web process could send MAXINT and cause an allocation failure.

[Alex Christensen]

Wouldn't it crash either way?

[Alex Christensen]

Created attachment 321262 [details]
Patch

[Alex Christensen]

http://trac.webkit.org/r222233

[Radar WebKit Bug Importer]

<rdar://problem/34693204>