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

Implement blob url support in the fetch stack. #13750

Merged
merged 7 commits into from Oct 14, 2016
Merged

Implement blob url support in the fetch stack. #13750

merged 7 commits into from Oct 14, 2016

Conversation

@Ms2ger
Copy link
Contributor

Ms2ger commented Oct 13, 2016

This change is Reviewable

@highfive
Copy link

highfive commented Oct 13, 2016

Heads up! This PR modifies the following files:

  • @KiChjang: components/net/filemanager_thread.rs, components/net/fetch/methods.rs, components/net/resource_thread.rs, components/net/blob_loader.rs
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Oct 13, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Oct 13, 2016

Trying commit a3760c2 with merge 30effbe...

bors-servo added a commit that referenced this pull request Oct 13, 2016
Implement blob url support in the fetch stack.
@Ms2ger Ms2ger force-pushed the fetch-blob branch 3 times, most recently from 8ce01a9 to 699287e Oct 13, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Oct 14, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Oct 14, 2016

Trying commit 699287e with merge ff2ad32...

bors-servo added a commit that referenced this pull request Oct 14, 2016
Implement blob url support in the fetch stack.

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

bors-servo commented Oct 14, 2016

💔 Test failed - linux-rel-wpt

@Ms2ger Ms2ger force-pushed the fetch-blob branch from 699287e to bad10a1 Oct 14, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Oct 14, 2016

@highfive highfive assigned Manishearth and unassigned emilio Oct 14, 2016
Copy link
Member

Manishearth left a comment

There should be passing tests here, if not add some.

let _ = sender.send(Err(FileManagerThreadError::BlobURLStoreError(e)));
}
})
if let Err(e) = store.try_read_file(&sender, id, check_url_validity,

This comment has been minimized.

@Manishearth

Manishearth Oct 14, 2016

Member

file I/O is slow too, shouldn't we spawn a thread for that? ditto for promotememory

@@ -119,3 +120,71 @@ fn load_blob<UI: 'static + UIProvider>
send_error(load_data.url.clone(), format_err, start_chan);
}
}

// TODO: make async.
pub fn load_blob_sync<UI: 'static + UIProvider>

This comment has been minimized.

@Manishearth

Manishearth Oct 14, 2016

Member

spec link

Ms2ger added 3 commits Oct 13, 2016
The spawned threads remain for select_file and select_files, as those may
need to wait indefinitely for the user's response.
@Ms2ger Ms2ger force-pushed the fetch-blob branch from bad10a1 to 6eaef7c Oct 14, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Oct 14, 2016

@bors-servo r=Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Oct 14, 2016

📌 Commit 6eaef7c has been approved by Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Oct 14, 2016

Testing commit 6eaef7c with merge d692cf1...

bors-servo added a commit that referenced this pull request Oct 14, 2016
Implement blob url support in the fetch stack.

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

bors-servo commented Oct 14, 2016

@bors-servo bors-servo merged commit 6eaef7c into master Oct 14, 2016
3 of 4 checks passed
3 of 4 checks passed
dependency-ci Failed dependency checks
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@Ms2ger Ms2ger deleted the fetch-blob branch Oct 19, 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

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