Bug 241407 - SharedBuffer IPC encoders have duplicated and redundant code
Summary: SharedBuffer IPC encoders have duplicated and redundant code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jean-Yves Avenard [:jya]
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-06-08 01:28 PDT by Kimmo Kinnunen
Modified: 2022-06-13 05:13 PDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kimmo Kinnunen 2022-06-08 01:28:54 PDT
SharedBuffer IPC encoders have duplicated and redundant code

There are multiple redundant top-level argument coders specialised for Ref,RefPtr:

template<> struct ArgumentCoder<RefPtr<WebCore::FragmentedSharedBuffer>> {
    static void encode(Encoder&, const RefPtr<WebCore::FragmentedSharedBuffer>&);
    static std::optional<RefPtr<WebCore::FragmentedSharedBuffer>> decode(Decoder&);
};

template<> struct ArgumentCoder<Ref<WebCore::FragmentedSharedBuffer>> {
    static void encode(Encoder&, const Ref<WebCore::FragmentedSharedBuffer>&);
    static std::optional<Ref<WebCore::FragmentedSharedBuffer>> decode(Decoder&);
};

template<> struct ArgumentCoder<RefPtr<WebCore::SharedBuffer>> {
    static void encode(Encoder&, const RefPtr<WebCore::SharedBuffer>&);
    static std::optional<RefPtr<WebCore::SharedBuffer>> decode(Decoder&);
};


Instead, there should be top-level argument coders only for the root types:

template<> struct ArgumentCoder<WebCore::FragmentedSharedBuffer> {
    static void encode(Encoder&, const WebCore::FragmentedSharedBuffer&);
    static std::optional<Ref<WebCore::FragmentedSharedBuffer>> decode(Decoder&);
};

template<> struct ArgumentCoder<WebCore::SharedBuffer> {
    static void encode(Encoder&, const WebCore::SharedBuffer&);
    static std::optional<Ref<WebCore::SharedBuffer>> decode(Decoder&);
};



The encoder implementations use multiple redundant functions and duplicated logic for encoding and decoding

static void encodeSharedBuffer(Encoder& encoder, const FragmentedSharedBuffer* buffer)
static void encodeTypesAndData(Encoder& encoder, const Vector<String>& types, const Vector<RefPtr<SharedBuffer>>& data)


Instead, the implementations should be of form:
ArgumentCoder<WebCore::FragmentedSharedBuffer>::encode(Encoder&, const WebCore::FragmentedSharedBuffer& buffer) 
{
    encode the buffer
}

Generic Vector<T> encoders should be used to encode vectors of buffers, avoiding duplication of "encode size, for each buffer encode the buffer".
Generic RefPtr<T> encoders should be used  to encode the refptrs of buffers, avoiding duplication of "if exists then encode true and buffer else encode false".

The decoder implementations use multiple redundant functions and duplicated logic

static WARN_UNUSED_RETURN bool decodeSharedBuffer(Decoder& decoder, RefPtr<SharedBuffer>& buffer)

     if (!decodeSharedBuffer(decoder, content.dataInWebArchiveFormat))
        return false;

instead, the decoding should be of form:

std::optional<PasteboardWebContent> ArgumentCoder<PasteboardWebContent>::decode(Decoder& decoder)
{
     auto dataInWebArchievFormat = decoder.decode<RefPtr<SharedBuffer>>();
     .....
     if (UNLIKELY(!decoder.isValid())
          return std::nullopt;
     return PasteboardWebContent { ...., WTFMove(*dataInWebArchiveFormat), ... };
}
Comment 1 Radar WebKit Bug Importer 2022-06-08 04:28:05 PDT
<rdar://problem/94621576>
Comment 2 Jean-Yves Avenard [:jya] 2022-06-08 06:43:47 PDT
Pull request: https://github.com/WebKit/WebKit/pull/1377
Comment 3 EWS 2022-06-13 05:13:21 PDT
Committed r295487 (251492@main): <https://commits.webkit.org/251492@main>

Reviewed commits have been landed. Closing PR #1377 and removing active labels.