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

Change RefCells in the DOM to a custom JSRefCell/DOMRefCell #3050

Closed
pcwalton opened this issue Aug 7, 2014 · 4 comments
Closed

Change RefCells in the DOM to a custom JSRefCell/DOMRefCell #3050

pcwalton opened this issue Aug 7, 2014 · 4 comments

Comments

@pcwalton
Copy link
Contributor

@pcwalton pcwalton commented Aug 7, 2014

This ref cell would be just like the built-in ref cell but it would do two things:

  1. Calling any borrow/borrow-mut method would ensure this is not the layout thread in debug builds.
  2. A new special borrow-in-layout method would ensure that this is the layout thread in debug builds.

This would help prevent problems like the one that broke the in-progress Rust upgrade and would make our code nicer as well.

@tetsuharuohzeki
Copy link
Member

@tetsuharuohzeki tetsuharuohzeki commented Aug 9, 2014

Does this import cell.rs from rust master and custom it?
Or we introduce a new sub-type object like this?:

struct DOMRefCell<T> {
  base: RefCell<T>,
  ...
}
impl<T> DOMRefCell<T> {
  ...
}
@jdm
Copy link
Member

@jdm jdm commented Aug 10, 2014

I think it makes sense to wrap the original type like your example.

@jdm
Copy link
Member

@jdm jdm commented Oct 16, 2014

To implement the safety assertions, let's add a local_data! in layout_interface that LayoutTask::new initializes. Borrow and borrow_in_layout can check it inside cfg!(debug) blocks, since I'm pretty sure task-local data is relatively slow right now.

bors-servo pushed a commit that referenced this issue Oct 16, 2014
@tetsuharuohzeki
Copy link
Member

@tetsuharuohzeki tetsuharuohzeki commented Oct 16, 2014

Once we'll replace all usage of RefCell in script to DOMRefCell for the above safety checking. I'll start to work it :)

bors-servo pushed a commit that referenced this issue Oct 21, 2014
#3050

Altough LayoutDataRef is touched from layout, we don't use DOMRefCell in it becasuse
it's expected to manipulate in layout task.
bors-servo pushed a commit that referenced this issue Oct 21, 2014
#3050

Altough LayoutDataRef is touched from layout, we don't use DOMRefCell in it becasuse
it's expected to manipulate in layout task.
bors-servo pushed a commit that referenced this issue Oct 22, 2014
#3050

Altough LayoutDataRef is touched from layout, we don't use DOMRefCell in it becasuse
it's expected to manipulate in layout task.
bors-servo pushed a commit that referenced this issue Oct 22, 2014
#3050

Altough LayoutDataRef is touched from layout, we don't use DOMRefCell in it becasuse
it's expected to manipulate in layout task.
bors-servo pushed a commit that referenced this issue Oct 22, 2014
#3050

Altough LayoutDataRef is touched from layout, we don't use DOMRefCell in it becasuse
it's expected to manipulate in layout task.
@Ms2ger Ms2ger closed this Nov 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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