Handle the result of OnStartRequest correctly in HttpTransactionParent::RecvOnStartRequest

RESOLVED FIXED

Status

()

enhancement
P2
normal
RESOLVED FIXED
7 months ago
5 months ago

People

(Reporter: kershaw, Assigned: kershaw)

Tracking

(Blocks 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [necko-triaged])

Attachments

(1 attachment)

(Assignee)

Description

7 months ago
We currently do nothing if HttpChannel::OnStartRequest returns failure [1], which is not correct.
We can try to adapt the logic in HttpChannelChild [2] here.

Moreover, we should also consider if we need ChannelEventQueue in HttpTransactionParent.


[1] https://hg.mozilla.org/projects/larch/file/tip/netwerk/protocol/http/HttpTransactionParent.cpp#l205
[2] https://searchfox.org/mozilla-central/rev/924e3d96d81a40d2f0eec1db5f74fc6594337128/netwerk/protocol/http/HttpChannelChild.cpp#771-775
(Assignee)

Updated

7 months ago
Assignee: nobody → kershaw
Blocks: socket-proc
Priority: -- → P2
Whiteboard: [necko-triaged]
I think when [1] fails we should Cancel(rv) the pipe (which is |this| actually, the HttpTransactionParent object), rv = chan->OnStartRequest(this, nullptr);.  

The contract is that when OnStart fails, we must not call OnData and propagate the error to OnStop
QA Contact: jduell.mcbugs
(Assignee)

Comment 2

7 months ago
When OnStart fails, we must not call OnData and propagate
the error to OnStop.
QA Contact: jduell.mcbugs
Blocks: 1497270
Looks like this can land now, unless there is some dependency not listed in this bug.
Flags: needinfo?(kershaw)
(Assignee)

Comment 4

5 months ago
(In reply to Honza Bambas (:mayhemer) from comment #3)
> Looks like this can land now, unless there is some dependency not listed in
> this bug.

Already landed.
Flags: needinfo?(kershaw)
(Assignee)

Comment 5

5 months ago
https://hg.mozilla.org/projects/larch/rev/9f014528a74861685601f3d52522c7d14cc907ef
Status: NEW → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.