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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 11 commits into from
Closed

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

Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
622597a
Add support for posting results using cross-origin communication.
tobie Oct 6, 2012
7c53cb8
Fix error console pollution in WebKit when accessing windows on diffe…
tobie Oct 7, 2012
3820e47
Style.
tobie Oct 7, 2012
b6e9deb
Memoize test array for structured cloning.
tobie Oct 7, 2012
1e6ffd8
Grammar.
tobie Oct 7, 2012
bcef802
Factor out statuses.
tobie Oct 8, 2012
cb65ce7
Add support for window.opener. Refactor accordingly.
tobie Oct 8, 2012
fd9730c
Fix comment.
tobie Oct 8, 2012
0196461
Prevent an error being thrown when attempting cross-origin communicat…
tobie Oct 8, 2012
9a22575
Turn supports_post_message(w) method into a helper function.
tobie Oct 8, 2012
ce182ed
Missing semi-colon.
tobie Oct 9, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 132 additions & 25 deletions testharness.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,22 @@ policies and contribution forms [3].
* These are given the same arguments as the corresponding internal callbacks
* described above.
*
* == 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
Copy link
Owner

Choose a reason for hiding this comment

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

Nit: this should be ancestor and opener

* 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 }
*
* == List of assertions ==
*
* assert_true(actual, description)
Expand Down Expand Up @@ -895,6 +911,24 @@ policies and contribution forms [3].
NOTRUN:3
};

Test.prototype.structured_clone = function()
{
if(!this._structured_clone)
{
var msg = this.message;
msg = msg ? String(msg) : msg;
this._structured_clone = {
PASS:0,
Copy link
Owner

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

WFM. What about as a prop of the constructor?

Test.statuses = {...};

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, that sounds OK.

FAIL:1,
TIMEOUT:2,
NOTRUN:3,
name:String(this.name),
status:this.status,
message:msg
};
}
return this._structured_clone;
};

Test.prototype.step = function(func, this_obj)
{
Expand Down Expand Up @@ -1008,6 +1042,23 @@ policies and contribution forms [3].
TIMEOUT:2
};

TestsStatus.prototype.structured_clone = function()
{
if(!this._structured_clone)
{
var msg = this.message;
msg = msg ? String(msg) : msg;
this._structured_clone = {
OK:0,
ERROR:1,
TIMEOUT:2,
status:this.status,
message:msg
};
}
return this._structured_clone;
};

function Tests()
{
this.tests = [];
Expand Down Expand Up @@ -1149,10 +1200,10 @@ policies and contribution forms [3].
{
callback(this_obj.properties);
});
forEach(ancestor_windows(),
function(w)
forEach_windows(
function(w, is_same_origin)
{
if(w.start_callback)
if(is_same_origin && w.start_callback)
{
try
{
Expand All @@ -1166,6 +1217,7 @@ policies and contribution forms [3].
}
}
}
this_obj.post_message(w, "start", { properties: this_obj.properties });
});
};

Expand All @@ -1189,10 +1241,10 @@ policies and contribution forms [3].
callback(test, this_obj);
});

forEach(ancestor_windows(),
function(w)
forEach_windows(
function(w, is_same_origin)
{
if(w.result_callback)
if(is_same_origin && w.result_callback)
{
try
{
Expand All @@ -1205,6 +1257,7 @@ policies and contribution forms [3].
}
}
}
this_obj.post_message(w, "result", { test: test.structured_clone() });
});
this.processing_callbacks = false;
if (this_obj.all_done())
Expand All @@ -1225,6 +1278,11 @@ policies and contribution forms [3].
{
clearTimeout(this.timeout_id);
var this_obj = this;
var tests = map(this_obj.tests,
function(test)
{
return test.structured_clone();
});
if (this.status.status === null)
{
this.status.status = this.status.OK;
Expand All @@ -1236,10 +1294,10 @@ policies and contribution forms [3].
callback(this_obj.tests, this_obj.status);
});

forEach(ancestor_windows(),
function(w)
forEach_windows(
function(w, is_same_origin)
{
if(w.completion_callback)
if(is_same_origin && w.completion_callback)
{
try
{
Expand All @@ -1253,9 +1311,22 @@ policies and contribution forms [3].
}
}
}
this_obj.post_message(w, "complete", {
tests: tests,
status: this_obj.status.structured_clone()
});
});
};

Tests.prototype.post_message = function(w, type, data)
{
if (typeof w.postMessage == "function")
Copy link
Owner

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Owner

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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).

{
data.type = type;
w.postMessage(data, '*');
}
};

var tests = new Tests();

function timeout() {
Expand Down Expand Up @@ -1802,22 +1873,58 @@ policies and contribution forms [3].
target[components[components.length - 1]] = object;
}

function ancestor_windows() {
//Get the windows [self ... top] as an array
if ("result_cache" in ancestor_windows)
{
return ancestor_windows.result_cache;
}
var rv = [self];
var w = self;
while (w != w.parent)
{
w = w.parent;
rv.push(w);
}
ancestor_windows.result_cache = rv;
return rv;
}
function forEach_windows(callback) {
// Iterate of the the windows [self ... top]. The callback is passed
Copy link
Owner

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Owner

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

// two objects, the first one is the windows object itself, the second one
// is a boolean indicating whether or not its on the same origin as the
// current window.
var cache = forEach_windows.result_cache;
if (!cache) {
cache = [[self, true]];
Copy link
Owner

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

var w = self;
var i = 0;
var is_same_origin;
var origins = location.ancestorOrigins;
while (w != w.parent)
{
w = w.parent;
// In WebKit, calls to parent windows' properties that aren't on the same
// origin cause an error message to be displayed in the error console but
// don't throw an exception. This is a deviation from the current HTML5
// spec. See: https://bugs.webkit.org/show_bug.cgi?id=43504
// The problem with WebKit's behavior is that it pollutes the error console
// with error messages that can't be caught.
//
// This issue can be mitigated by relying on the (for now) proprietary
// `location.ancestorOrigins` property which returns an ordered list of
// the origins of enclosing windows. See:
// http://trac.webkit.org/changeset/113945.
if(origins) {
is_same_origin = (location.origin == origins[i]);
}
// For browsers that don't support this behaviour, we just do a try..catch
// on a ramdom prop and see if it throws.
else
{
is_same_origin = true
try {
'random_prop' in w;
} catch(e) {
is_same_origin = false;
}
}
cache.push([w, is_same_origin]);
i++;
}
forEach_windows.result_cache = cache;
}

forEach(cache,
function(a)
{
callback.apply(null, a);
});
}

})();
// vim: set expandtab shiftwidth=4 tabstop=4: