Closed
Bug 1466121
Opened 6 years ago
Closed 6 years ago
Rename JSCompartment to JS::Compartment, split vm/JSCompartment.h/cpp
Categories
(Core :: JavaScript Engine, enhancement, P3)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(7 files)
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
At some point we should rename JSCompartment to JS::Compartment (then we'll have JS::Realm, JS::Compartment, JS::Zone). We should also split vm/JSCompartment.* in vm/Realm.* and vm/Compartment.*
Assignee | ||
Comment 1•6 years ago
|
||
Mostly very mechanical, except I fixed up some formatting and changed some comments to say realm instead of compartment.
Assignee | ||
Comment 2•6 years ago
|
||
For consistency with JS::Realm, make JS::Compartment and JS::Zone classes instead of structs.
Attachment #8984070 -
Flags: review?(luke)
Assignee | ||
Comment 3•6 years ago
|
||
I think the next step (after these patches land) is to: * merge vm/Realm.cpp into vm/JSCompartment.cpp * hg mv vm/JSCompartment.cpp/h => vm/Compartment.cpp/h * hg cp vm/Compartment.cpp/h => vm/Realm.cpp/h and remove the Compartment code from Realm.cpp/h and remove the Realm code from Compartment.cpp/h
Updated•6 years ago
|
Attachment #8984065 -
Flags: review?(luke) → review+
Comment 4•6 years ago
|
||
Comment on attachment 8984070 [details] [diff] [review] Part 2 - Make Compartment and Zone classes instead of structs Review of attachment 8984070 [details] [diff] [review]: ----------------------------------------------------------------- Nice!
Attachment #8984070 -
Flags: review?(luke) → review+
Pushed by jandemooij@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b640dc9b8998 part 1 - Rename JSCompartment to JS::Compartment. r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/e3b458961e04 part 2 - Make Compartment and Zone classes instead of structs. r=luke
Assignee | ||
Updated•6 years ago
|
Keywords: leave-open
Comment 6•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b640dc9b8998 https://hg.mozilla.org/mozilla-central/rev/e3b458961e04
Assignee | ||
Comment 7•6 years ago
|
||
Taking one step back, so we can take two steps forward...
Attachment #8984376 -
Flags: review?(luke)
Assignee | ||
Comment 9•6 years ago
|
||
Just a |hg cp| and then I removed code from both files. The #includes are the hard part because it's not clear which ones we need where. I fixed the obvious ones.
Attachment #8984379 -
Flags: review?(luke)
Assignee | ||
Comment 12•6 years ago
|
||
I'll probably land each of these parts separately because renaming/adding files can have weird side effects due to unified builds...
Updated•6 years ago
|
Attachment #8984376 -
Flags: review?(luke) → review+
Updated•6 years ago
|
Attachment #8984377 -
Flags: review?(luke) → review+
Comment 13•6 years ago
|
||
Comment on attachment 8984379 [details] [diff] [review] Part 5 - Split Compartment.h from Realm.h Review of attachment 8984379 [details] [diff] [review]: ----------------------------------------------------------------- Nice job not losing the history.
Attachment #8984379 -
Flags: review?(luke) → review+
Updated•6 years ago
|
Attachment #8984380 -
Flags: review?(luke) → review+
Updated•6 years ago
|
Attachment #8984381 -
Flags: review?(luke) → review+
Comment 14•6 years ago
|
||
Pushed by jandemooij@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4db98fad2f4e part 3 - Merge Realm.cpp into JSCompartment.cpp to simplify later patches. r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/ac87103cdf38 part 4 - Rename vm/JSCompartment* to vm/Realm*. r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/af265d75fc00 part 5 - Split Compartment.h from Realm.h. r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/45e231683dbf part 6 - Split Compartment.cpp from Realm.cpp. r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/cceb75ca1a1d part 7 - Split Compartment-inl.h from Realm-inl.h. r=luke
Assignee | ||
Comment 15•6 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #12) > I'll probably land each of these parts separately because renaming/adding > files can have weird side effects due to unified builds... I decided not to do this because the stack of patches was green on Try and splitting them up now is probably a bigger risk. Also if there is some weird Talos regression, it's easy enough to bisect on Try because separate revisions.
Keywords: leave-open
Comment 16•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4db98fad2f4e https://hg.mozilla.org/mozilla-central/rev/ac87103cdf38 https://hg.mozilla.org/mozilla-central/rev/af265d75fc00 https://hg.mozilla.org/mozilla-central/rev/45e231683dbf https://hg.mozilla.org/mozilla-central/rev/cceb75ca1a1d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in
before you can comment on or make changes to this bug.
Description
•