Closed Bug 1408673 Opened 2 years ago Closed 2 years ago

Cleanup EnvironmentObject comments

Categories

(Core :: JavaScript Engine, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: tcampbell, Assigned: tcampbell)

Details

Attachments

(1 file)

Now that shared-global JSM support (Bug 1186409) has stuck its landing, we should update the comments in EnvironmentObject.h that describe embedding use cases.

In particular, component loading is different, and subscript loading under a shared-global JSM needs to be documented.
Assignee: nobody → tcampbell
Status: NEW → ASSIGNED
Priority: -- → P3
Comment on attachment 8957645 [details]
Bug 1408673 - Update EnvironmentObject comments for Gecko

https://reviewboard.mozilla.org/r/226548/#review232432

::: js/src/vm/EnvironmentObject.h:182
(Diff revision 1)
>   * bottom, outermost to innermost.
>   *
>   * A. Component loading
>   *
> - * Components may be loaded in "reuse loader global" mode, where to save on
> - * memory, all JSMs and JS-implemented XPCOM modules are loaded into a single
> + * Components may be loaded in a shared global mode where most JSMs share a
> + * single global in order to save on memory and avoid CCWs. To support this a

Uber-nit: Comma after "this"

::: js/src/vm/EnvironmentObject.h:209
(Diff revision 1)
>   *       |
> - *   LexicalEnvironmentObject
> + *   LexicalEnvironmentObject[this=target]
> + *
> + * B.2 Subscript loading (Shared-global JSM)
>   *
> - * C. Frame scripts
> + * The target object of a subscript load may be in a JSM in which case we will

"A JSM with a shared global,"

::: js/src/vm/EnvironmentObject.h:236
(Diff revision 1)
>   *       |
> - *   Global lexical scope
> + *   LexicalEnvironmentObject[this=global]
>   *       |
>   *   NonSyntacticVariablesObject
>   *       |
> - *   LexicalEnvironmentObject
> + *   LexicalEnvironmentObject[this=nsvo]

Is this true? I thought we decided to preserve the old this=global behavior for frame script NSVOs
Attachment #8957645 - Flags: review?(kmaglione+bmo) → review+
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #2)

> Is this true? I thought we decided to preserve the old this=global behavior
> for frame script NSVOs

Bah! Good catch...
Pushed by tcampbell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/45b3adc56a53
Update EnvironmentObject comments for Gecko r=kmag
https://hg.mozilla.org/mozilla-central/rev/45b3adc56a53
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.