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

Update rust-mozjs #22353

Merged
merged 2 commits into from Jan 17, 2019
Merged

Update rust-mozjs #22353

merged 2 commits into from Jan 17, 2019

Conversation

@jdm
Copy link
Member

jdm commented Dec 2, 2018 •
edited by SimonSapin

These changes adjust our uses of the rust-mozjs APIs to accommodate the changes in servo/rust-mozjs#450.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #22342.
  • There are tests for these changes

This change is Reviewable
@highfive
Copy link

highfive commented Dec 2, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/globalscope.rs, components/script/dom/workerglobalscope.rs, components/script/Cargo.toml, components/script/dom/dedicatedworkerglobalscope.rs, components/script/dom/worklet.rs and 3 more
  • @KiChjang: components/script/dom/globalscope.rs, components/script/dom/workerglobalscope.rs, components/script/Cargo.toml, components/script/dom/dedicatedworkerglobalscope.rs, components/script/dom/worklet.rs and 3 more
@highfive
Copy link

highfive commented Dec 2, 2018

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!
@jdm
Copy link
Member Author

jdm commented Dec 2, 2018

@bors-servo try=wpt
@bors-servo
Copy link
Contributor

bors-servo commented Dec 2, 2018

⌛ Trying commit 9e12537 with merge 5fe9b42...
bors-servo added a commit that referenced this pull request Dec 2, 2018
@bors-servo
Auto merge of #22353 - jdm:runtime-parent, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
5fe9b42 
Runtime parent

These changes adjust our uses of the rust-mozjs APIs to accommodate the changes in servo/rust-mozjs#450.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22342.
- [x] There are tests for these changes
@jdm
Copy link
Member Author

jdm commented Dec 2, 2018

@paulrouget We'll need to figure out if this new model requires any changes to accommodate the FxR integration.
@bors-servo
Copy link
Contributor

bors-servo commented Dec 2, 2018

☀️ Test successful - linux-rel-css, linux-rel-wpt
State: approved= try=True
@jdm jdm force-pushed the jdm:runtime-parent branch 3 times, most recently from 0e3a880 to 79a3774 Dec 2, 2018
@CYBAI
CYBAI reviewed Dec 3, 2018
View changes
components/script/dom/globalscope.rs Show resolved Hide resolved
@jdm jdm changed the title Runtime parent Update rust-mozjs Dec 3, 2018
@paulrouget
Copy link
Contributor

paulrouget commented Dec 4, 2018 •
edited by CYBAI

Side note. There was a force push and the Cargo.lock turned to be wrong. This is necessary:

index 9ea9d39f71..6432091dc5 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2512,7 +2512,7 @@ dependencies = [
 [[package]]
 name = "mozjs"
 version = "0.10.0"
-source = "git+https://github.com/jdm/rust-mozjs?branch=parent-refactor#ac7f350c0a16217bd4e6a558a747ad5669a2f203"
+source = "git+https://github.com/jdm/rust-mozjs?branch=parent-refactor#69cfd4ac9b3f313214e635cb292812813a694ba4"
 dependencies = [
  "cc 1.0.18 (registry+https://github.com/rust-lang/crates.io-index)",
  "integer-atomics 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
@paulrouget
Copy link
Contributor

paulrouget commented Dec 4, 2018 

error: declaration of an `unsafe` function
   --> components/script/script_runtime.rs:331:1
    |
331 | / pub unsafe fn new_child_runtime(parent: ParentRuntime) -> Runtime {
332 | |     new_rt_and_cx_with_parent(Some(parent))
333 | | }
    | |_^
    |
note: lint level defined here
   --> components/script/lib.rs:11:9
    |
11  | #![deny(unsafe_code)]
    |         ^^^^^^^^^^^

error: usage of an `unsafe` block
   --> components/script/script_runtime.rs:336:5
    |
336 |     unsafe { new_rt_and_cx_with_parent(None) }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

error: Could not compile `script`.
@paulrouget
Copy link
Contributor

paulrouget commented Dec 5, 2018

@jdm this appears to work well in FxR.
@bors-servo
Copy link
Contributor

bors-servo commented Dec 29, 2018

☔ The latest upstream changes (presumably #22574) made this pull request unmergeable. Please resolve the merge conflicts.
@jdm jdm force-pushed the jdm:runtime-parent branch 4 times, most recently from 89344e7 to e3a1190 Jan 14, 2019
@jdm
Copy link
Member Author

jdm commented Jan 15, 2019

r? @nox
@highfive highfive assigned nox and unassigned avadacatavra Jan 15, 2019
@jdm jdm force-pushed the jdm:runtime-parent branch from e3a1190 to ce56f79 Jan 15, 2019
@jdm jdm force-pushed the jdm:runtime-parent branch 2 times, most recently from 0940e9f to ac606fc Jan 15, 2019
@servo-wpt-sync servo-wpt-sync mentioned this pull request Jan 15, 2019
Closed
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Jan 15, 2019

Opened new PR for upstreamable changes.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#14868.
jdm added 2 commits Dec 2, 2018
@jdm
Update to new JS runtime creation APIs.
644101e
@jdm
Reintroduce parent runtimes for worker threads.
Loading status checks…
367014a
@jdm jdm force-pushed the jdm:runtime-parent branch from ac606fc to 367014a Jan 15, 2019
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Jan 15, 2019

No upstreamable changes; closed existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#14868.
@jdm jdm mentioned this pull request Jan 17, 2019
Closed
@nox
Copy link
Member

nox commented Jan 17, 2019

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Jan 17, 2019

📌 Commit 367014a has been approved by nox
@bors-servo
Copy link
Contributor

bors-servo commented Jan 17, 2019

⌛ Testing commit 367014a with merge fb95f9d...
bors-servo added a commit that referenced this pull request Jan 17, 2019
@bors-servo
Auto merge of #22353 - jdm:runtime-parent, r=nox
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
fb95f9d 
Update rust-mozjs

These changes adjust our uses of the rust-mozjs APIs to accommodate the changes in servo/rust-mozjs#450.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22342.
- [x] There are tests for these changes

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

bors-servo commented Jan 17, 2019

☀️ Test successful - android-mac, arm32, arm64, linux-rel-css, linux-rel-wpt, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, magicleap, status-taskcluster
Approved by: nox
Pushing fb95f9d to master...
@bors-servo bors-servo merged commit 367014a into servo:master Jan 17, 2019
4 checks passed
4 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CYBAI CYBAI

Assignees

@nox nox

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

GC crash while reloading a page
8 participants
@jdm @highfive @bors-servo @paulrouget @servo-wpt-sync @nox @CYBAI @avadacatavra
You can’t perform that action at this time.