Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving the error handling out of network loader #9942

Merged
merged 4 commits into from Apr 20, 2016
Merged

Moving the error handling out of network loader #9942

merged 4 commits into from Apr 20, 2016

Conversation

@jdm
Copy link
Member

jdm commented Mar 9, 2016

Rebase of #8851. Fixes #8678. Fixes #9944.

This change is Review on Reviewable
@wafflespeanut wafflespeanut mentioned this pull request Mar 9, 2016
Closed
@jdm
Copy link
Member Author

jdm commented Mar 9, 2016

Note, currently writing tests for the security properties of chrome_loader.rs.
@jdm jdm force-pushed the jdm:load_error branch from c38a4ae to 15eeebb Mar 9, 2016
@jdm
Copy link
Member Author

jdm commented Mar 9, 2016

Tests written. I've reviewed all the commits by @wafflespeanut but someone else should review my changes in the final commit.
@jdm jdm force-pushed the jdm:load_error branch from 15eeebb to 5c5916d Mar 9, 2016
@jdm jdm mentioned this pull request Mar 9, 2016
Closed
@KiChjang KiChjang closed this Mar 9, 2016
@KiChjang KiChjang reopened this Mar 9, 2016
@Ms2ger
Copy link
Contributor

Ms2ger commented Mar 18, 2016

LGTM
@jdm jdm force-pushed the jdm:load_error branch from 5c5916d to 15bed8e Mar 18, 2016
@jdm jdm assigned Ms2ger and unassigned glennw Mar 18, 2016
@jdm
Copy link
Member Author

jdm commented Mar 18, 2016

@bors-servo: r=jdm,Ms2ger
@bors-servo
Copy link
Contributor

bors-servo commented Mar 18, 2016

📌 Commit 15bed8e has been approved by jdm,Ms2ger
@jdm
Copy link
Member Author

jdm commented Mar 18, 2016

@bors-servo: r-
It's not clear from the appveyor log that the tests actually ran.
@jdm jdm force-pushed the jdm:load_error branch from 15bed8e to 248b3f1 Mar 18, 2016
@Ms2ger
Copy link
Contributor

Ms2ger commented Mar 21, 2016

Running C:/projects/servo/target\debug\deps\gfx_tests-34a41c7816b6d727.exe
Command exited with code 127
@jdm
Copy link
Member Author

jdm commented Mar 21, 2016

Yeah. I'm guessing it's mach PATH shenanigans like all the times people have encountered that with ./mach run on Windows.
@jdm jdm force-pushed the jdm:load_error branch 3 times, most recently from 3c72ad7 to 39e991d Mar 21, 2016
@Ms2ger
Copy link
Contributor

Ms2ger commented Mar 23, 2016

r? @larsbergstrom
@highfive highfive assigned larsbergstrom and unassigned Ms2ger Mar 23, 2016
@jdm jdm force-pushed the jdm:load_error branch 2 times, most recently from 979b82d to 0d4ebaf Apr 20, 2016
@jdm jdm removed the S-needs-rebase label Apr 20, 2016
@jdm
Copy link
Member Author

jdm commented Apr 20, 2016

Waiting on the result of the appveyor test.
wafflespeanut and others added 4 commits Dec 31, 2015
@wafflespeanut @jdm
Propagating the load errors from network loader
5e6f32a
@wafflespeanut @jdm
Updated the unit tests to include NetworkError
945a2c6
@wafflespeanut @jdm
Added 'about:sslfail' and a reftest for SSL failure
16096f1
@jdm
Make new test use local resources only.
Loading status checks…
d888ed3
@jdm jdm force-pushed the jdm:load_error branch from 0d4ebaf to d888ed3 Apr 20, 2016
@jdm
Copy link
Member Author

jdm commented Apr 20, 2016

@bors-servo: r=ms2ger
@bors-servo
Copy link
Contributor

bors-servo commented Apr 20, 2016

📌 Commit d888ed3 has been approved by ms2ger
@bors-servo
Copy link
Contributor

bors-servo commented Apr 20, 2016

⌛ Testing commit d888ed3 with merge f051028...
bors-servo added a commit that referenced this pull request Apr 20, 2016
@bors-servo
Auto merge of #9942 - jdm:load_error, r=ms2ger
Loading status checks…
f051028 
Moving the error handling out of network loader

Rebase of #8851. Fixes #8678. Fixes #9944.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9942)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Apr 20, 2016

💔 Test failed - linux-rel
@KiChjang
Copy link
Member

KiChjang commented Apr 20, 2016 

Ran 4682 tests finished in 361.0 seconds.
  • 4680 ran as expected. 1363 tests skipped.
  • 1 tests failed unexpectedly
  • 1 tests had unexpected subtest results

Tests with unexpected results:
  ▶ Unexpected subtest result in /html/browsers/browsing-the-web/scroll-to-fragid/scroll-frag-percent-encoded.html:
  └ PASS [expected FAIL] Fragment Navigation: fragment id should be percent-decoded

  ▶ FAIL [expected PASS] /_mozilla/css/incremental_trailing_whitespace_a.html
  └   → /_mozilla/css/incremental_trailing_whitespace_a.html da54041086f2975f8e3c776d2283ad6609e6862a
/_mozilla/css/incremental_trailing_whitespace_ref.html f5d0147c8bddbdc772a7e5e86c7c9e433fcd486b
Testing da54041086f2975f8e3c776d2283ad6609e6862a == f5d0147c8bddbdc772a7e5e86c7c9e433fcd486b
@jdm
Copy link
Member Author

jdm commented Apr 20, 2016

@bors-servo: retry
@bors-servo
Copy link
Contributor

bors-servo commented Apr 20, 2016

⚡ Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css are reusable. Rebuilding only linux-rel, mac-rel-wpt...
@bors-servo
Copy link
Contributor

bors-servo commented Apr 20, 2016

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt
@bors-servo bors-servo merged commit d888ed3 into servo:master Apr 20, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
This was referenced Apr 20, 2016
Closed
Merged
Merged
Merged
Merged
@jdm jdm mentioned this pull request May 24, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Ms2ger Ms2ger

Labels
S-tests-failed
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants
@jdm @Ms2ger @bors-servo @larsbergstrom @frewsxcv @KiChjang @glennw @highfive @wafflespeanut
You can’t perform that action at this time.