Add support for posting results using cross-origin communication. #3

Closed
wants to merge 11 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

tobie commented Oct 6, 2012

Where supported, the test harness will now also send messages using
cross-document messaging to each ancestor browsing context. Since it
uses the wildcard keyword (*), cross-origin communication is enabled and
script on different origins can collect the results.

This API follows similar conventions as those described above only slightly
modified to accommodate message event API. Each message is sent by the harness
is passed a single vanilla object, available as the data property of the
event object. These objects are structures as follows:

start - { type: "start" }
result - { type: "result", test: Test }
complete - { type: "complete", tests: [Test, ...], status: TestsStatus }

Add support for posting results using cross-origin communication.
Where supported, the test harness will now also send messages using
cross-document messaging to each ancestor browsing context. Since it
uses the wildcard keyword (*), cross-origin communication is enabled and
script on different origins can collect the results.

This API follows similar conventions as those described above only slightly
modified to accommodate message event API. Each message is sent by the harness
is passed a single vanilla object, available as the `data` property of the
event object. These objects are structures as follows:

start - { type: "start" }
result - { type: "result", test: Test }
complete - { type: "complete", tests: [Test, ...], status: TestsStatus }
Contributor

tobie commented Oct 6, 2012

/cc @darobin

Contributor

tobie commented Oct 7, 2012

Solved the pending WebKit issue (pollution of the console by error messages). This is ready to go. Happy to answer questions you might have, @jgraham, and/or revise commits.

testharness.js
+ var msg = this.message;
+ msg = msg ? String(msg) : msg;
+ this._structured_clone = {
+ PASS:0,
@jgraham

jgraham Oct 8, 2012

Owner

I wonder if we could move these constants out into a seperate object so they are only defined in one place and write a merge function to copy the properties over, so that this code would look like

this._structured_clone = merge({name:String(this.name), status:this.status, message:message}, test_statuses)

and something similar when they are put on the Test prototype object.

@tobie

tobie Oct 8, 2012

Contributor

WFM. What about as a prop of the constructor?

Test.statuses = {...};
@jgraham

jgraham Oct 8, 2012

Owner

Yeah, that sounds OK.

testharness.js
});
};
+ Tests.prototype.post_message = function(w, type, data)
+ {
+ if (typeof w.postMessage == "function")
@jgraham

jgraham Oct 8, 2012

Owner

nit: Should use === equality

What are we guarding against with this check? If an implementation doesn't support postMessage it's not clear that silent failure is the correct option; we should at least have some mechanism to inform the user that things are going wrong. Or is there some other case where the check can fail?

@tobie

tobie Oct 8, 2012

Contributor

Seems the enclosing window should be responsible for verifying the UA is able to handle postMessages. So this guard is just there to avoid triggering errors when reporting the results to windows on the same origin using the current system.

@jgraham

jgraham Oct 8, 2012

Owner

Sorry, I'm being slow. Can you give me an example of a situation where this check will fail, assuming postMessage is supported?

@tobie

tobie Oct 8, 2012

Contributor

No, I'm the one that's slow here. There should be a try..catch, to avoid triggering an error when touching the postMessage property in a UA that throws for SOP violations and doesn't implement postMessage yet. Would that make more sense? (This is of course provided we want to offer postMessage support both for same origin and different origin, which I think we should).

testharness.js
- return rv;
- }
+ function forEach_windows(callback) {
+ // Iterate of the the windows [self ... top]. The callback is passed
@jgraham

jgraham Oct 8, 2012

Owner

So, I think that for your usecase, stopping at top isn't good enough

It is quite possible to write a test that has to be run in a top level browsing context. Although it is possible to place the burden on authors and say that if they do that they have to open a tlbc themselves (and indeed for some tests that is a rather sensible thing to do e.g. where precise control over the history is needed), it is a lot of overhead for many tests. So a test runner should always window.open a new browsing context for the tests to run in, rather than using an iframe or similar. Hence if you want to report back to the harness you also need to ascend into opener windows.

@tobie

tobie Oct 8, 2012

Contributor

Unless I'm missing something, this wasn't supported before either. Think its a valuable addition, but not particularly tied to this feature either.

@jgraham

jgraham Oct 8, 2012

Owner

No, it wasn't, but I remember writing the code once. I wonder why it didn't make it to the repository.

In any case it is tied to this feature in-as-much as the use case that requires postMessage also requires this addition.

@tobie

tobie Oct 8, 2012

Contributor

postMessages aren't supported cross-window in IE anyway. Yeah… :(

+ // current window.
+ var cache = forEach_windows.result_cache;
+ if (!cache) {
+ cache = [[self, true]];
@jgraham

jgraham Oct 8, 2012

Owner

So this is all kind of ugly, but I'm not sure I have a better suggestion…

@tobie

tobie Oct 8, 2012

Contributor

Agreed. Spent some time trying to find a decent solution. That's the best I could come up with given the constraints (and the WebKit ugliness). Open to suggestions.

Contributor

tobie commented Oct 15, 2012

Hi James,

Is there anything that still needs work on my side before this makes it in?

If so, please LMK.

Thanks,

--tobie

+ * == External API through cross-document messaging ==
+ *
+ * Where supported, the test harness will also send messages using
+ * cross-document messaging to each ancestor browsing context. Since it
@jgraham

jgraham Oct 15, 2012

Owner

Nit: this should be ancestor and opener

Owner

jgraham commented Oct 15, 2012

This is OK now, I think; I will assume that you fix the docs without needing another review cycle. Are you OK to push it to mercurial?

@jgraham jgraham closed this Oct 15, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment