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

Remove the dependency of fetch on filemanager_thread::UIProvider. #14275

Merged
merged 5 commits into from Nov 22, 2016
Merged

Remove the dependency of fetch on filemanager_thread::UIProvider. #14275

merged 5 commits into from Nov 22, 2016

Conversation

@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 18, 2016 •
edited by larsbergstrom

This change is Reviewable
@highfive
Copy link

highfive commented Nov 18, 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
@highfive
Copy link

highfive commented Nov 18, 2016

warning Warning warning

  • These commits modify net code, but no tests are modified. Please consider adding a test!
@Ms2ger Ms2ger mentioned this pull request Nov 18, 2016
Closed
31 of 31 tasks complete
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Nov 18, 2016

Fails unit tests, hold on.
@Ms2ger Ms2ger force-pushed the fm-ui branch from 44843c1 to 45073e5 Nov 18, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Nov 18, 2016

Fixed.
Ms2ger added 5 commits Nov 18, 2016
@Ms2ger
Move the UIProvider from FileManagerStore to FileManager.
95ccd2d 
This will allow us to reduce the amount of code that needs to care about it.
@Ms2ger
Stop cloning FileManager::store so much.
508e6ad
@Ms2ger
Factor out FileManager::read_file().
5ce869d
@Ms2ger
Factor out FileManager::promote_memory().
f672bf9
@Ms2ger
Pass the UIProvider to FileManager::handle() as needed.
Loading status checks…
ae1340b
@Ms2ger Ms2ger force-pushed the fm-ui branch from 45073e5 to ae1340b Nov 21, 2016
@jdm
jdm requested changes Nov 21, 2016
View changes
components/net/filemanager_thread.rs Outdated
@@ -112,26 +112,15 @@ enum FileImpl {
Sliced(Uuid, RelativePos),
}

pub struct FileManager<UI: 'static + UIProvider> {
#[derive(Clone)]

This comment has been minimized.

Sign in to view
Copy link
@jdm

jdm Nov 21, 2016

Member

Is this necessary?
@jdm jdm assigned jdm and unassigned metajack Nov 21, 2016
@jdm
Copy link
Member

jdm commented Nov 21, 2016

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

bors-servo commented Nov 21, 2016

📌 Commit ae1340b has been approved by jdm
@jdm
jdm approved these changes Nov 21, 2016
View changes
@bors-servo
Copy link
Contributor

bors-servo commented Nov 22, 2016

⌛ Testing commit ae1340b with merge d287f58...
bors-servo added a commit that referenced this pull request Nov 22, 2016
@bors-servo
Auto merge of #14275 - servo:fm-ui, r=jdm
Loading status checks…
d287f58 
Remove the dependency of fetch on filemanager_thread::UIProvider.

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

bors-servo commented Nov 22, 2016

☀️ Test successful - 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 ae1340b into master Nov 22, 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 fm-ui branch Nov 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdm jdm

Assignees

@jdm jdm

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 @jdm @bors-servo @metajack
You can’t perform that action at this time.