Auto merge of #26395 - CYBAI:refactor-module-script, r=Manishearth

Move away from Promise.all way and check if we need to finish manually

In the previous Promise.all way, we registered a promise for every
module script which means we will need to do many complex checkings like
"is this top level?" and it will make us much more difficult to understand
how the module script algorithm works.

In the new manual checking way, we will only register promises for top
level modules to notify its owner (e.g. the script element) to finish
the load. So, we can understand it much more easily and would be more
spec-aligned.

Also, I think the `Ready` and `FetchFailed` status are quite confusing
and we actually don't need them so they're removed in this patch. Then,
we will always go to `Finished` instead.

It would basically be following steps:

```
                                          +-----------------+
                                          | Failed to fetch | ----------+
+--------------+       +----------+     / +-----------------+           |
| Fetch module | ----> | Fetching | ---+                                v
+--------------+       +----------+     \ +---------+        +---------------------+
                                          | Fetched |        | Advance to Finished |
                                          +---------+        +---------------------+
                                               |                        ^
                                               v                        |
                                      +-------------------+             |
                                      | Fetch descendants | ----- if no descendants
                                      +-------------------+
                                               |
                                               V
                                     +----------------------+
                                     | Fetching Descendants |
                                     +----------------------+
```

In `Advance to Finished`, it means that module script is about to finished so it will

1. Notify all of its `ready` and `not finished` parents to finish
2. Link (instantiate) the module
3. Resolve its promise to notify owner(s) to finish

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #25442
- [x] These changes do not require tests because it basically refactors the script module and should not break anything.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
This commit is contained in:
bors-servo 2020-06-13 08:33:54 -04:00 committed by GitHub
commit 2ce4bc5ad6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 563 additions and 631 deletions

View file

@ -870,52 +870,40 @@ impl HTMLScriptElement {
let global = window.upcast::<GlobalScope>();
let _aes = AutoEntryScript::new(&global);
if script.external {
let module_map = global.get_module_map().borrow();
let tree = if script.external {
global.get_module_map().borrow().get(&script.url).cloned()
} else {
global
.get_inline_module_map()
.borrow()
.get(&self.id.clone())
.cloned()
};
if let Some(module_tree) = module_map.get(&script.url) {
// Step 6.
{
let module_error = module_tree.get_error().borrow();
if module_error.is_some() {
module_tree.report_error(&global);
return;
}
}
let module_record = module_tree.get_record().borrow();
if let Some(record) = &*module_record {
let evaluated = module_tree.execute_module(global, record.handle());
if let Err(exception) = evaluated {
module_tree.set_error(Some(exception.clone()));
module_tree.report_error(&global);
return;
}
if let Some(module_tree) = tree {
// Step 6.
{
let module_error = module_tree.get_rethrow_error().borrow();
let network_error = module_tree.get_network_error().borrow();
if module_error.is_some() && network_error.is_none() {
module_tree.report_error(&global);
return;
}
}
} else {
let inline_module_map = global.get_inline_module_map().borrow();
if let Some(module_tree) = inline_module_map.get(&self.id.clone()) {
// Step 6.
{
let module_error = module_tree.get_error().borrow();
if module_error.is_some() {
module_tree.report_error(&global);
return;
}
}
let record = module_tree
.get_record()
.borrow()
.as_ref()
.map(|record| record.handle());
let module_record = module_tree.get_record().borrow();
if let Some(record) = &*module_record {
let evaluated = module_tree.execute_module(global, record.handle());
if let Some(record) = record {
let evaluated = module_tree.execute_module(global, record);
if let Err(exception) = evaluated {
module_tree.set_error(Some(exception.clone()));
module_tree.report_error(&global);
return;
}
if let Err(exception) = evaluated {
module_tree.set_rethrow_error(exception);
module_tree.report_error(&global);
return;
}
}
}

File diff suppressed because it is too large Load diff