1241377 - Add nsIFormPOSTActionChannel interface to enable custom channel to accept POST.

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Tooru Fujisawa [:arai]]

+++ This bug was initially created as a clone of Bug #1241100 +++

bug 1208124 patch will be backed out in bug 1241100.
we'll need another way to enable custom channel to accept POST, by adding dedicated interface.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Part 1: Implement nsIFormPOSTActionChannel for the channel accepts form POST.

Renamed interface name, and fixed the comment.

now I'm preparing testcase, but it may take some more time.

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: (WIP) Part 2: Add test for nsIFormPOSTActionChannel.

Here's a WIP patch for testcase.


It implements custom protocol handler that returns custom channel.
the channel shows form for "x-bug1241377://dummy/form/*.html" URI, and it submits the form with POST to "x-bug1241377://dummy/action/*.html"

the channel for "x-bug1241377://dummy/action/*.html" implements nsIUploadChannel or nsIFormPOSTActionChannel, depends on the URI.

the testcase checks if the upload stream is set to the channel when submitting each case.


this patch at least works and the all tests (is) passed, but it has a problem around tab handling.
after running the test, following error happens, so looks like the tab state is somehow broken, or not updated properly.

> 30 INFO TEST-UNEXPECTED-FAIL | netwerk/test/browser/browser_nsIFormPOSTActionChannel.js | Found an unexpected tab at the end of test run: x-bug1241377://dummy/action/post.html - 
> 31 INFO TEST-UNEXPECTED-FAIL | netwerk/test/browser/browser_nsIFormPOSTActionChannel.js | Found an unexpected tab at the end of test run: x-bug1241377://dummy/form/upload.html - 


can I get some feedback on it?
is this correct approach to test channel?
what could be the best way to avoid the "unexpected tab" issue?

Comment 3

[Honza Bambas (:mayhemer)]

Comment on attachment 8710312 [details] [diff] [review]
Part 1: Implement nsIFormPOSTActionChannel for the channel accepts form POST.

One more thing: the patch doesn't apply on the latest m-c, please remerge.

Comment 4

[Tooru Fujisawa [:arai]]

Thank you for reviewing!

I forgot to note that the patch is based on the backout patch in bug 1241100.
I just landed it to m-i.

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Part 2: Add test for nsIFormPOSTActionChannel.

noticed that I should wait for frame's load.
now tab is removed correctly, and I can test the channel information without global variable.

in x-bug1241377://dummy/action/*.html page, 2 input element contains information about upload channel and posted data.  frame script checks those values and sends the data to message listener.

Currently this custom protocol handler does not work in e10s.
so this test is skipped in e10s.

> The address wasn't understood
> 
> Firefox doesn't know how to open this address, because one of the following
> protocols (x-bug1241377) isn't associated with any program or is not allowed
> in this context.
> 
>     You might need to install other software to open this address.

Comment 6

[Honza Bambas (:mayhemer)]

Comment on attachment 8710933 [details] [diff] [review]
Part 2: Add test for nsIFormPOSTActionChannel.

Review of attachment 8710933 [details] [diff] [review]:
-----------------------------------------------------------------

if the tests fails w/o the patch and passes w/ the patch, then r+

::: netwerk/test/browser/browser_nsIFormPOSTActionChannel.js
@@ +185,5 @@
> +    };
> +    Services.tm.currentThread.dispatch(runnable, Ci.nsIEventTarget.DISPATCH_NORMAL);
> +  },
> +  asyncOpen2: function(aListener, aContext) {
> +    throw Cr.NS_ERROR_NOT_IMPLEMENTED;

just forward to asyncOpen? and I think there is no context arg

@@ +193,5 @@
> +  get name() {
> +    return this.uri.spec;
> +  },
> +  isPending: function () {
> +      return false;

indent

Comment 7

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/acbb1a73119e5d91080c4c055555c14d9ee139fd
Bug 1241377 - Part 1: Implement nsIFormPOSTActionChannel for the channel accepts form POST. r=mayhemer

https://hg.mozilla.org/integration/mozilla-inbound/rev/9d310fb25e569412cddee9a366310c7890cbb881
Bug 1241377 - Part 2: Add test for nsIFormPOSTActionChannel. r=mayhemer

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/acbb1a73119e
https://hg.mozilla.org/mozilla-central/rev/9d310fb25e56

Comment 9

[Daniel Veditz [:dveditz]]

If uplifted to aurora/beta to re-fix bug 1208124 then also land the test patch from bug 1241100
https://hg.mozilla.org/mozilla-central/rev/7f1349b7c571

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Arai: Should we uplift this fix to Beta45 and Aurora46?

Comment 11

[Tooru Fujisawa [:arai]]

I don't think this needs uplifting, as we don't get any request for uplifting this feature from add-on authors until now.
bug 1208124, bug 1241100, and this bug mean that we're delaying to add customizability for add-on that implements protocol handler and channel, from firefox 44 to 47.