[Webkit-unassigned] [Bug 137299] POST form data is missing with a custom NSURLProtocol

[bugzilla-daemon at webkit.org]

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

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #240538|review?                     |review-
              Flags|                            |

--- Comment #20 from Darin Adler <darin at apple.com> ---
Comment on attachment 240538
  --> https://bugs.webkit.org/attachment.cgi?id=240538
Patch

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

I don’t think the approach is quite right here. Also, the patch does not build on any of the EWS bots, so review-.

> Source/WebCore/platform/network/ResourceRequestWithBody.h:35
> +class ResourceRequestWithBody {

I don’t understand the purpose of this class. It seems to wrap a ResourceRequest and add nothing, with forwarding functions. Does not seem like a good pattern to add a class like this. I understand the desire to have clarity about whether a body is present, but this does not seem to be the right way to accomplish that. We don’t want a class that we have to keep updating every time we change the design of ResourceRequest.

> Source/WebCore/platform/network/ResourceRequestWithBody.h:36
> +    ResourceRequest m_request;

This is the wrong style for WebKit. We put private data members last, after public members, and we explicitly write "private".

> Source/WebCore/platform/network/ResourceRequestWithBody.h:49
> +#if ENABLE(CACHE_PARTITIONING)
> +    const String& cachePartition() const { return m_request.cachePartition(); }
> +#endif
> +#if ENABLE(INSPECTOR)
> +    // Whether this request should be hidden from the Inspector.
> +    bool hiddenFromInspector() const { return m_request.hiddenFromInspector(); }
> +#endif

These should have blank lines around them, so they make their own paragraphs.

> Source/WebCore/platform/network/ResourceRequestWithBody.h:52
> +    template<class Encoder> void encodeWithoutPlatformData(Encoder& e) const { m_request.encodeWithoutPlatformData(e); }

We use words for argument names, not letters. This should be "encoder".

-- 
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/20141028/f1f4f6c0/attachment-0002.html>