Switch the remaining `SafeJSContext` usages to `&mut JSContext` inside
script_module.rs
Testing: It compiles.
Part of #40600
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Drop the `ModuleOwner` logic in favour of closures passed down by the
script fetching initiator.
When processing inline module scripts a task in now queued on the
networking task source. Since `Rc<ModuleTree>` is not `Send`, a `result`
field is now introduced to `HTMLScriptElement`, which is initialized
before queueing the task.
This slightly improves `inline-async-inserted-execorder.html`, which now
fails at the fourth assertion instead of stopping at the second one (the
inline module script with no dependencies still resolves after the one
that has a parse error).
Testing: Covered by existing tests.
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Our implementation of [fetch a single module
script](https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-single-module-script)
was missing the task queueing steps:
```
5. If moduleMap[(url, moduleType)] is "fetching", wait in parallel until that entry's value changes,
then queue a task on the networking task source to proceed with running the following steps.
6. If moduleMap[(url, moduleType)] exists, run onComplete given moduleMap[(url, moduleType)], and return.
```
Instead we appended a `PromiseNativeHandler`, which would run
`on_complete` when resolved, on the promise of the pending fetch.
Testing: This change shouldn't be observable since modules are evaluated
in sync, but it's required for #39417.
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Removed ```introduction_type_override``` field from
```HTMLScriptElement``` & passed the new variable to
```fetch_inline_module_script```
Testing: No testing required, compiles successfully.
Fixes: #43980
Signed-off-by: Sabb <sarafaabbas@gmail.com>
`blob` URLs have a implicit blob URL entry attached, which stores the
data contained in the blob. The specification requires this entry to be
resolved as the URL is parsed. We only resolve it inside `net` when
loading the URL. That causes problems if the blob entry has been revoked
in the meantime - see https://github.com/servo/servo/issues/25226.
Ideally we would want to resolve blobs at parse-time as required. But
because `ServoUrl` is such a fundamental type, I've not managed to do
this change without having to touch hundreds of files at once.
Thus, we now require passing a `UrlWithBlobClaim` instead of a
`ServoUrl` when `fetch`-ing. This type proves that the caller has
acquired the blob beforehand.
As a temporary escape hatch, I've added
`UrlWithBlobClaim::from_url_without_having_claimed_blob`. That method
logs a warning if its used unsafely. This method is currently used in
most places to keep this change small. Only workers now acquire the blob
beforehand.
Testing: A new test starts to pass
Part of https://github.com/servo/servo/issues/43326
Part of https://github.com/servo/servo/issues/25226
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
Implement render-blocking option for script fetch and load events
## Testing:
### Test summary on main branch
```bash
▶ OK [expected CRASH] /html/semantics/scripting-1/the-script-element/moving-between-documents/move-back-createHTMLDocument-success-external-module.html
Ran 470 tests finished in 217.9 seconds.
• 453 ran as expected.
• 4 tests crashed unexpectedly
• 1 tests timed out unexpectedly
• 11 tests unexpectedly okay
• 2 tests had unexpected subtest results
/home/elomscansio/.local/share/uv/python/cpython-3.11.15-linux-x86_64-gnu/lib/python3.11/multiprocessing/resource_tracker.py:254: UserWarning: resource_tracker: There appear to be 3 leaked semaphore objects to clean up at shutdown
warnings.warn('resource_tracker: There appear to be %d '
```
### Test summary for this commits
```bash
Ran 470 tests finished in 488.0 seconds.
• 397 ran as expected.
• 15 tests crashed unexpectedly
• 27 tests timed out unexpectedly
• 11 tests unexpectedly okay
• 28 tests had unexpected subtest results
```
Fixes: #43697Fixes: #43354
---------
Signed-off-by: Emmanuel Paul Elom <elomemmanuel007@gmail.com>
Signed-off-by: elomscansio <163124154+elomscansio@users.noreply.github.com>
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Co-authored-by: Gae24 <96017547+Gae24@users.noreply.github.com>
All callers were constructing a DOMString only to have it immediately
converted back to String inside internal_warn, resulting in an
unnecessary allocation and clone. This changes the parameter type to
String and updates all call sites to pass the result of format! or
.to_string() directly.
Fixes: #43091
---------
Signed-off-by: Kelechi Ebiri <ebiritg@gmail.com>
According to spec we should fetch worker modules using the _outside
settings_, however we can't use it directly since we are on a different
thread. To fix it, a new struct `ModuleFetchClient` is now part of
`LoadState` and replace the `with_global_scope` call done in
`fetch_a_single_module_script`.
This also tidy worker classic script related code.
Testing: There are new passes
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
This is follow-up of #41459.
We replace some of these:
```rust
let mut cx = unsafe { script_bindings::script_runtime::temp_cx() };
```
..by passing a `cx: &mut JSContext` instead.
---
Testing: existing WPT tests
Fixes: #43453
Signed-off-by: pylbrecht <pylbrecht@mailbox.org>
Remove `referrer` field from `ScriptFetchOptions` and pass it directly
to `script_fetch_request`.
This keeps ScriptFetchOptions aligned with the HTML
fixes#42875
---------
Signed-off-by: Kelechi Ebiri <ebiritg@gmail.com>
This is no longer present in the spec. Instead, the
`process_request_body` is the new way. These two
methods were called right after each other and there was only 1
implementation in `htmlvideoelement`. That implementation is now moved
to `process_request_body` and hence we can remove the unnecessary
method.
Testing: It compiles
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Pass `&mut JSContext` to `HTMLScriptElement::execute`, propagate it
inside `global_scope_script_execution.rs` and then switch to wrappers2
bindings.
Testing: A successful build is enough.
Part of #40600
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
While we now pass a lot of worker tests, we still fail a bunch:
```
% fd '^worker-.*.html.ini'
tests/wpt/meta/content-security-policy/gen/top.http-rp/script-src-self/worker-import.http.html.ini
tests/wpt/meta/content-security-policy/gen/top.http-rp/script-src-self/worker-import.https.html.ini
tests/wpt/meta/content-security-policy/gen/top.http-rp/worker-src-self/worker-import.http.html.ini
tests/wpt/meta/content-security-policy/gen/top.http-rp/worker-src-self/worker-import.https.html.ini
tests/wpt/meta/content-security-policy/gen/top.meta/script-src-self/worker-import.http.html.ini
tests/wpt/meta/content-security-policy/gen/top.meta/script-src-self/worker-import.https.html.ini
tests/wpt/meta/content-security-policy/gen/top.meta/worker-src-self/worker-import.http.html.ini
tests/wpt/meta/content-security-policy/gen/top.meta/worker-src-self/worker-import.https.html.ini
tests/wpt/meta/content-security-policy/script-src/worker-data-set-timeout.sub.html.ini
tests/wpt/meta/content-security-policy/script-src/worker-importscripts.sub.html.ini
tests/wpt/meta/content-security-policy/script-src/worker-set-timeout.sub.html.ini
tests/wpt/meta/fetch/metadata/generated/worker-dedicated-constructor.sub.html.ini
tests/wpt/meta/fetch/metadata/generated/worker-dedicated-importscripts.https.sub.html.ini
tests/wpt/meta/fetch/metadata/generated/worker-dedicated-importscripts.sub.html.ini
tests/wpt/meta/mixed-content/gen/top.http-rp/opt-in/worker-import-data.https.html.ini
tests/wpt/meta/mixed-content/gen/top.http-rp/opt-in/worker-import.https.html.ini
tests/wpt/meta/referrer-policy/generic/subresource-test/worker-messaging.html.ini
tests/wpt/meta/service-workers/service-worker/worker-client-id.https.html.ini
tests/wpt/meta/service-workers/service-worker/worker-in-sandboxed-iframe-by-csp-fetch-event.https.html.ini
tests/wpt/meta/service-workers/service-worker/worker-interception-redirect.https.html.ini
tests/wpt/meta/service-workers/service-worker/worker-interception.https.html.ini
tests/wpt/meta/upgrade-insecure-requests/gen/worker-classic.http-rp/upgrade/worker-classic.https.html.ini
tests/wpt/meta/upgrade-insecure-requests/gen/worker-classic.http-rp/upgrade/worker-module.https.html.ini
tests/wpt/meta/upgrade-insecure-requests/gen/worker-module.http-rp/upgrade/worker-classic.https.html.ini
tests/wpt/meta/upgrade-insecure-requests/gen/worker-module.http-rp/upgrade/worker-module.https.html.ini
tests/wpt/meta/wasm/webapi/esm-integration/worker-import-source-phase.tentative.html.ini
tests/wpt/meta/wasm/webapi/esm-integration/worker-import.tentative.html.ini
tests/wpt/meta/workers/Worker-creation-happens-in-parallel.https.html.ini
tests/wpt/meta/workers/Worker-postMessage-happens-in-parallel.https.html.ini
tests/wpt/meta/workers/Worker-terminate-forever-during-evaluation.html.ini
tests/wpt/meta/workers/worker-request-animation-frame.html.ini
```
Also, we are passing `Option<PolicyContainer>` down the call stack
through `LoadState`, which is more of a workaround. However, since
maintaining this long-lived branch is becoming a bit painful (merge
conflicts) and I was starting to lose momentum because of that, I would
like get this merged rather sooner than later.
We will address the failing tests and find the right place for
`PolicyContainer` in follow-ups, including the added
`#[allow(clippy::too_many_arguments)]`.
# See also
* https://html.spec.whatwg.org/multipage/#worker-processing-model
*
https://html.spec.whatwg.org/multipage/#fetch-a-module-worker-script-tree
---
Fixes: #23308
Testing: WPT tests
Signed-off-by: pylbrecht <pylbrecht@mailbox.org>
Co-authored-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Add the cx parameter to `fn process_response` in the
`FetchResponseListener` trait and the traits that that interface change
requires. Chose to add it as the first parameter, following the same
order that `FetchResponseListener::process_response_eof` uses.
Testing: Checked that servo builds locally as well as `./mach fmt` and
`./mach test-tidy`. I don't think more tests are needed as we are not
introducing new functionality
Fixes: #42840
---------
Signed-off-by: Javier Olaechea <pirata@gmail.com>
Add support to modulepreload link elements. Currently we only fetch the
root module, as I think it is needs to be discussed whether we should
always fetch module dependencies, or if it should be limited due to
network constraints.
Inside `bind_to_tree` I ended up calling
`fetch_and_process_modulepreload` inside a delayed task, since it would
cause a crash due to DOM not being in a stable state when firing an
event (queueing the event seems to also do the trick).
Remaining tests failures are due to not supporting CSS modules and
performance entries's `transfer_size` being 0.
Testing: Covered by existing tests
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
For the longest time we've had a `dispatch_event` boolean flag in our
error reporting code that determines whether or not to fire an `error`
event. It was only set to `false` in the event handler code due to a
double borrow that would occur otherwise. It seems that this is no
longer a problem, so we can remove the flag and correctly fire the event
in that case too.
Testing: A new test starts to pass and
`html/webappapis/scripting/processing-model-2/runtime-error-in-body-onerror.html`
does not crash
Fixes: https://github.com/servo/servo/issues/13152
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
This method is the same as `DOMString::from` with a `String` argument
and `From` and `Into` are preferred when writing modern Rust.
Testing: This should not change behavior and is thus covered by existing
tests.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
I only wanted to get `&mut JSContext` in microtask chunk and checkpoint,
but this in turn needed `&mut JSContext` in servoparser, which then
caused need for even more changes in script.
I tried to limit the size by putting some `temp_cx` in:
- drops of `LoadBlocker`, `GenericAutoEntryScript`
- methods of `VirtualMethods`
- methods of `FetchResponseListener`
Testing: Just refactor, but should be covered by WPT tests.
Part of #40600
---------
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
Previously, the resolution loop would break immediately after running
`resolve_imports_match` against the first module specifier map of the
import map's scopes, regardless of whether a match was actually found.
Testing: All sub-tests of `resolving.html` now pass
Part of #37553
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Unless the feature `preserve_order` is enabled, serde_json will sort the
map's entries in an alphanumerical order.
The specification instead expect the entries to be visited by the order
they are listed, a latter entry should replace a previous one when they
are resolved to the same url.
I've also removed a couple of `CanGc` introduced in #37405, unless I've
missed something there is not code that can trigger a GC.
Testing: A subtest now pass
Part of #37553
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
`process_response_eof` is the only method that needs cx at least right
now. This PR removes one temp_cx and introduces one, removing that one
will is hard (needs VirtualMethods and a lot of work)
Testing: Just refactor
Part of #40600
---------
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
After debugging some of the failing tests, it appears spec text is
wrong.
Inside [merge existing and new import
maps](https://html.spec.whatwg.org/multipage/#merge-existing-and-new-import-maps)
algorithm, when checking which imports needs to be ignored,
specification will check if each _specifier_ of _newImportMap's imports_
will override any specifier of [resolved module
set](https://html.spec.whatwg.org/multipage/#resolved-module-set), by
checking if specifier starts with `record.specifier`.
Since each specifier gets normalized before they are inserted in the
resolved module set, the are cases where the check fails and we end up
overriding a module resolution.
This was an oversight, since for _newImportMap's scopes_ specification
performs the right check:
`specifierKey is a code unit prefix of record's specifier;`
Testing: More tests are now passing
Part of #37553
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
`ImportMap` _integrity_ entry was practically unused, since it is only
needed for [resolving a module integrity
metadata](https://html.spec.whatwg.org/multipage/#resolving-a-module-integrity-metadata).
Now, we correctly initialize `ScriptFetchOptions` when loading a
module's descendants.
I slightly modified `nonimport-integrity.html` test to run
`modulepreload` test cases at the end. Since we haven't an
implementation for it, the test timeout, making the test cases that
comes after them not run.
Testing: More tests start passing
Part of #37553
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Continuation of https://github.com/servo/servo/pull/42135, switch
Error::Type and Error::Range to also use CStrings internally, as they
are converted to CString for throwing JS exceptions (other get thrown as
DomException object, which uses rust string internally).
Changes in script crate are mechanical.
Testing: Should be covered by WPT tests.
Part of #42126
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
This is companion to https://github.com/servo/mozjs/pull/703 which makes
mozjs to use CStr(ing) in the API (where we would silently do conversion
in mozjs). This way we can avoid rust string -> c string allocations.
In the followup PR we should switch Error::Type and Error::Range to also
use CStrings internally, as they are converted to CString for throwing
JS exceptions (other get thrown as DomException object, which uses rust
string internally - although this gets converted to JSString somewhere).
Testing: It should be just refactor without any side effects so there
should be no changes to WPT results.
Try run: https://github.com/sagudev/servo/actions/runs/21328878448
Part of #42126
---------
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
Start using `&mut JSContext` and wrappers2 functions for module script
code.
Testing: A successful build should be enough
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
With import attributes enabled we can now support non javascript
modules, for now we are limited to json ones.
Switch `GlobalScope` `modulemap` to be keyed by the tuple url and
module's type.
Testing: Enabled a new directory, new tests should pass
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
Since we use `NetworkError` just for logging reasons, we don't really
need to pass it around; instead lets follow spec more closely and pass a
`None` on network failures.
Make more explicit if a `modulemap` entry is currently fetching or
ready.
Testing: No functional change, covered by existing tests
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Changed the return type of `ModuleObject::handle()` to the original type
of its contained handle by removing `.into()` call, as requested in the
linked issue.
Additional changes are due to temporary borrows now sharing lifetime
with returned handle reference.
Testing: No new tests needed, as the code is functionally the same after
the changes. Verified with running `./mach test-build` and `./mach
test-unit` successfully.
Fixes: https://github.com/servo/servo/issues/42051
Signed-off-by: Krzysztof Biedroń <arkendil@gmail.com>
This better reflects the intention of the code, which is that these
fields are set at most one time.
Testing: No behaviour change expected, so existing tests suffice.
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
This commit remove more unused code from `script_module`,
`inline_module_map` and `dynamic_modules` fields from `GlobalScope` and
the now unused custom interface `DynamicModuleOwner`.
Testing: No functional change, a successful build is enough.
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
This change is reviewable per commits:
In first commit we added `&mut JSContext` to `run_box` (it is very hard
to bring `&mut JSContext` to `remove_script_and_layout_blocker`).
In second commit we pass `&mut JSContext` to `run_once`.
In third commit we added support for accepting `&mut JSContext` in
closures of `task!` macro and lastly we demo new macro invocations (to
ensure they actually compile)
Testing: Just refactor, but should be covered by WPT
Part of #40600
---------
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
Rather than having each callside specifying the relevant
information from the GlobalScope, do this via a trait instead.
This would have saved us quite a bit of test debugging
since we would often forget to set relevant information
from the global context for a request.
Now, in the future when we need additional information from
the globalscope for a request, we only need to update this
method to make that happen.
Previously it would also sometimes use `document`, but
calling the relevant information on either `document` or
`globalscope` doesn't matter, since the `globalscope`
defers to the value from the `document` anyways.
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
This implements waiting for pending preloads, where the preload request
is still fetching the result when the second "real" request is started.
It is
implemented by storing responses in the `SharedPreloadedResources`
which is communicated via `PreloadId` send to the `CoreResourceManager`.
Part of #35035
---------
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
Replace NetworkError::Internal with structured enum variants
- Adds
UnsupportedScheme,CorsViolation,ConnectionFailure,Timeout,RedirectError,InvalidMethod,ResourceError,SecurityBlock,MixedContent,CacheError,InvalidPort,
LocalDirectoryError, variants in NetworkError enum.
- Refactored the usage of NetworkError::Internal(String) to use the
appropriate new variant
Testing: Changes does not require test.
Fixes: https://github.com/servo/servo/issues/36434
---------
Signed-off-by: Uthman Yahaya Baba <uthmanyahayababa@gmail.com>
Signed-off-by: Usman Yahaya Baba <91813795+uthmaniv@users.noreply.github.com>
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Co-authored-by: Tim van der Lippe <tvanderlippe@gmail.com>
Inside `notify_owner_to_finish` we already retrieve the `ModuleTree`
tied to the corresponding `ModuleIdentity`, but we ended up retrieving
it a second time inside of `run_a_module_script` instead of simply
passing it.
Testing: Covered by existing tests, a simple cleanup.
---------
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
