[Webkit-unassigned] [Bug 241407] New: SharedBuffer IPC encoders have duplicated and redundant code

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 8 01:28:54 PDT 2022


https://bugs.webkit.org/show_bug.cgi?id=241407

            Bug ID: 241407
           Summary: SharedBuffer IPC encoders have duplicated and
                    redundant code
           Product: WebKit
           Version: WebKit Nightly Build
          Hardware: Unspecified
                OS: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: WebKit2
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: kkinnunen at apple.com
                CC: jean-yves.avenard at apple.com, kkinnunen at apple.com

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

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220608/0eff5098/attachment-0001.htm>


More information about the webkit-unassigned mailing list