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

Test that various MessageEvents are trusted #4025

Merged
merged 3 commits into from
Feb 8, 2017
Merged

Conversation

domenic
Copy link
Member

@domenic domenic commented Oct 19, 2016

@wpt-pr-bot
Copy link
Collaborator

Reviewers for this pull request are: @Ms2ger, @velmont, @Yaffle, @aogilvie, @deniak, @jdm, @plehegar, @sideshowbarker, and @zqzhang.

@domenic
Copy link
Member Author

domenic commented Oct 19, 2016

Note that I cannot get the BroadcastChannel tests to work in Chrome or Firefox---they timeout instead---so if someone can figure out my mistake that would be great.

@annevk
Copy link
Member

annevk commented Oct 20, 2016

You need to have at least two BroadcastChannel objects I think. Ideally in separate globals. (Maybe use an <iframe>?)

@sideshowbarker
Copy link
Contributor

I cannot get the BroadcastChannel tests to work in Chrome or Firefox---they timeout instead

You need to have at least two BroadcastChannel objects I think. Ideally in separate globals.

The following works (though not sure it’s the best way to do it…)

diff --git a/webmessaging/MessageEvent-trusted.html b/webmessaging/MessageEvent-trusted.html
index e3d26f8..502cd51 100644
--- a/webmessaging/MessageEvent-trusted.html
+++ b/webmessaging/MessageEvent-trusted.html
@@ -1,3 +1,8 @@
+<!--
+const channel = new BroadcastChannel("name");
+channel.postMessage("ping");
+/*
+-->
 <!DOCTYPE html>
 <title>MessagePort message events are trusted</title>
 <script src="/resources/testharness.js"></script>
@@ -26,13 +31,13 @@ async_test(t => {
 async_test(t => {
   assert_true("BroadcastChannel" in self, "The browser must support BroadcastChannel");

+  const worker = new Worker("#");
   const channel = new BroadcastChannel("name");

   channel.onmessage = t.step_func_done(e => {
     assert_equals(e.isTrusted, true);
   });

-  channel.postMessage("ping");
 }, "With a BroadcastChannel");

 async_test(t => {
@@ -43,3 +48,6 @@ async_test(t => {
   window.postMessage("ping", "*");
 }, "With window");
 </script>
+<!--
+*/
+-->

(btw would be nice if we could just use template literals in WPT tests but I’m told we shouldn’t yet)

@domenic
Copy link
Member Author

domenic commented Oct 20, 2016

Thanks all, fixed the test, and now it indeed passes in Chrome and Firefox.

@wpt-stability-bot
Copy link

Testing in Chrome
Files changed:

  • /home/travis/build/w3c/web-platform-tests/eventsource/eventsource-onmessage-trusted.htm
  • /home/travis/build/w3c/web-platform-tests/webmessaging/MessageEvent-trusted-worker.js
  • /home/travis/build/w3c/web-platform-tests/webmessaging/MessageEvent-trusted.html

All results were stable

All results

Test Subtest Results
/eventsource/eventsource-onmessage-trusted.htm OK: 10
EventSource message events are trusted PASS: 10
/webmessaging/MessageEvent-trusted.html OK: 10
With a MessageChannel and its MessagePorts PASS: 10
With a BroadcastChannel PASS: 10
With window PASS: 10

@wpt-stability-bot
Copy link

Testing in Firefox
Files changed:

  • /home/travis/build/w3c/web-platform-tests/eventsource/eventsource-onmessage-trusted.htm
  • /home/travis/build/w3c/web-platform-tests/webmessaging/MessageEvent-trusted-worker.js
  • /home/travis/build/w3c/web-platform-tests/webmessaging/MessageEvent-trusted.html

All results were stable

All results

Test Subtest Results
/eventsource/eventsource-onmessage-trusted.htm OK: 10
EventSource message events are trusted PASS: 10
/webmessaging/MessageEvent-trusted.html OK: 10
With a MessageChannel and its MessagePorts PASS: 10
With a BroadcastChannel PASS: 10
With window FAIL: 10

@cdumez
Copy link
Contributor

cdumez commented Oct 20, 2016

Wow, it now runs tests in browsers!? so cool!
I wish it would cover Safari (TP) too :D

@ayg ayg closed this Oct 26, 2016
@ayg ayg deleted the messageevents-trusted branch October 26, 2016 17:38
@ayg ayg restored the messageevents-trusted branch October 26, 2016 18:04
@ayg ayg reopened this Oct 26, 2016
@wpt-stability-bot
Copy link

Firefox

Testing revision ab50501
Starting 10 test iterations
All results were stable

All results

/eventsource/eventsource-onmessage-trusted.htm

Subtest Results
OK
EventSource message events are trusted PASS

/webmessaging/MessageEvent-trusted.html

Subtest Results
OK
With a MessageChannel and its MessagePorts PASS
With a BroadcastChannel PASS
With window FAIL

@wpt-stability-bot
Copy link

Chrome

Testing revision 4fd89ca
Starting 10 test iterations
All results were stable

All results

/eventsource/eventsource-onmessage-trusted.htm

Subtest Results
OK
EventSource message events are trusted PASS

/webmessaging/MessageEvent-trusted.html

Subtest Results
OK
With a MessageChannel and its MessagePorts PASS
With a BroadcastChannel PASS
With window PASS

async_test(t => {
assert_true("BroadcastChannel" in self, "The browser must support BroadcastChannel");

new Worker("MessageEvent-trusted-worker.js");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is probably safer to not create this worker until after you've created the BroadcastChannel instance and added the onmessage event handler. I don't think there is anything that guarantees the worker won't be created, ran, and posts its message to the channel before this method has a chance to connect to the channel itself (unless I'm missing something in how cross-thread-synchronization is specified in the html spec)

@domenic
Copy link
Member Author

domenic commented Dec 5, 2016

@mkruisselbrink fixed, thanks!

@wpt-stability-bot
Copy link

Firefox

Testing revision 814f27d
Starting 10 test iterations
All results were stable

All results

/eventsource/eventsource-onmessage-trusted.htm

Subtest Results
OK
EventSource message events are trusted PASS

/webmessaging/MessageEvent-trusted.html

Subtest Results
OK
With a MessageChannel and its MessagePorts PASS
With a BroadcastChannel PASS
With window FAIL

@wpt-stability-bot
Copy link

Chrome

Testing revision 814f27d
Starting 10 test iterations
All results were stable

All results

/eventsource/eventsource-onmessage-trusted.htm

Subtest Results
OK
EventSource message events are trusted PASS

/webmessaging/MessageEvent-trusted.html

Subtest Results
OK
With a MessageChannel and its MessagePorts PASS
With a BroadcastChannel PASS
With window PASS

@domenic
Copy link
Member Author

domenic commented Feb 8, 2017

Ping @mkruisselbrink for a final sign-off? This seems to have gotten left behind at some point.

@mkruisselbrink
Copy link
Contributor

Ping @mkruisselbrink for a final sign-off? This seems to have gotten left behind at some point.

Looks good to me.

@domenic domenic merged commit 0541620 into master Feb 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants