M1502: Improve HTTP monitoring devtool support

[akumar21NCSU]

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @jdm (or someone else) soon.

[jdm]

Great work! Most of my comments that I've left are style issues or things that could be written more clearly!
-S-awaiting-review +S-needs-code-changes

---

Reviewed 9 of 9 files at r1, 2 of 2 files at r2, 1 of 1 files at r3, 4 of 4 files at r4, 1 of 1 files at r5, 1 of 1 files at r6, 2 of 2 files at r7, 2 of 2 files at r8.
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed.

---

^{components/net/http_loader.rs, line 5 [r1] (raw file):}
Let's move this to lib.rs with the other ones.

---

^{components/net/http_loader.rs, line 42 [r1] (raw file):}
This doesn't need the self:: with the previous change.

---

^{components/net/http_loader.rs, line 597 [r7] (raw file):}
Let's remove this comment.

---

^{components/net/http_loader.rs, line 603 [r7] (raw file):}
These three lines of arguments should be indented by four spaces.

---

^{components/net/http_loader.rs, line 449 [r8] (raw file):}
Let's style this like:

let request = DevtoolsHttpRequest {
    url: url, method: method, headers: headers, body: body, pipeline_id: pipeline_id
};

---

^{tests/unit/net/http_loader.rs, line 5 [r4] (raw file):}
Let's move this to lib.rs with the others.

---

^{tests/unit/net/http_loader.rs, line 30 [r4] (raw file):}
Remove self:: after the previous change.

---

^{tests/unit/net/http_loader.rs, line 388 [r4] (raw file):}
No need for this local value; just inline Some(pipeline_id) in the next line.

---

^{tests/unit/net/http_loader.rs, line 459 [r4] (raw file):}
No need to clone here, and we can just inline None instead of creating the optional value.

---

^{tests/unit/net/http_loader.rs, line 461 [r4] (raw file):}
This isn't necessary in this test. We can use the default headers in the LoadData value.

---

^{tests/unit/net/http_loader.rs, line 468 [r8] (raw file):}
This can be assert!(devtools_port.try_recv().is_error()).

---

Comments from the review on Reviewable.io

[bors-servo]

☔ The latest upstream changes (presumably #8244) made this pull request unmergeable. Please resolve the merge conflicts.

[jdm]

Lovely! We just need to clean up the remaining errors from ./mach test-tidy, and squash all the changes into one.
-S-awaiting-review -S-needs-rebase +S-fails-tidy +S-needs-squash

---

Reviewed 9 of 9 files at r9.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from the review on Reviewable.io

[jdm]

@bors-servo: r+

---

Reviewed 4 of 4 files at r10.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io

[bors-servo]

📌 Commit 9c6374d has been approved by jdm

[bors-servo]

⌛ Testing commit 9c6374d with merge 5646871...

[bors-servo]

💔 Test failed - mac-dev-ref-unit

[eefriedman]

---- http_loader::test_request_and_response_data_with_network_messages stdout ----
    thread 'http_loader::test_request_and_response_data_with_network_messages' panicked at 'assertion failed: `(left == right)` (left: `HttpRequest { url: Url { scheme: "https", scheme_data: Relative(RelativeSchemeData { username: "", password: None, host: Domain("mozilla.com"), port: None, default_port: Some(443), path: [""] }), query: None, fragment: None }, method: Get, headers: Headers { User-Agent: Test-agent, Accept: text/html, application/xhtml+xml, application/xml; q=0.9, */*; q=0.8, Accept-Encoding: gzip, deflate, br, Host: mozilla.com, }, body: None, pipeline_id: PipelineId { namespace_id: PipelineNamespaceId(0), index: PipelineIndex(0) } }`, right: `HttpRequest { url: Url { scheme: "https", scheme_data: Relative(RelativeSchemeData { username: "", password: None, host: Domain("mozilla.com"), port: None, default_port: Some(443), path: [""] }), query: None, fragment: None }, method: Get, headers: Headers { User-Agent: Test-agent, Host: mozilla.com, Accept-Encoding: gzip, deflate, Accept: text/html, application/xhtml+xml, application/xml; q=0.9, */*; q=0.8, }, body: None, pipeline_id: PipelineId { namespace_id: PipelineNamespaceId(0), index: PipelineIndex(0) } }`)', /Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/net/http_loader.rs:433
stack backtrace:
   1:        0x102115d28 - sys::backtrace::tracing::imp::write::h662793599f713c74T5s
   2:        0x10211782f - panicking::log_panic::_<closure>::closure.38923
   3:        0x1021172fc - panicking::log_panic::h87af6096e0b6fa7c4Uw
   4:        0x1021097b6 - sys_common::unwind::begin_unwind_inner::h1e338bc64cf56d9dr8r
   5:        0x102109b9e - sys_common::unwind::begin_unwind_fmt::he992719329bdd2c6x7r
   6:        0x101e1578b - http_loader::test_request_and_response_data_with_network_messages::h4823d798daf0308cVRc
   7:        0x1020b8edb - boxed::_<impl>::call_box::call_box::h1439439458556714188
   8:        0x1020bb62d - boxed::_<impl>::call_box::call_box::h8069092559954270103
   9:        0x1020b9832 - sys_common::unwind::try::try_fn::try_fn::h11324081254561251759
  10:        0x1021151f8 - __rust_try
  11:        0x102112dce - sys_common::unwind::try::inner_try::hc13d8e198528cd0fZ4r
  12:        0x1020b99e2 - boxed::_<impl>::call_box::call_box::h6022582708213645161
  13:        0x102116bfd - sys::thread::_<impl>::new::thread_start::h906ddce3a93d4852ksw
  14:     0x7fff8416a059 - _pthread_body
  15:     0x7fff84169fd6 - _pthread_start


failures:
    http_loader::test_request_and_response_data_with_network_messages

test result: FAILED. 149 passed; 1 failed; 0 ignored; 0 measured

[akumar21NCSU]

@jdm Thanks. I have changed default headers accordingly.

[jdm]

@bors-servo: r+

[bors-servo]

📌 Commit de4f8ea has been approved by jdm

[bors-servo]

⌛ Testing commit de4f8ea with merge 742cc29...

[bors-servo]

💔 Test failed - mac-rel-wpt

[jdm]

@bors-servo: retry

• #7787

[bors-servo]

⌛ Testing commit de4f8ea with merge 7151d8a...

[bors-servo]

💔 Test failed - mac-rel-wpt

[jdm]

@bors-servo: retry

• #7787

[bors-servo]

⌛ Testing commit de4f8ea with merge 45f07ec...

[bors-servo]

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt