Bug 241407 – SharedBuffer IPC encoders have duplicated and redundant code

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Kimmo Kinnunen]

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), ... };
}

[Radar WebKit Bug Importer]

<rdar://problem/94621576>

[Jean-Yves Avenard [:jya]]

Pull request: https://github.com/WebKit/WebKit/pull/1377

[EWS]

Committed r295487 (251492@main): <https://commits.webkit.org/251492@main>

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