Make the net monitor panel in FF's devtools show meaningful output. #11593

jdm · 2016-06-03T17:51:01Z

Advertise support for the network monitor in the initial protocol communication.
Only notify the developer tools server about the final request in an HTTP transaction.
Add timing information for connecting to the HTTP server and sending the HTTP request.
Reduce duplication between various networkEventUpdate structures by creating a helper function
that merges two JSON structures together. This also corrects the JSON structure so the devtools
client interprets the output correctly.
Calculate various header size fields correctly.
Remove unnecessary usize->u32 casts by making the appropriate fields usize.
Add header values to request and response header messages.
Support triggering page reloads via the devtools client.

I apologize that these aren't broken apart. I was making a lot of changes trying to figure out why the panel wasn't working right, and a lot of them were tangled together.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes do not require tests because there are no automated tests for the devtools server.

This change is

highfive · 2016-06-03T17:51:04Z

Heads up! This PR modifies the following files:

@fitzgen: components/devtools/actors/root.rs, components/devtools_traits/lib.rs, components/devtools_traits/lib.rs, components/devtools/actors/network_event.rs, components/devtools/protocol.rs, components/devtools/lib.rs, components/devtools/actors/tab.rs
@KiChjang: components/net/fetch/methods.rs, components/net/http_loader.rs, components/script/script_thread.rs, components/script/devtools.rs

highfive · 2016-06-03T17:51:05Z

Warning

These commits modify net and script code, but no tests are modified. Please consider adding a test!

jdm · 2016-06-03T17:53:39Z

cc @dd0x68
There's still lots to do to make the network panel useful (including verifying that it's actually reporting valid data for the cases in which Servo is attempting to provide it), but this gives us a place to start.

metajack · 2016-06-04T06:10:03Z

Wow!

nox · 2016-06-04T10:21:20Z

Do you think you could split it in commits that follow the steps you describe in the PR description?

nox · 2016-06-04T23:11:25Z

Oh never mind, didn't notice your note at the end of the PR description.

nox · 2016-06-05T11:03:00Z

@bors-servo r+

bors-servo · 2016-06-05T11:03:01Z

📌 Commit caa102f has been approved by nox

bors-servo · 2016-06-05T11:46:13Z

⌛ Testing commit caa102f with merge de2ea38...


        Auto merge of #11593 - jdm:netmon, r=nox

Make the net monitor panel in FF's devtools show meaningful output.  1. Advertise support for the network monitor in the initial protocol communication. 1. Only notify the developer tools server about the final request in an HTTP transaction. 1. Add timing information for connecting to the HTTP server and sending the HTTP request. 1. Reduce duplication between various networkEventUpdate structures by creating a helper function that merges two JSON structures together. This also corrects the JSON structure so the devtools client interprets the output correctly. 1. Calculate various header size fields correctly. 1. Remove unnecessary usize->u32 casts by making the appropriate fields usize. 1. Add header values to request and response header messages. 1. Support triggering page reloads via the devtools client. I apologize that these aren't broken apart. I was making a lot of changes trying to figure out why the panel wasn't working right, and a lot of them were tangled together. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes do not require tests because there are no automated tests for the devtools server.   --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11593)

bors-servo · 2016-06-05T11:57:17Z

💔 Test failed - linux-dev

nox · 2016-06-05T11:58:21Z

/home/servo/buildbot/slave/linux-dev/build/tests/unit/net/http_loader.rs:500:23: 500:42 error: missing fields `connect_time`, `send_time`, `timeStamp` in initializer of `devtools_traits::HttpRequest` [E0063]
/home/servo/buildbot/slave/linux-dev/build/tests/unit/net/http_loader.rs:500     let httprequest = DevtoolsHttpRequest {
                                                                                                   ^~~~~~~~~~~~~~~~~~~
/home/servo/buildbot/slave/linux-dev/build/tests/unit/net/http_loader.rs:500:23: 500:42 help: run `rustc --explain E0063` to see a detailed explanation
error: aborting due to previous error

The unit tests need to be updated.


        Make the net monitor panel in FF's devtools show meaningful output.

0) Advertise support for the network monitor in the initial protocol communication. 1) Only notify the developer tools server about the final request in an HTTP transaction. 2) Add timing information for connecting to the HTTP server and sending the HTTP request. 3) Reduce duplication between various networkEventUpdate structures by creating a helper function that merges two JSON structures together. This also corrects the JSON structure so the devtools client interprets the output correctly. 4) Calculate various header size fields correctly. 5) Remove unnecessary usize->u32 casts by making the appropriate fields usize. 6) Add header values to request and response header messages. 7) Support triggering page reloads via the devtools client.

highfive · 2016-06-05T23:51:57Z

New code was committed to pull request.

jdm · 2016-06-05T23:52:05Z

@bors-servo: r=nox

bors-servo · 2016-06-05T23:52:05Z

📌 Commit 7bf2e19 has been approved by nox

bors-servo · 2016-06-05T23:52:07Z

⌛ Testing commit 7bf2e19 with merge 825b5b6...


        Auto merge of #11593 - jdm:netmon, r=nox

Make the net monitor panel in FF's devtools show meaningful output.  1. Advertise support for the network monitor in the initial protocol communication. 1. Only notify the developer tools server about the final request in an HTTP transaction. 1. Add timing information for connecting to the HTTP server and sending the HTTP request. 1. Reduce duplication between various networkEventUpdate structures by creating a helper function that merges two JSON structures together. This also corrects the JSON structure so the devtools client interprets the output correctly. 1. Calculate various header size fields correctly. 1. Remove unnecessary usize->u32 casts by making the appropriate fields usize. 1. Add header values to request and response header messages. 1. Support triggering page reloads via the devtools client. I apologize that these aren't broken apart. I was making a lot of changes trying to figure out why the panel wasn't working right, and a lot of them were tangled together. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes do not require tests because there are no automated tests for the devtools server.   --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11593)

bors-servo · 2016-06-06T00:40:20Z

💔 Test failed - linux-rel

highfive · 2016-06-06T00:40:24Z

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform-abspos-002.htm
  └   → /css-transforms-1_dev/html/transform-abspos-002.htm 71f0313eedfbfcce0b0fcc5ae55f34b1daa3b8d8
/css-transforms-1_dev/html/reference/transform-abspos-ref.htm 78d197606924062e8dd2a773c977afcecf8940f8
Testing 71f0313eedfbfcce0b0fcc5ae55f34b1daa3b8d8 == 78d197606924062e8dd2a773c977afcecf8940f8

nox · 2016-06-06T00:41:20Z

@bors-servo retry #11561

bors-servo · 2016-06-06T00:41:21Z

⚡ Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows are reusable. Rebuilding only linux-rel...

bors-servo · 2016-06-06T01:29:18Z

💔 Test failed - linux-rel

highfive · 2016-06-06T01:29:20Z

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform3d-translatez-001.htm
  └   → /css-transforms-1_dev/html/transform3d-translatez-001.htm 902d90a8d198625335e738587fdfe81dcc90392d
/css-transforms-1_dev/html/reference/transform3d-translatez-ref.htm b14318ccfd8a59b7b249d41521e178d617678823
Testing 902d90a8d198625335e738587fdfe81dcc90392d == b14318ccfd8a59b7b249d41521e178d617678823
/css-transforms-1_dev/html/transform3d-translatez-001.htm 902d90a8d198625335e738587fdfe81dcc90392d
/css-transforms-1_dev/html/reference/transform3d-translatez-notref.htm 902d90a8d198625335e738587fdfe81dcc90392d
Testing 902d90a8d198625335e738587fdfe81dcc90392d != 902d90a8d198625335e738587fdfe81dcc90392d

jdm · 2016-06-06T01:46:21Z

@bors-servo: retry

#11631

bors-servo · 2016-06-06T02:19:23Z

⌛ Testing commit 7bf2e19 with merge 1bc94c1...


        Auto merge of #11593 - jdm:netmon, r=nox

Make the net monitor panel in FF's devtools show meaningful output.  1. Advertise support for the network monitor in the initial protocol communication. 1. Only notify the developer tools server about the final request in an HTTP transaction. 1. Add timing information for connecting to the HTTP server and sending the HTTP request. 1. Reduce duplication between various networkEventUpdate structures by creating a helper function that merges two JSON structures together. This also corrects the JSON structure so the devtools client interprets the output correctly. 1. Calculate various header size fields correctly. 1. Remove unnecessary usize->u32 casts by making the appropriate fields usize. 1. Add header values to request and response header messages. 1. Support triggering page reloads via the devtools client. I apologize that these aren't broken apart. I was making a lot of changes trying to figure out why the panel wasn't working right, and a lot of them were tangled together. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes do not require tests because there are no automated tests for the devtools server.   --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11593)

bors-servo · 2016-06-06T03:07:49Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows

highfive assigned pcwalton Jun 3, 2016

highfive added the S-awaiting-review label Jun 3, 2016

jdm assigned nox and unassigned pcwalton Jun 3, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Jun 5, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jun 5, 2016

nox added S-needs-code-changes and removed S-tests-failed labels Jun 5, 2016

jdm force-pushed the jdm:netmon branch from caa102f to 7bf2e19 Jun 5, 2016

highfive removed the S-needs-code-changes label Jun 5, 2016

highfive added the S-awaiting-review label Jun 5, 2016

highfive removed the S-awaiting-review label Jun 5, 2016

highfive added the S-awaiting-merge label Jun 5, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jun 6, 2016

highfive added the S-tests-failed label Jun 6, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Jun 6, 2016

bors-servo merged commit 7bf2e19 into servo:master Jun 6, 2016
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Jun 6, 2016

jdm mentioned this pull request Jun 17, 2016

Improve network monitor integration #11773

Closed

servo / servo

Make the net monitor panel in FF's devtools show meaningful output. #11593

Make the net monitor panel in FF's devtools show meaningful output. #11593

jdm commented Jun 3, 2016 •

edited by larsbergstrom

highfive commented Jun 3, 2016

highfive commented Jun 3, 2016

jdm commented Jun 3, 2016

metajack commented Jun 4, 2016

nox commented Jun 4, 2016

nox commented Jun 4, 2016

nox commented Jun 5, 2016

bors-servo commented Jun 5, 2016

bors-servo commented Jun 5, 2016

bors-servo commented Jun 5, 2016

nox commented Jun 5, 2016

highfive commented Jun 5, 2016

jdm commented Jun 5, 2016

bors-servo commented Jun 5, 2016

bors-servo commented Jun 5, 2016

bors-servo commented Jun 6, 2016

highfive commented Jun 6, 2016

nox commented Jun 6, 2016

bors-servo commented Jun 6, 2016

bors-servo commented Jun 6, 2016

highfive commented Jun 6, 2016

jdm commented Jun 6, 2016

bors-servo commented Jun 6, 2016

bors-servo commented Jun 6, 2016

servo / servo

Join GitHub today

Make the net monitor panel in FF's devtools show meaningful output. #11593

Make the net monitor panel in FF's devtools show meaningful output. #11593

Conversation

jdm commented Jun 3, 2016 • edited by larsbergstrom

highfive commented Jun 3, 2016

highfive commented Jun 3, 2016

jdm commented Jun 3, 2016

metajack commented Jun 4, 2016

nox commented Jun 4, 2016

nox commented Jun 4, 2016

nox commented Jun 5, 2016

bors-servo commented Jun 5, 2016

bors-servo commented Jun 5, 2016

bors-servo commented Jun 5, 2016

nox commented Jun 5, 2016

highfive commented Jun 5, 2016

jdm commented Jun 5, 2016

bors-servo commented Jun 5, 2016

bors-servo commented Jun 5, 2016

bors-servo commented Jun 6, 2016

highfive commented Jun 6, 2016

nox commented Jun 6, 2016

bors-servo commented Jun 6, 2016

bors-servo commented Jun 6, 2016

highfive commented Jun 6, 2016

jdm commented Jun 6, 2016

bors-servo commented Jun 6, 2016

bors-servo commented Jun 6, 2016

jdm commented Jun 3, 2016 •

edited by larsbergstrom