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

Clear resources in a namespace #1445

Merged
merged 1 commit into from Jul 20, 2017
Merged

Clear resources in a namespace #1445

merged 1 commit into from Jul 20, 2017

Conversation

@kvark
Copy link
Member

kvark commented Jun 30, 2017

Builds upon #1435

cc @nical
r? @glennw


This change is Reviewable

@bors-servo
Copy link
Contributor

bors-servo commented Jun 30, 2017

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

@kvark kvark force-pushed the kvark:namespace branch 3 times, most recently from 0c6cd5b to 54d23fd Jun 30, 2017
@kvark kvark changed the title IdNamespace for everyone Clear resources in a namespace Jul 1, 2017
@@ -764,6 +764,24 @@ impl ResourceCache {
debug_assert_eq!(self.state, State::QueryResources);
self.state = State::Idle;
}

pub fn clear_namespace(&mut self, namespace: IdNamespace) {
//TODO: use `retain` when we are on Rust-1.18

This comment has been minimized.

@glennw

glennw Jul 5, 2017

Member

We'll want to also remove the items from the texture cache, if they exist in there, I guess?

This comment has been minimized.

@nical

nical Jul 6, 2017

Collaborator

These would eventually expire on their own, right? But yeah sounds like it would be a good thing to do as well, and also remove items from the texture cache when removing image and font templates.

@kvark kvark closed this Jul 10, 2017
@kvark kvark deleted the kvark:namespace branch Jul 10, 2017
@kvark kvark restored the kvark:namespace branch Jul 10, 2017
@kvark
Copy link
Member Author

kvark commented Jul 10, 2017

Oops, reopening

@kvark kvark reopened this Jul 10, 2017
@glennw
Copy link
Member

glennw commented Jul 18, 2017

@kvark I think this just needs an update to remove items from the texture cache, if that's feasible.

@kvark kvark force-pushed the kvark:namespace branch from 54d23fd to ea7b268 Jul 20, 2017
@kvark
Copy link
Member Author

kvark commented Jul 20, 2017

@glennw please take another look

@glennw
glennw approved these changes Jul 20, 2017
@glennw
Copy link
Member

glennw commented Jul 20, 2017

@kvark Looks good! r=me, do you need to remove the text-shadow commit that is in here? Or will git handle that cleanly?

@kvark kvark force-pushed the kvark:namespace branch from ea7b268 to 6a2662c Jul 20, 2017
@glennw
Copy link
Member

glennw commented Jul 20, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Jul 20, 2017

📌 Commit 6a2662c has been approved by glennw

@bors-servo
Copy link
Contributor

bors-servo commented Jul 20, 2017

Testing commit 6a2662c with merge dc746ed...

bors-servo added a commit that referenced this pull request Jul 20, 2017
Clear resources in a namespace

Builds upon #1435

cc @nical
r? @glennw

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

bors-servo commented Jul 20, 2017

☀️ Test successful - status-travis
Approved by: glennw
Pushing dc746ed to master...

@bors-servo bors-servo merged commit 6a2662c into servo:master Jul 20, 2017
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@kvark kvark deleted the kvark:namespace branch Jul 26, 2018
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

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