Skip to content

/ servo

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

Refactors some entities from script_thread into script_runtime #10342

Merged
merged 1 commit into from Apr 6, 2016
Merged

Refactors some entities from script_thread into script_runtime #10342

merged 1 commit into from Apr 6, 2016

Conversation

@creativcoder
Copy link
Contributor

creativcoder commented Apr 1, 2016

Fixes #10271.

This change is Reviewable
@highfive
Copy link

highfive commented Apr 1, 2016

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/bindings/global.rs, components/script/dom/workerglobalscope.rs, components/script/network_listener.rs, components/script/dom/htmlscriptelement.rs, components/script/dom/filereader.rs, components/script/dom/window.rs, components/script/dom/servohtmlparser.rs, components/script/cors.rs, components/script/dom/document.rs, components/script/dom/bindings/refcounted.rs, components/script/task_source/history_traversal.rs, components/script/dom/xmlhttprequest.rs, components/script/task_source/networking.rs, components/script/task_source/file_reading.rs, components/script/lib.rs, components/script/dom/htmldetailselement.rs, components/script/dom/dedicatedworkerglobalscope.rs, components/script/dom/htmlimageelement.rs, components/script/script_thread.rs, components/script/dom/htmlinputelement.rs, components/script/script_runtime.rs, components/script/dom/worker.rs, components/script/dom/storage.rs, components/script/dom/htmlformelement.rs, components/script/task_source/user_interaction.rs, components/script/dom/bindings/trace.rs, components/script/dom/websocket.rs, components/script/dom/closeevent.rs
@highfive
Copy link

highfive commented Apr 1, 2016

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify script code, but no tests are modified. Please consider adding a test!
@creativcoder creativcoder changed the title For #10271 Refactors some entities from script_thread into script_runtime Refactors some entities from script_thread into script_runtime Apr 1, 2016
@jdm
Copy link
Member

jdm commented Apr 1, 2016

These changes look great! Please also move the things in script_thread.rs that were made public, since they're only used by the other moved code.
@Ms2ger
Copy link
Contributor

Ms2ger commented Apr 1, 2016

Reviewed 28 of 28 files at r1.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

components/script/lib.rs, line 95 [r1] (raw file):
Let's move this to the places where it's needed instead.

components/script/script_thread.rs, line 117 [r1] (raw file):
Let's add a static function ScriptTask::trace_thread(tr: *mut JSTracer) and move the SCRIPT_THREAD_ROOT.with() call into it. Then move trace_rust_roots into script_runtime too.

components/script/script_thread.rs, line 313 [r1] (raw file):
Are there two of those now? Remove this one.

components/script/script_thread.rs, line 513 [r1] (raw file):
Move this too, along with the thread_locals

components/script/script_thread.rs, line 553 [r1] (raw file):
Ditto

Comments from the review on Reviewable.io
@Ms2ger Ms2ger assigned Ms2ger and unassigned nox Apr 1, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Apr 1, 2016

☔ The latest upstream changes (presumably #10312) made this pull request unmergeable. Please resolve the merge conflicts.
@creativcoder creativcoder force-pushed the creativcoder:script-runtime-module branch from 8eff11a to 3a38296 Apr 2, 2016
@creativcoder creativcoder force-pushed the creativcoder:script-runtime-module branch from 3a38296 to 793e22a Apr 2, 2016
@KiChjang KiChjang removed the S-needs-rebase label Apr 2, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Apr 4, 2016

☔ The latest upstream changes (presumably #10034) made this pull request unmergeable. Please resolve the merge conflicts.
@creativcoder creativcoder force-pushed the creativcoder:script-runtime-module branch from 793e22a to fc9d846 Apr 4, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Apr 5, 2016

☔ The latest upstream changes (presumably #10325) made this pull request unmergeable. Please resolve the merge conflicts.
@creativcoder creativcoder force-pushed the creativcoder:script-runtime-module branch from fc9d846 to 699db6c Apr 5, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Apr 5, 2016

☔ The latest upstream changes (presumably #9811) made this pull request unmergeable. Please resolve the merge conflicts.
@creativcoder creativcoder force-pushed the creativcoder:script-runtime-module branch from 699db6c to 17b9284 Apr 6, 2016
@Ms2ger
Copy link
Contributor

Ms2ger commented Apr 6, 2016

Reviewed 2 of 6 files at r2, 5 of 8 files at r3, 5 of 5 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

components/script/script_runtime.rs, line 10 [r4] (raw file):
Nit: no {}.

Comments from Reviewable
@creativcoder
refactors entities from script_thread into script_runtime
Loading status checks…
2caa9a2
@creativcoder creativcoder force-pushed the creativcoder:script-runtime-module branch from 17b9284 to 2caa9a2 Apr 6, 2016
@Ms2ger
Copy link
Contributor

Ms2ger commented Apr 6, 2016

@bors-servo r+

Thanks!

Reviewed 1 of 1 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable
@bors-servo
Copy link
Contributor

bors-servo commented Apr 6, 2016

📌 Commit 2caa9a2 has been approved by Ms2ger
@Ms2ger
Copy link
Contributor

Ms2ger commented Apr 6, 2016

@bors-servo p=2
@bors-servo
Copy link
Contributor

bors-servo commented Apr 6, 2016

⌛ Testing commit 2caa9a2 with merge 883cde4...
bors-servo added a commit that referenced this pull request Apr 6, 2016
@bors-servo
Auto merge of #10342 - creativcoder:script-runtime-module, r=Ms2ger
Loading status checks…
883cde4 
Refactors some entities from script_thread into script_runtime

Fixes #10271.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10342)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Apr 6, 2016

☀️ Test successful - android, arm32, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, status-appveyor
@bors-servo bors-servo merged commit 2caa9a2 into servo:master Apr 6, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
metajack added a commit to metajack/servo that referenced this pull request Apr 6, 2016
@metajack
Use better JS engine defaults
93cd0f2 
This adds in preferences for all the SM 39 available options (as
retrieved from Gecko), and uses the same defaults as Gecko. A few
properties are not supported yet, and incremental GC is still always
disabled regardless of the preference setting.

This also adds back in the options that were accidentally deleted when
\servo#10342 was rebased, which moved things from script_thread.rs to
script_runtime.rs.
metajack added a commit to metajack/servo that referenced this pull request Apr 8, 2016
@metajack
Use better JS engine defaults
ebea51c 
This adds in preferences for all the SM 39 available options (as
retrieved from Gecko), and uses the same defaults as Gecko. A few
properties are not supported yet, and incremental GC is still always
disabled regardless of the preference setting.

This also adds back in the options that were accidentally deleted when
\servo#10342 was rebased, which moved things from script_thread.rs to
script_runtime.rs.
metajack added a commit to metajack/servo that referenced this pull request Apr 8, 2016
@metajack
Use better JS engine defaults
1bec193 
This adds in preferences for all the SM 39 available options (as
retrieved from Gecko), and uses the same defaults as Gecko. A few
properties are not supported yet, and incremental GC is still always
disabled regardless of the preference setting.

This also adds back in the options that were accidentally deleted when
\servo#10342 was rebased, which moved things from script_thread.rs to
script_runtime.rs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Ms2ger Ms2ger

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
@creativcoder @highfive @jdm @Ms2ger @bors-servo @nox @KiChjang
You can’t perform that action at this time.