1754737 - Extract tunnel setup code, i.e. only CONNECT request handling from nsHttpConnection

Status: RESOLVED FIXED

(Core :: Networking: HTTP, task, P2)

Description

[Dragana Damjanovic [:dragana]]

Currently, this code is an internal part of nsHttpConnection, it can be moved to a separate class with a small interface.
It would be good to add some state machine to nsHttpConnection so that is easier to detect what does the connection does at the moment.

To explore: can we make this a layer under nsHttpConnection?

Comment 1

[Dragana Damjanovic [:dragana]]

Attachment: Extract the http connect stream from nsHttpConnection

Comment 2

[Dragana Damjanovic [:dragana]]

Attachment: Add some structure to nsHttpConnection:

• nsHttpConnection now has states. Currently only proxy setup phase is added to the states.
• The states are used in nsHttpConnection::OnSocketWritable tto make the code more understandable.
• Some pieces of selfcontained code are extracted from nsHttpConnection::OnHeadersAvailable, i.e. HandleTunnelResponse and HandleWebSocketResponse

Comment 3

[Dragana Damjanovic [:dragana]]

Attachment: We do not need to keep info about the use of a tunnel in the state.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/13ab95d47842
Add some structure to nsHttpConnection: r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/0cc869e52a2f
We do not need to keep info about the use of a tunnel in the state. r=necko-reviewers,kershaw

Comment 5

[Noemi Erli[:noemi_erli]]

Backed out 2 changesets (Bug 1754737) for causing failures in nsHttpConnection.cpp CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=0cc869e52a2fff042580a8f75445f53de57baa35&selectedTaskRun=bViWgmmRRm-_c0JCqzqUHA.0

Log: https://treeherder.mozilla.org/logviewer?job_id=368773137&repo=autoland&lineNumber=1652
Busted build: https://treeherder.mozilla.org/logviewer?job_id=368774427&repo=autoland&lineNumber=26637

Backout: https://hg.mozilla.org/integration/autoland/rev/1bb40041ed904e71c8d4dea5c97543fcd8d8a566

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/25650cac85f4
Add some structure to nsHttpConnection: r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/0377fb241cc9
We do not need to keep info about the use of a tunnel in the state. r=necko-reviewers,kershaw

Comment 7

[Sandor Molnar[:smolnar]]

Backed out 2 changesets (bug 1754737) for causing linux build bustage in netwerk/protocol/http/nsHttpConnection.

Backout link: https://hg.mozilla.org/integration/autoland/rev/d651411ec33c640ea7c0f3e581b829a974aa4a6c

Push with failures

Failure log

INFO -  /builds/worker/checkouts/gecko/netwerk/protocol/http/nsHttpConnection.cpp: In member function 'nsresult mozilla::net::nsHttpConnection::CheckTunnelIsNeeded()':
[task 2022-03-23T00:50:07.894Z] 00:50:07    ERROR -  /builds/worker/checkouts/gecko/netwerk/protocol/http/nsHttpConnection.cpp:2708:1: error: control reaches end of non-void function [-Werror=return-type]
[task 2022-03-23T00:50:07.894Z] 00:50:07     INFO -   }
[task 2022-03-23T00:50:07.894Z] 00:50:07     INFO -   ^
[task 2022-03-23T00:50:07.895Z] 00:50:07     INFO -  /builds/worker/checkouts/gecko/netwerk/protocol/http/nsHttpConnection.cpp: In member function 'nsresult mozilla::net::nsHttpConnection::OnSocketReadable()':
[task 2022-03-23T00:50:07.895Z] 00:50:07  WARNING -  /builds/worker/checkouts/gecko/netwerk/protocol/http/nsHttpConnection.cpp:1977:12: warning: 'rv' may be used uninitialized in this function [-Wmaybe-uninitialized]
[task 2022-03-23T00:50:07.897Z] 00:50:07     INFO -     nsresult rv;
[task 2022-03-23T00:50:07.897Z] 00:50:07     INFO -              ^~
[task 2022-03-23T00:50:07.898Z] 00:50:07     INFO -  cc1plus: all warnings being treated as errors

Comment 8

[Dragana Damjanovic [:dragana]]

I was fixed.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0dc7d14597df
Add some structure to nsHttpConnection: r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/ba3b0e315592
We do not need to keep info about the use of a tunnel in the state. r=necko-reviewers,kershaw

Comment 10

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/0dc7d14597df
https://hg.mozilla.org/mozilla-central/rev/ba3b0e315592