Bug 1412856 part 2. Stop using NS_ERROR_DOM_TYPE_ERR in Client::Navigate. r=dom-workers-and-storage-reviewers,perry?

Differential Revision: https://phabricator.services.mozilla.com/D61197

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Boris Zbarsky 2020-01-30 08:09:38 +00:00
parent a389959875
commit 3c46021c3c
6 changed files with 47 additions and 24 deletions

View File

@ -201,9 +201,8 @@ already_AddRefed<Promise> Client::Navigate(const nsAString& aURL,
outerPromise->MaybeResolve(newClient);
},
[self, outerPromise](const CopyableErrorResult& aResult) {
// TODO: Improve this error in bug 1412856. Ideally we should throw
// the TypeError in the child process and pass it back to here.
outerPromise->MaybeReject(NS_ERROR_DOM_TYPE_ERR);
CopyableErrorResult result(aResult);
outerPromise->MaybeReject(result);
});
return outerPromise.forget();

View File

@ -207,14 +207,17 @@ RefPtr<ClientOpPromise> ClientNavigateOpChild::DoNavigate(
rv = NS_NewURI(getter_AddRefs(url), aArgs.url(), nullptr,
shouldUseBaseURL ? baseURL.get() : nullptr);
if (NS_FAILED(rv)) {
// Per https://w3c.github.io/ServiceWorker/#dom-windowclient-navigate step
// 2, if the URL fails to parse, we reject with a TypeError.
nsPrintfCString err("Invalid URL \"%s\"", aArgs.url().get());
CopyableErrorResult result;
result.Throw(rv);
result.ThrowTypeError(NS_ConvertUTF8toUTF16(err));
return ClientOpPromise::CreateAndReject(result, __func__);
}
if (url->GetSpecOrDefault().EqualsLiteral("about:blank")) {
CopyableErrorResult result;
result.Throw(NS_ERROR_FAILURE);
result.ThrowTypeError(u"Client.navigate to \"about:blank\" is not allowed");
return ClientOpPromise::CreateAndReject(result, __func__);
}
@ -249,8 +252,14 @@ RefPtr<ClientOpPromise> ClientNavigateOpChild::DoNavigate(
loadState->SetFirstParty(true);
rv = docShell->LoadURI(loadState, false);
if (NS_FAILED(rv)) {
/// There are tests that try sending file:/// and mixed-content URLs
/// in here and expect them to reject with a TypeError. This does not match
/// the spec, but does match the current behavior of both us and Chrome.
/// https://github.com/w3c/ServiceWorker/issues/1500 tracks sorting that
/// out.
nsPrintfCString err("Invalid URL \"%s\"", aArgs.url().get());
CopyableErrorResult result;
result.Throw(rv);
result.ThrowTypeError(NS_ConvertUTF8toUTF16(err));
return ClientOpPromise::CreateAndReject(result, __func__);
}

View File

@ -1,13 +1,13 @@
[windowclient-navigate.https.html]
expected:
if (os == "linux") and not debug and not webrender and not fission and (processor == "x86_64"): ["OK", "TIMEOUT"]
[in scope but not controlled test on installing worker]
[in scope but not controlled test on installing worker worker side]
expected: FAIL
[in scope but not controlled test on active worker]
[in scope but not controlled test on active worker worker side]
expected: FAIL
[out of scope]
[out of scope worker side]
expected: FAIL
[invalid url (about:blank)]

View File

@ -53,7 +53,7 @@ self.onmessage = function(e) {
promise_test(function(t) {
this.add_cleanup(function() { port.postMessage(pass(test, "")); });
return self.clients.get(clientId)
.then(client => promise_rejects(t, new TypeError(), client.navigate("about:blank")))
.then(client => promise_rejects_js(t, TypeError, client.navigate("about:blank")))
.catch(unreached_rejection(t));
}, "Navigating to about:blank should reject with TypeError");
} else if (test === "test_client_navigate_mixed_content") {
@ -64,7 +64,7 @@ self.onmessage = function(e) {
// and navigating to http:// would create a mixed-content violation.
var url = get_host_info()['HTTP_REMOTE_ORIGIN'] + path;
return self.clients.get(clientId)
.then(client => promise_rejects(t, new TypeError(), client.navigate(url)))
.then(client => promise_rejects_js(t, TypeError, client.navigate(url)))
.catch(unreached_rejection(t));
}, "Navigating to mixed-content iframe should reject with TypeError");
} else if (test === "test_client_navigate_redirect") {

View File

@ -1,12 +1,15 @@
importScripts('/resources/testharness.js');
function match_query(query_string) {
return self.location.search.substr(1) == query_string;
}
function navigate_test(e) {
async function navigate_test(t, e) {
var port = e.data.port;
var url = e.data.url;
var expected = e.data.expected;
return clients.matchAll({ includeUncontrolled : true })
var p = clients.matchAll({ includeUncontrolled : true })
.then(function(client_list) {
for (var i = 0; i < client_list.length; i++) {
var client = client_list[i];
@ -14,17 +17,25 @@ function navigate_test(e) {
return client.navigate(url);
}
}
port.postMessage('Could not locate window client.');
throw 'Could not locate window client.';
})
.then(function(new_client) {
if (new_client === null)
port.postMessage(new_client);
else
port.postMessage(new_client.url);
})
.catch(function(error) {
port.postMessage(error.name);
// If we didn't reject, we better get resolved with the right thing.
if (new_client === null) {
assert_equals(new_client, expected);
} else {
assert_equals(new_client.url, expected);
}
});
if (typeof self[expected] == "function") {
// It's a JS error type name. We are expecting our promise to be rejected
// with that error.
p = promise_rejects_js(t, self[expected], p);
}
// Let our caller know we are done.
return p.finally(() => port.postMessage(null));
}
function getTestClient() {
@ -62,5 +73,6 @@ if (match_query('installing')) {
}
self.addEventListener('message', function(e) {
e.waitUntil(navigate_test(e));
e.waitUntil(promise_test(t => navigate_test(t, e),
e.data.description + " worker side"));
});

View File

@ -154,12 +154,15 @@ function navigate_test(override_parameters) {
channel.port1.onmessage = test.step_func(resolve);
service_worker.postMessage({
port: channel.port2,
url: parameters.dest_url
url: parameters.dest_url,
expected: parameters.expected,
description: parameters.description,
}, [channel.port2]);
});
})
.then(function(response) {
assert_equals(response.data, parameters.expected);
assert_equals(response.data, null);
return fetch_tests_from_worker(service_worker);
});
}, parameters.description);
}