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

Make the fetch target non-optional. #14465

Merged
merged 9 commits into from Dec 15, 2016
Merged

Make the fetch target non-optional. #14465

merged 9 commits into from Dec 15, 2016

Conversation

@Ms2ger
Copy link
Contributor

Ms2ger commented Dec 5, 2016 •
edited by larsbergstrom

This change is Reviewable
@highfive
Copy link

highfive commented Dec 5, 2016

Heads up! This PR modifies the following files:

  • @KiChjang: components/net/fetch/methods.rs, components/net/resource_thread.rs, components/net/http_loader.rs
@Ms2ger Ms2ger force-pushed the http branch from 001c5cc to 426554a Dec 6, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Dec 14, 2016

☔ The latest upstream changes (presumably #14570) made this pull request unmergeable. Please resolve the merge conflicts.
Ms2ger added 9 commits Dec 2, 2016
@Ms2ger
Avoid unlocking the response body while it is in an inconsistent state.
8a4a5c0
@Ms2ger
Qualify the fetch() calls in unit tests.
8d34ef1
@Ms2ger
Introduce fetch_with_context() to reduce repetition.
9d9f048
@Ms2ger
Replace fetch_async() by a new fetch() function.
c03cd45 
fetch() returns immediately after processing EOF, at the latest, so not
spinning up a thread should not cause noticeable delays. OTOH, it might
reduce the contention for cores, and reduce the overall time needed.
@Ms2ger
Use the asynchronous fetching code in unit tests.
a5efc01 
This is the only code that is used in Servo proper, so it's a more useful
thing to test.
@Ms2ger
Use the asynchronous fetching code for fetch_with_context().
306905a
@Ms2ger
Introduce fetch_with_cors_cache() API for tests.
6f2606c
@Ms2ger
Make the fetch target non-optional.
217f44b
@Ms2ger
Stop returning the response from fetch().
Loading status checks…
1e0ab08
@Ms2ger Ms2ger force-pushed the http branch from 4759152 to 1e0ab08 Dec 15, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Dec 15, 2016

r?
@nox
Copy link
Member

nox commented Dec 15, 2016

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Dec 15, 2016

📌 Commit 1e0ab08 has been approved by nox
@bors-servo
Copy link
Contributor

bors-servo commented Dec 15, 2016

⌛ Testing commit 1e0ab08 with merge 911dd49...
bors-servo added a commit that referenced this pull request Dec 15, 2016
@bors-servo
Auto merge of #14465 - servo:http, r=nox
Loading status checks…
911dd49 
Make the fetch target non-optional.

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

bors-servo commented Dec 15, 2016

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

KiChjang commented Dec 15, 2016 

Ran 7917 tests finished in 883.0 seconds.
  • 7915 ran as expected. 1890 tests skipped.
  • 1 tests failed unexpectedly
  • 1 tests had unexpected subtest results

Tests with unexpected results:
  ▶ FAIL [expected PASS] /_mozilla/mozilla/simple_scroll_to_fragment.html
  └   → /_mozilla/mozilla/simple_scroll_to_fragment.html 1a9f5b056d9d5c7e985e172701179543d007e367
/_mozilla/mozilla/simple_scroll_to_fragment_ref.html 111b0141f71ae09366d1ce49dcece7c79df65484
Testing 1a9f5b056d9d5c7e985e172701179543d007e367 == 111b0141f71ae09366d1ce49dcece7c79df65484

  ▶ Unexpected subtest result in /html/semantics/embedded-content/the-iframe-element/iframe-allowfullscreen.html:
  └ PASS [expected FAIL] iframe-noload-noallowfullscreen

  ▶ Unexpected subtest result in /html/semantics/embedded-content/the-iframe-element/iframe-allowfullscreen.html:
  └ PASS [expected FAIL] iframe-noload-allowfullscreen
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Dec 15, 2016 •
edited by KiChjang

@bors-servo retry #14606, #14607
@bors-servo
Copy link
Contributor

bors-servo commented Dec 15, 2016

⌛ Testing commit 1e0ab08 with merge 1b2daae...
bors-servo added a commit that referenced this pull request Dec 15, 2016
@bors-servo
Auto merge of #14465 - servo:http, r=nox
Loading status checks…
1b2daae 
Make the fetch target non-optional.

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

bors-servo commented Dec 15, 2016

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev
@bors-servo bors-servo merged commit 1e0ab08 into master Dec 15, 2016
4 checks passed
4 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
dependency-ci Dependencies checked
Details
homu Test successful
Details
@Ms2ger Ms2ger deleted the http branch Dec 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nox nox

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@Ms2ger @highfive @bors-servo @nox @KiChjang
You can’t perform that action at this time.