Accept Brotli-compressed HTTP responses #8156 #8192

nxnfufunezn · 2015-10-25T17:14:05Z

jdm · 2015-10-25T17:32:48Z

This looks like what I expected; thanks! Have you tested it manually? Easiest way to verify it's working is to comment out the branch on 427 and find a page that serves Brotli-encoded data (and therefore is full of garbage), then uncomment that branch and see that the page shows up correctly.

nxnfufunezn · 2015-10-26T02:30:05Z

@jdm Is there any site that serves Brotli-encoded data ? Or how can I verify this any other way ?

nxnfufunezn · 2015-10-26T03:00:57Z

@jdm It should be Accept-Encoding: br instead of Accept-Encoding: Brotli am I right ?

frewsxcv · 2015-10-26T04:15:44Z

components/net/Cargo.toml

@@ -16,6 +16,9 @@ path = "../util"
 [dependencies.devtools_traits]
 path = "../devtools_traits"

+[dependencies.brotli]
+git = "https://github.com/ende76/brotli-rs"


Don't forget to add the Cargo.lock files

jdm · 2015-10-26T05:54:02Z

@nxnfufunezn You're right! Aso, https://http2.cloudflare.com/ supports Brotli encoding according to my Firefox network inspector.

nxnfufunezn · 2015-10-26T05:58:29Z

Oh I am using this python script : https://gist.github.com/nxnfufunezn/d3fe9c805004a94a262d for testing...

nxnfufunezn · 2015-10-26T06:00:53Z

Also, I think I need to add brotli here also https://github.com/servo/servo/blob/master/components/net/http_loader.rs#L156

jdm · 2015-10-26T06:01:24Z

Even better! That looks like something we could turn into an automated test, like this one: http://mxr.mozilla.org/servo/source/tests/wpt/web-platform-tests/XMLHttpRequest/resources/gzip.py?force=1

jdm · 2015-10-26T06:01:41Z

Yep, you're correct.

nxnfufunezn · 2015-10-26T06:03:31Z

okay I will do that also.. 👍

nxnfufunezn · 2015-10-26T06:37:42Z

@jdm It works now..

nxnfufunezn · 2015-10-26T18:36:27Z

Here is what I have done so far https://gist.github.com/nxnfufunezn/220bd5dc1dfb590bb8d6

jdm · 2015-10-26T19:05:06Z

@nxnfufunezn That looks really good!

nxnfufunezn · 2015-10-29T05:45:30Z

@jdm

jdm · 2015-10-29T11:47:30Z

Looks great! We just need to rebase this to get the proper manifest output, as described in the comment. Please also include the Cargo.lock changes, like @frewsxcv pointed out.
-S-awaiting-review +S-needs-rebase

Reviewed 3 of 3 files at r1, 6 of 6 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

^{tests/wpt/mozilla/meta/MANIFEST.json, line 3806 [r2] (raw file):}
If you rebase to master and run ./mach test-wpt --update-manifest again, this should produce difference results since #8185 merged.

Comments from the review on Reviewable.io

frewsxcv · 2015-10-29T12:19:05Z

^{tests/wpt/mozilla/tests/mozilla/resources/brotli.py, line 5 [r2] (raw file):}
This can be written:

output = request.GET.get("content", request.body)

https://docs.python.org/2/library/stdtypes.html#dict.get

Comments from the review on Reviewable.io

nxnfufunezn · 2015-10-29T12:30:31Z

^{tests/wpt/mozilla/tests/mozilla/resources/brotli.py, line 5 [r2] (raw file):}
So I don't need brotli compressed data there ?

Comments from the review on Reviewable.io

frewsxcv · 2015-10-29T12:32:56Z

^{tests/wpt/mozilla/tests/mozilla/resources/brotli.py, line 5 [r2] (raw file):}
What do you mean by that? My code block is identical in functionality to your if..else statement

Comments from the review on Reviewable.io

nxnfufunezn · 2015-10-29T12:34:27Z

^{tests/wpt/mozilla/tests/mozilla/resources/brotli.py, line 5 [r2] (raw file):}
oh.. I got it..

Comments from the review on Reviewable.io

jdm · 2015-11-01T14:43:38Z

@bors-servo: r+

bors-servo · 2015-11-01T14:43:39Z

📌 Commit 468eaac has been approved by jdm

bors-servo · 2015-11-01T14:43:41Z

⌛ Testing commit 468eaac with merge 58a0ded...


        Auto merge of #8192 - nxnfufunezn:brotli-decompressor, r=jdm

@jdm

Accept Brotli-compressed HTTP responses #8156 r? @jdm  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8192)

bors-servo · 2015-11-01T15:21:17Z

💔 Test failed - mac-rel-wpt

frewsxcv · 2015-11-01T15:31:25Z

@bors-servo retry

#7787

bors-servo · 2015-11-01T17:43:23Z

⌛ Testing commit 468eaac with merge 2e4454a...


        Auto merge of #8192 - nxnfufunezn:brotli-decompressor, r=jdm

@jdm

Accept Brotli-compressed HTTP responses #8156 r? @jdm  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8192)

bors-servo · 2015-11-01T18:17:36Z

💔 Test failed - mac-rel-wpt

jdm · 2015-11-01T18:28:44Z

@bors-servo: retry

#7731

bors-servo · 2015-11-01T18:39:23Z

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

bors-servo · 2015-11-01T19:18:16Z

💔 Test failed - mac-rel-wpt

eefriedman · 2015-11-01T19:19:55Z

@bors-servo retry

#7787

bors-servo · 2015-11-01T19:19:57Z

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

bors-servo · 2015-11-01T19:50:00Z

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

highfive added the S-awaiting-review label Oct 25, 2015

frewsxcv reviewed Oct 26, 2015
View changes

jdm self-assigned this Oct 26, 2015

nxnfufunezn force-pushed the nxnfufunezn:brotli-decompressor branch from d1a0431 to 05ca36b Oct 26, 2015

nxnfufunezn mentioned this pull request Oct 26, 2015

Add wpt-test for brotli compression #8207

Closed

jdm added S-needs-code-changes and removed S-awaiting-review labels Oct 26, 2015

nxnfufunezn force-pushed the nxnfufunezn:brotli-decompressor branch from 05ca36b to 58207c0 Oct 28, 2015

highfive added S-awaiting-review and removed S-needs-code-changes labels Oct 28, 2015

jdm added S-needs-rebase and removed S-awaiting-review labels Oct 29, 2015

highfive added S-awaiting-merge and removed S-awaiting-review labels Nov 1, 2015

highfive added S-tests-failed and removed S-awaiting-merge labels Nov 1, 2015

highfive added S-awaiting-merge and removed S-tests-failed labels Nov 1, 2015

highfive added S-tests-failed and removed S-awaiting-merge labels Nov 1, 2015

bors-servo mentioned this pull request Nov 1, 2015

Implement Range#deleteContents #6826

Merged

nxnfufunezn deleted the nxnfufunezn:brotli-decompressor branch Nov 2, 2015

jdm mentioned this pull request Nov 3, 2015

M1502: Improve HTTP monitoring devtool support #8216

Merged

SimonSapin removed the S-tests-failed label Jan 27, 2016

servo / servo

Accept Brotli-compressed HTTP responses #8156 #8192

Accept Brotli-compressed HTTP responses #8156 #8192

nxnfufunezn commented Oct 25, 2015

jdm commented Oct 25, 2015

nxnfufunezn commented Oct 26, 2015

nxnfufunezn commented Oct 26, 2015

This comment has been minimized.

jdm commented Oct 26, 2015

nxnfufunezn commented Oct 26, 2015

nxnfufunezn commented Oct 26, 2015

jdm commented Oct 26, 2015

jdm commented Oct 26, 2015

nxnfufunezn commented Oct 26, 2015

nxnfufunezn commented Oct 26, 2015

nxnfufunezn commented Oct 26, 2015

jdm commented Oct 26, 2015

nxnfufunezn commented Oct 29, 2015

jdm commented Oct 29, 2015

frewsxcv commented Oct 29, 2015

nxnfufunezn commented Oct 29, 2015

frewsxcv commented Oct 29, 2015

nxnfufunezn commented Oct 29, 2015

jdm commented Nov 1, 2015

bors-servo commented Nov 1, 2015

bors-servo commented Nov 1, 2015

bors-servo commented Nov 1, 2015

frewsxcv commented Nov 1, 2015

bors-servo commented Nov 1, 2015

bors-servo commented Nov 1, 2015

jdm commented Nov 1, 2015

bors-servo commented Nov 1, 2015

bors-servo commented Nov 1, 2015

eefriedman commented Nov 1, 2015

bors-servo commented Nov 1, 2015

bors-servo commented Nov 1, 2015

servo / servo

Join GitHub today

Accept Brotli-compressed HTTP responses #8156 #8192

Accept Brotli-compressed HTTP responses #8156 #8192

Conversation

nxnfufunezn commented Oct 25, 2015

jdm commented Oct 25, 2015

nxnfufunezn commented Oct 26, 2015

nxnfufunezn commented Oct 26, 2015

This comment has been minimized.

frewsxcv Oct 26, 2015

jdm commented Oct 26, 2015

nxnfufunezn commented Oct 26, 2015

nxnfufunezn commented Oct 26, 2015

jdm commented Oct 26, 2015

jdm commented Oct 26, 2015

nxnfufunezn commented Oct 26, 2015

nxnfufunezn commented Oct 26, 2015

nxnfufunezn commented Oct 26, 2015

jdm commented Oct 26, 2015

nxnfufunezn commented Oct 29, 2015

jdm commented Oct 29, 2015

frewsxcv commented Oct 29, 2015

nxnfufunezn commented Oct 29, 2015

frewsxcv commented Oct 29, 2015

nxnfufunezn commented Oct 29, 2015

jdm commented Nov 1, 2015

bors-servo commented Nov 1, 2015

bors-servo commented Nov 1, 2015

bors-servo commented Nov 1, 2015

frewsxcv commented Nov 1, 2015

bors-servo commented Nov 1, 2015

bors-servo commented Nov 1, 2015

jdm commented Nov 1, 2015

bors-servo commented Nov 1, 2015

bors-servo commented Nov 1, 2015

eefriedman commented Nov 1, 2015

bors-servo commented Nov 1, 2015

bors-servo commented Nov 1, 2015