[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

--- Comment #28 from Daniel <danielo at opera.com> ---
Thanks for taking a look Maciej.

(1) I'll rebase and reupload to "unred" the windows.
(2) You've asked 2 questions here:
The first is "why it's not general, but only for startLoading case?". It's because it's a minimum that's enough for our use cases. Having this the Chrome team can adjust their code to take a body from on the protocol level, and then use it in the delegate callbacks. Another reason is that having it global is a vague performance problem (in some conditions, on some platforms, hard to test), and that's why it was stripped in the first place. Having it for a single case makes performance implications clear.

My first attempt was to enable it to be passed globally, you can see it in obsolete patches:
https://bugs.webkit.org/attachment.cgi?id=239328&action=prettypatch
And it was rejected for performance reasons, which you mention as well.

The 2nd question is: "how did I test it on iOS?". The answer is in the bug description:
You have to call [WKBrowsingContextController registerSchemeForCustomProtocol:], then it works as expected.

(3) The patch doesn't lack tests. The test case was uploaded in the first place (you can see it in obsolete patches), and it is still present in the last version of the patch. It's in the file Tools/TestWebKitAPI/Tests/WebKit2ObjC/CustomProtocolsTest.mm . See:
https://bugs.webkit.org/attachment.cgi?id=240667&action=diff#a/Tools/TestWebKitAPI/Tests/WebKit2ObjC/CustomProtocolsTest.mm_sec1

(5) I agree about that having file upload support is important. I just want the work to be split in separate tasks, that's why I made a separate bug 138131 for supporting uploads. I don't think that the work done here code-wise depends on 138131 or vice-versa, because:
a) separate fields are used for normal body and stream
b) serialization is done separately
c) different methods should be applied when passing this stream through IPC (and that can be a long discussion for streams)
d) separate test case needs to be made
For me this is enough to split it as a separate issue, and work on it separately.

The second reason for splitting it is that some clients might be already happy by only having simple POST forms.

The third reason is that it's the first experience, and I wanted to start gently with small and targeted changes, where it's easy for us to argue about the solution and if it makes sense or not. By no means I wanted to start with some huge architectural changes that affect performance in unknown ways, and require a lot of experience and testing.

If this work is accepted, and a related 137302 is accepted (which is already reviewed, but blocked by this bug), I was planning to start working on uploads shortly, because we definitely need them as well. So don't worry, the streams case won't be forgotten.

-- 
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/20141204/a760e5ae/attachment-0002.html>