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

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

Merged
merged 1 commit into from Jun 13, 2020
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

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
  • Loading branch information
CYBAI committed Jun 13, 2020
commit dc2c2c8dfb0e56dd6714c19f14c822825360ffac
@@ -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();

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());
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 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;
}
}
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.