Skip to content
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

Implement source for postMessage events #22514

Merged
merged 7 commits into from Jan 8, 2019
Merged

Implement source for postMessage events #22514

merged 7 commits into from Jan 8, 2019

Conversation

@jdm
Copy link
Member

jdm commented Dec 20, 2018

Also make similar-origin iframes access their newly loaded document as soon as the new document is created. This should allow tests that check the event.source property to run correctly.


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

This change is Reviewable

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Dec 20, 2018

Error syncing changes upstream. Logs saved in error-snapshot-1545341457705.

@highfive
Copy link

highfive commented Dec 20, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/script_thread.rs, components/script/dom/worker.rs, components/script/dom/webidls/MessageEvent.webidl, components/script/dom/messageevent.rs, components/script/dom/eventsource.rs and 5 more
  • @cbrewster: components/constellation/constellation.rs
  • @jgraham: tests/wpt/include.ini
  • @paulrouget: components/constellation/constellation.rs
  • @KiChjang: components/script_traits/lib.rs, components/script_traits/script_msg.rs, components/script/script_thread.rs, components/script/dom/worker.rs, components/script/dom/webidls/MessageEvent.webidl and 6 more
@highfive
Copy link

highfive commented Dec 20, 2018

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@jdm
Copy link
Member Author

jdm commented Dec 20, 2018

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2018

Trying commit ccbfb92 with merge c5bbdae...

bors-servo added a commit that referenced this pull request Dec 20, 2018
Implement source for postMessage events

This should allow tests that check the event.source property to run correctly.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22499 and fix #12715.
- [x] There are tests for these changes
@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2018

💔 Test failed - status-taskcluster

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Dec 20, 2018

Opened new PR for upstreamable changes.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#14634.

@@ -424,7 +424,7 @@ impl DedicatedWorkerGlobalScope {
JSAutoCompartment::new(scope.get_cx(), scope.reflector().get_jsobject().get());
rooted!(in(scope.get_cx()) let mut message = UndefinedValue());
data.read(scope.upcast(), message.handle_mut());
MessageEvent::dispatch_jsval(target, scope.upcast(), message.handle(), None);
MessageEvent::dispatch_jsval(target, scope.upcast(), message.handle(), None, None);

This comment has been minimized.

@KiChjang

KiChjang Dec 20, 2018

Member

Some day we're gonna have to use the builder pattern for this :)

@jdm jdm force-pushed the jdm:source branch from d7198cd to 0797998 Dec 21, 2018
@jdm
Copy link
Member Author

jdm commented Dec 21, 2018

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Dec 21, 2018

Trying commit 0797998 with merge 3ebe83a...

bors-servo added a commit that referenced this pull request Dec 21, 2018
Implement source for postMessage events

Also make similar-origin iframes access their newly loaded document as soon as the new document is created. This should allow tests that check the event.source property to run correctly.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22499 and fix #12715 and fix #22514.
- [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/22514)
<!-- Reviewable:end -->
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Dec 21, 2018

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#14634.

@bors-servo
Copy link
Contributor

bors-servo commented Dec 21, 2018

💔 Test failed - status-taskcluster

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Dec 21, 2018

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#14634.

@jdm
Copy link
Member Author

jdm commented Dec 21, 2018

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Dec 21, 2018

Trying commit 07f38f5 with merge 870d694...

bors-servo added a commit that referenced this pull request Dec 21, 2018
Implement source for postMessage events

Also make similar-origin iframes access their newly loaded document as soon as the new document is created. This should allow tests that check the event.source property to run correctly.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22499 and fix #12715 and fix #22514.
- [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/22514)
<!-- Reviewable:end -->
@jdm
Copy link
Member Author

jdm commented Dec 21, 2018

I feel so much better about the referrer-policy tests now.

@jdm
Copy link
Member Author

jdm commented Dec 21, 2018

@CYBAI
Copy link
Collaborator

CYBAI commented Jan 8, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2019

Testing commit 9afa722 with merge c050229...

bors-servo added a commit that referenced this pull request Jan 8, 2019
Implement source for postMessage events

Also make similar-origin iframes access their newly loaded document as soon as the new document is created. This should allow tests that check the event.source property to run correctly.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22499 and fix #12715 and fix #22514.
- [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/22514)
<!-- Reviewable:end -->
@CYBAI
Copy link
Collaborator

CYBAI commented Jan 8, 2019

oops, I found Travis is complaining about tidy

Diff in /home/travis/build/servo/servo/components/script/script_thread.rs at line 2110:
                         return warn!(
                             "postMessage after source pipeline {} closed.",
                             source_pipeline_id,
-                        )
+                        );
                     },
                     Some(source) => source,
                 };
Run `./mach fmt` to fix the formatting
@jdm jdm force-pushed the jdm:source branch from 9afa722 to 8ff0c41 Jan 8, 2019
@jdm
Copy link
Member Author

jdm commented Jan 8, 2019

Thanks for noticing!
@bors-servo r=Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2019

📌 Commit 8ff0c41 has been approved by Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2019

Testing commit 8ff0c41 with merge 11d1184...

bors-servo added a commit that referenced this pull request Jan 8, 2019
Implement source for postMessage events

Also make similar-origin iframes access their newly loaded document as soon as the new document is created. This should allow tests that check the event.source property to run correctly.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22499 and fix #12715 and fix #22514.
- [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/22514)
<!-- Reviewable:end -->
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Jan 8, 2019

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#14634.

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2019

@bors-servo bors-servo merged commit 8ff0c41 into servo:master Jan 8, 2019
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
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Jan 8, 2019

Error syncing changes upstream. Logs saved in error-snapshot-1546923619003.

@CYBAI
Copy link
Collaborator

CYBAI commented Jan 8, 2019

Error syncing changes upstream. Logs saved in error-snapshot-1546923619003.

I found web-platform-tests/wpt#14634 is not merged but the labels are removed 🤔

@jdm
Copy link
Member Author

jdm commented Jan 8, 2019

Merged upstream manually in web-platform-tests/wpt#14634.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.