Re: [heycam/webidl] Modernize invoking user code (#113)

OK, so here's the plan. Let me know what you think, and I can try to implement it tomorrow.

### Step 1: Fixing this PR

We note that every object (not just platform object, but every object) has an associated Realm. For functions it coincides with the value returned by GetFunctionRealm(); for other objects it's not yet specified.

The four "call user code" operations here all use the corresponding object's Realm to set up the entry stuff. (I.e., get the object's realm's settings object's realm execution context, increment its entrance counter, and push it onto JS execution context stack.)

For the case of "call a user object's operation", which is more complicated since it can potentially invoke user code twice, we potentially set up the entry stuff twice: once for the Get(), using the object, and another time for the Call(), using the function. So if you have an object whose realm is A with a handleEvent property that is a function whose realm is B, invoking the getter will happen with a different entry global than invoking the handleEvent.

(We could change that to use the object in both cases, but that seems weird to me. I'm flexible though.)

This should set up the correct entry settings object per your earlier PR comments.

### Step 2: Fix HTML's "report the exception"

"Report the exception" gets modified to take an exception object, a global object, a filename/line number/column number tuple, and an optional script.

The optional script is necessary to perform muted error checks. (See ["report an error"](https://html.spec.whatwg.org/multipage/webappapis.html#report-the-error).) If it's not present then we assume the error is un-muted. We no longer derive the filename from the script, to allow the better results you mention.

We probably consolidate "report the exception" and "report an error".

### Step 3: Add "guarded" user code invocation to Web IDL

(This would be a follow-up PR performed after step 2, not as part of this step 1 PR.)

We add a new optional flag to all of these four user-code-calling operations: "guarded", default unset. The flag is not allowed to be set if the attribute or operation in question returns a promise type.

If the guarded flag is set, then when an exception is thrown during any of:
- converting arguments
- Get()/Set()/Call()
- converting return values

Then we do "report the exception" on the thrown exception, passing in the exception, the entry global (i.e. the global we went to all this trouble to make the entry global), "some relevant filename/line number/column number" (purposefully underspecified), and the callback's script-or-module.

Note that this means that in our `var f = new window2.Function("stuff")` case, we use the script of this `var` declaration for the muted errors check. This seems correct.

(I'm not sure if we should immediately call out to "report the exception" before un-entryifying the appropriate global, or if we should just save the appropriate global, entryify it, run possibly-throwing steps, unentryify it, and then call out to "report the exception" with the appropriate global. Maybe they are equivalent. I will assume we should do it before un-entryifying since that seems to be the direction your earlier PR comments were hinting at.)

### Step 4: Fix all call sites of "report an exception"

Very few places should directly call "report an exception". Most should switch over to calling one of IDL's invoke-user-code operations with the new "guarded" flag set.

Here are the call sites I know of to "report the exception":

#### HTML

- Custom element callback reactions should switch to using the guarded flag.
- Custom element upgrade reactions will need a bit of care; will have to think on it a bit more.
- Failed module resolution should report to the script's settings object's global, which will be the same as the initiating global for the fetch (except for module workers, where it will be the inner worker global, not the outer global)
- creating a module script will report to the script's settings object's global
- running a classic/module script will report to the entry settings object set during running the script
- EnqueueJob will report to the entry settings object set during job execution
- Timer functions, when supplied a string: their CSP checks will "report the exception" if CSP blocks string compilation. This will need a bit of care to capture the active script at the time setTimeout() et al. are called so it can be used for muted-errors checks later. It already captures the current realm, which it can use for error reporting.
- Timer functions, when supplied a function: these currently call IDL "invoke" but seem to omit "report the exception." That seems like a straight bug. Regardless, it should switch to using the guarded flag.
- Animation frame callbacks should switch to using the guarded flag.
- Custom element errors during parsing will be reported to the document's Window. Might need to thread through the correct script here to perform "muted errors" checks.

#### DOM

- Calling handleEvent should switch to using the guarded flag.
- Calling mutation observer callbacks should switch to using the guarded flag.

#### Pretty much everywhere else

- Should switch to using the guarded flag.

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/pull/113#issuecomment-218640902

Received on Thursday, 12 May 2016 02:07:00 UTC