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

Create a mechanism to automatically reset relevant members of Document during Document.open #15371

Open
jdm opened this issue Feb 3, 2017 · 7 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Feb 3, 2017

There are a lot of members that need to be reset to initial states during Document.open. Right now it's just a list of reinitializations in that method, which is not very maintainable. We should have a structure that contains the members that need to be cleared, and auto-derive a trait for every member. This will make it impossible to forget to clear a member, and will make it easy to tell if a member is reinitialized or not by looking at the member initialization list for Document.

@Dylan-DPC
Copy link

@Dylan-DPC Dylan-DPC commented Feb 15, 2018

Hey. I can take this up

@jdm
Copy link
Member Author

@jdm jdm commented Feb 15, 2018

That would be great! Please ask questions if anything is unclear!

@jdm jdm added the C-assigned label Feb 15, 2018
@jdm
Copy link
Member Author

@jdm jdm commented Mar 25, 2018

@Dylan-DPC Are you still working on this?

@Dylan-DPC
Copy link

@Dylan-DPC Dylan-DPC commented Mar 25, 2018

Hey got a bit busy so didn't get time. If anyone else wants to take it then fine else i'll look into it during the coming weekend.

@jdm
Copy link
Member Author

@jdm jdm commented Mar 25, 2018

I'll mark it unassigned in the time being. Let us know if you start working on it!

@jdm jdm removed the C-assigned label Mar 25, 2018
@nipunG314
Copy link

@nipunG314 nipunG314 commented Dec 16, 2019

Hi. I would like to take this up if this is still pending.

Just to clarify, I have to refactor the Open function residing in components/script/dom/document.rs.

fn Open(
&self,
_unused1: Option<DOMString>,
_unused2: Option<DOMString>,
) -> Fallible<DomRoot<Document>> {

I can see that this method consists of a number of steps. Steps 8 to 15 seem to responsible for reinitializing the members of the Document struct. So when I create a struct to automatically reset the relevant members, there would be one auto-derived trait for each step. And the corresponding step's code would be moved into that trait.

Is that correct, or have I missed anything?

// Step 8
for node in self
.upcast::<Node>()
.traverse_preorder(ShadowIncluding::Yes)
{
node.upcast::<EventTarget>().remove_all_listeners();
}
// Step 9
if self.window.Document() == DomRoot::from_ref(self) {
self.window.upcast::<EventTarget>().remove_all_listeners();
}
// Step 10
// TODO: https://github.com/servo/servo/issues/21936
Node::replace_all(None, self.upcast::<Node>());
// Step 11
if self.is_fully_active() {
let mut new_url = entry_responsible_document.url();
if entry_responsible_document != DomRoot::from_ref(self) {
new_url.set_fragment(None);
}
// TODO: https://github.com/servo/servo/issues/21939
self.set_url(new_url);
}
// Step 12
// TODO: https://github.com/servo/servo/issues/21938
// Step 13
self.set_quirks_mode(QuirksMode::NoQuirks);
// Step 14
let resource_threads = self
.window
.upcast::<GlobalScope>()
.resource_threads()
.clone();
*self.loader.borrow_mut() =
DocumentLoader::new_with_threads(resource_threads, Some(self.url()));
ServoParser::parse_html_script_input(self, self.url());
// Step 15
self.ready_state.set(DocumentReadyState::Loading);

@jdm
Copy link
Member Author

@jdm jdm commented Dec 16, 2019

I suspect this code has changed in the 2.5 years since I filed this issue. I'm pretty sure the code I had mind back when was this code, but it's not clear to me if my proposal still makes sense.

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
3 participants
You can’t perform that action at this time.