Skip to content
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

Move remaining users of the legacy networking stack to fetch. #13961

Merged
merged 13 commits into from Nov 2, 2016
Merged

Conversation

@Ms2ger
Copy link
Contributor

Ms2ger commented Oct 28, 2016

Fixes #13931.
Fixes #13714.


This change is Reviewable

@highfive
Copy link

highfive commented Oct 28, 2016

Heads up! This PR modifies the following files:

  • @fitzgen: components/script/network_listener.rs, components/script/dom/workerglobalscope.rs, components/script/dom/dedicatedworkerglobalscope.rs, components/script/script_thread.rs, components/script/dom/servoparser/mod.rs, components/script_traits/lib.rs, components/script_traits/lib.rs, components/script/dom/serviceworkerglobalscope.rs
  • @KiChjang: components/net/fetch/methods.rs, components/net/resource_thread.rs, components/net/http_loader.rs, components/script/network_listener.rs, components/script/dom/workerglobalscope.rs, components/script/dom/dedicatedworkerglobalscope.rs, components/net_traits/lib.rs, components/net_traits/lib.rs, components/script/script_thread.rs, components/script/dom/servoparser/mod.rs, components/script_traits/lib.rs, components/script_traits/lib.rs, components/net_traits/response.rs, components/net_traits/response.rs, components/script/dom/serviceworkerglobalscope.rs, components/net/image_cache_thread.rs
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Oct 28, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Oct 28, 2016

Trying commit 9856ccf with merge b883928...

bors-servo added a commit that referenced this pull request Oct 28, 2016
[WIP] Sync fetch
@bors-servo
Copy link
Contributor

bors-servo commented Oct 28, 2016

💔 Test failed - linux-rel-wpt

@jdm
Copy link
Member

jdm commented Oct 29, 2016

@bors-servo: retry

@bors-servo
Copy link
Contributor

bors-servo commented Oct 29, 2016

Trying commit 9856ccf with merge 1b77dfa...

bors-servo added a commit that referenced this pull request Oct 29, 2016
[WIP] Sync fetch

<!-- 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/13961)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 29, 2016

💔 Test failed - mac-rel-wpt2

@@ -1148,8 +1148,14 @@ fn http_network_fetch<UI: 'static + UIProvider>(request: Rc<Request>,
// TODO this step isn't possible yet
// Step 13

// Step 14.
if credentials_flag {
// TODO: should block cookies?

This comment has been minimized.

@KiChjang

KiChjang Oct 30, 2016

Member

The context parameter contains an http_state which in turns contains a content_blocker of type RuleList which has rules that tell us whether cookies should be blocked.

@Ms2ger
Copy link
Contributor Author

Ms2ger commented Oct 31, 2016

  ▶ CRASH [expected OK] /workers/Worker_cross_origin_security_err.htm
  │ 
  │ VMware, Inc.
  │ Gallium 0.4 on softpipe
  │ 3.3 (Core Profile) Mesa 12.0.1
  │ Unimplemented scheme for Fetch (thread fetch thread for ftp://example.org/support/WorkerBasic.js, at /Users/servo/buildbot/slave/mac-rel-wpt2/build/components/net/fetch/methods.rs:496)
  │ stack backtrace:
  │    0:        0x10923c63e - backtrace::backtrace::trace::h6ac07ced1b846a59
  │    1:        0x10923c92c - backtrace::capture::Backtrace::new::h1d3339ed08c7f861
  │    2:        0x108018d74 - servo::main::_{{closure}}::hce6009c73746ec98
  │    3:        0x1099d55f3 - std::panicking::rust_panic_with_hook::hcd9d05f53fa0dafc
  │    4:        0x10818f694 - std::panicking::begin_panic::hc03e2830c2c89a5f
  │    5:        0x108276780 - net::fetch::methods::basic_fetch::ha806911f4cd925a5
  │    6:        0x108272a9a - net::fetch::methods::main_fetch::hb454a78ecbf1c83c
  │    7:        0x10819053a - std::panicking::try::do_call::h1db0bf7c9a597e4b
  │    8:        0x1099d661a - __rust_maybe_catch_panic
  │    9:        0x1081cc206 - _<F as alloc..boxed..FnBox<A>>::call_box::h3a6dba867d2cc296
  │   10:        0x1099d44a4 - std::sys::thread::Thread::new::thread_start::h50b05608a499d2b2
  │   11:     0x7fff88879059 - _pthread_body
  │   12:     0x7fff88878fd6 - _pthread_start
  │ ERROR:servo: Unimplemented scheme for Fetch
  │ Pipeline failed in hard-fail mode.  Crashing!
  └ thread panicked while processing panic. aborting.

Looks like that's the only problem. I think I'll just change to returning network errors for ftp fetches.

r? @jdm, but review the dependent PRs first :)

@jdm jdm assigned jdm and unassigned glennw Oct 31, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Nov 2, 2016

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

@Ms2ger Ms2ger force-pushed the sync-fetch branch from 9856ccf to ebae89a Nov 2, 2016
@Ms2ger Ms2ger force-pushed the sync-fetch branch from ebae89a to 98ed8d7 Nov 2, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Nov 2, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Nov 2, 2016

Trying commit 98ed8d7 with merge 8a975c8...

bors-servo added a commit that referenced this pull request Nov 2, 2016
[WIP] Sync fetch

<!-- 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/13961)

<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 2, 2016

@jdm
Copy link
Member

jdm commented Nov 2, 2016

This is good to merge once #13931 merges.

@Ms2ger Ms2ger force-pushed the sync-fetch branch from 98ed8d7 to 9213198 Nov 2, 2016
@Ms2ger Ms2ger force-pushed the sync-fetch branch from 9213198 to cecd60b Nov 2, 2016
@Ms2ger Ms2ger changed the title [WIP] Sync fetch Move remaining users of the legacy networking stack to fetch. Nov 2, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Nov 2, 2016

@bors-servo r=jdm

Let's land them all at once.

@bors-servo
Copy link
Contributor

bors-servo commented Nov 2, 2016

📌 Commit cecd60b has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Nov 2, 2016

Testing commit cecd60b with merge 56f4a69...

bors-servo added a commit that referenced this pull request Nov 2, 2016
Move remaining users of the legacy networking stack to fetch.

Fixes #13931.
Fixes #13714.
@bors-servo
Copy link
Contributor

bors-servo commented Nov 2, 2016

@bors-servo bors-servo merged commit cecd60b into master Nov 2, 2016
2 of 4 checks passed
2 of 4 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
dependency-ci Failed dependency checks
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@SimonSapin SimonSapin deleted the sync-fetch branch Nov 3, 2016
@Ms2ger Ms2ger mentioned this pull request Nov 7, 2016
31 of 31 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.