Commit 83e8d1a9 authored by Victor Costan's avatar Victor Costan Committed by Commit Bot

Async Cookies: Address feedback from spec repository.

This CL integrates the following issues:

https://github.com/WICG/cookie-store/issues/54 - startsWith -> starts-with
https://github.com/WICG/cookie-store/issues/51 - remove cookieStore.has

Bug: 729800
Change-Id: If296a9e1eb896cb0ef344a889b1715836610bc6d
Reviewed-on: https://chromium-review.googlesource.com/1112912
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: default avatarJoshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570157}
parent 98cf9db6
...@@ -84,14 +84,14 @@ promise_test(async testCase => { ...@@ -84,14 +84,14 @@ promise_test(async testCase => {
await cookieStore.set('cookie-name-2', 'cookie-value-2'); await cookieStore.set('cookie-name-2', 'cookie-value-2');
const cookies = await cookieStore.getAll( const cookies = await cookieStore.getAll(
'cookie-name-', { matchType: 'startsWith' }); 'cookie-name-', { matchType: 'starts-with' });
assert_equals(cookies.length, 1); assert_equals(cookies.length, 1);
assert_equals(cookies[0].name, 'cookie-name-2'); assert_equals(cookies[0].name, 'cookie-name-2');
assert_equals(cookies[0].value, 'cookie-value-2'); assert_equals(cookies[0].value, 'cookie-value-2');
await async_cleanup(() => cookieStore.delete('cookie-name')); await async_cleanup(() => cookieStore.delete('cookie-name'));
await async_cleanup(() => cookieStore.delete('cookie-name-2')); await async_cleanup(() => cookieStore.delete('cookie-name-2'));
}, 'cookieStore.getAll with matchType set to startsWith'); }, 'cookieStore.getAll with matchType set to starts-with');
promise_test(async testCase => { promise_test(async testCase => {
await cookieStore.set('cookie-name', 'cookie-value'); await cookieStore.set('cookie-name', 'cookie-value');
...@@ -109,11 +109,11 @@ promise_test(async testCase => { ...@@ -109,11 +109,11 @@ promise_test(async testCase => {
await cookieStore.set('cookie-name-2', 'cookie-value-2'); await cookieStore.set('cookie-name-2', 'cookie-value-2');
const cookies = await cookieStore.getAll( const cookies = await cookieStore.getAll(
{ matchType: 'startsWith', name: 'cookie-name-' }); { matchType: 'starts-with', name: 'cookie-name-' });
assert_equals(cookies.length, 1); assert_equals(cookies.length, 1);
assert_equals(cookies[0].name, 'cookie-name-2'); assert_equals(cookies[0].name, 'cookie-name-2');
assert_equals(cookies[0].value, 'cookie-value-2'); assert_equals(cookies[0].value, 'cookie-value-2');
await async_cleanup(() => cookieStore.delete('cookie-name')); await async_cleanup(() => cookieStore.delete('cookie-name'));
await async_cleanup(() => cookieStore.delete('cookie-name-2')); await async_cleanup(() => cookieStore.delete('cookie-name-2'));
}, 'cookieStore.getAll with matchType set to startsWith and name in options'); }, 'cookieStore.getAll with matchType set to starts-with and name in options');
...@@ -58,12 +58,12 @@ promise_test(async testCase => { ...@@ -58,12 +58,12 @@ promise_test(async testCase => {
await cookieStore.set('cookie-name', 'cookie-value'); await cookieStore.set('cookie-name', 'cookie-value');
const cookie = await cookieStore.get( const cookie = await cookieStore.get(
'cookie-na', { matchType: 'startsWith' }); 'cookie-na', { matchType: 'starts-with' });
assert_equals(cookie.name, 'cookie-name'); assert_equals(cookie.name, 'cookie-name');
assert_equals(cookie.value, 'cookie-value'); assert_equals(cookie.value, 'cookie-value');
async_cleanup(() => cookieStore.delete('cookie-name')); async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'cookieStore.get with matchType set to startsWith'); }, 'cookieStore.get with matchType set to starts-with');
promise_test(async testCase => { promise_test(async testCase => {
await cookieStore.set('cookie-name', 'cookie-value'); await cookieStore.set('cookie-name', 'cookie-value');
...@@ -78,9 +78,9 @@ promise_test(async testCase => { ...@@ -78,9 +78,9 @@ promise_test(async testCase => {
await cookieStore.set('cookie-name', 'cookie-value'); await cookieStore.set('cookie-name', 'cookie-value');
const cookie = await cookieStore.get( const cookie = await cookieStore.get(
{ matchType: 'startsWith', name: 'cookie-na' }); { matchType: 'starts-with', name: 'cookie-na' });
assert_equals(cookie.name, 'cookie-name'); assert_equals(cookie.name, 'cookie-name');
assert_equals(cookie.value, 'cookie-value'); assert_equals(cookie.value, 'cookie-value');
async_cleanup(() => cookieStore.delete('cookie-name')); async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'cookieStore.get with matchType set to startsWith and name in options'); }, 'cookieStore.get with matchType set to starts-with and name in options');
'use strict';
// Workaround because add_cleanup doesn't support async functions yet.
// See https://github.com/web-platform-tests/wpt/issues/6075
async function async_cleanup(cleanup_function) {
try {
await cleanup_function();
} catch (e) {
// Errors in cleanup functions shouldn't result in test failures.
}
}
promise_test(async testCase => {
await cookieStore.set('cookie-name', 'cookie-value');
await cookieStore.delete('cookie-name-2');
const has_cookie = await cookieStore.has('cookie-name');
assert_equals(has_cookie, true);
const has_cookie2 = await cookieStore.has('cookie-name-2');
assert_equals(has_cookie2, false);
await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'cookieStore.has with positional name');
promise_test(async testCase => {
await cookieStore.set('cookie-name', 'cookie-value');
await cookieStore.delete('cookie-name-2');
const has_cookie = await cookieStore.has({ name: 'cookie-name' });
assert_equals(has_cookie, true);
const has_cookie2 = await cookieStore.has({ name: 'cookie-name-2' });
assert_equals(has_cookie2, false);
await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'cookieStore.has with name in options');
promise_test(async testCase => {
await cookieStore.set('cookie-name', 'cookie-value');
await promise_rejects(testCase, new TypeError(), cookieStore.has(
'cookie-name', { name: 'cookie-name' }));
await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'cookieStore.has with name in both positional arguments and options');
promise_test(async testCase => {
await cookieStore.set('cookie-name', 'cookie-value');
const has_cookie = await cookieStore.has(
'cookie-na', { matchType: 'equals' });
assert_equals(has_cookie, false);
const has_cookie2 = await cookieStore.has(
'cookie-name', { matchType: 'equals' });
assert_equals(has_cookie2, true);
await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'cookieStore.has with matchType explicitly set to equals');
promise_test(async testCase => {
await cookieStore.set('cookie-name', 'cookie-value');
const has_cookie = await cookieStore.has(
'cookie-na', { matchType: 'startsWith' });
assert_equals(has_cookie, true);
const has_cookie2 = await cookieStore.has(
'cookie-name-', { matchType: 'startsWith' });
assert_equals(has_cookie2, false);
await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'cookieStore.has with matchType set to startsWith');
promise_test(async testCase => {
await cookieStore.set('cookie-name', 'cookie-value');
await promise_rejects(testCase, new TypeError(), cookieStore.has(
'cookie-name', { matchType: 'invalid' }));
await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'cookieStore.has with invalid matchType');
promise_test(async testCase => {
await cookieStore.set('cookie-name', 'cookie-value');
const has_cookie = await cookieStore.has(
{ matchType: 'startsWith', name: 'cookie-na' });
assert_equals(has_cookie, true);
const has_cookie2 = await cookieStore.has(
{ matchType: 'startsWith', name: 'cookie-name-' });
assert_equals(has_cookie2, false);
await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'cookieStore.has with matchType set to startsWith and name in options');
'use strict';
// Workaround because add_cleanup doesn't support async functions yet.
// See https://github.com/web-platform-tests/wpt/issues/6075
async function async_cleanup(cleanup_function) {
try {
await cleanup_function();
} catch (e) {
// Errors in cleanup functions shouldn't result in test failures.
}
}
promise_test(async testCase => {
await cookieStore.set('cookie-name', 'cookie-value');
assert_equals(await cookieStore.has('cookie-name'), true);
await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'cookieStore.has returns true for cookie set by cookieStore.set()');
promise_test(async testCase => {
await cookieStore.delete('cookie-name');
assert_equals(await cookieStore.has('cookie-name'), false);
await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'cookieStore.has returns false for cookie deleted by cookieStore.delete()');
...@@ -40,9 +40,9 @@ ...@@ -40,9 +40,9 @@
null, null,
'get with ${prefix} prefix name option should not reject'); 'get with ${prefix} prefix name option should not reject');
assert_equals( assert_equals(
await cookieStore.get({name: prefix, matchType: 'startsWith'}), await cookieStore.get({name: prefix, matchType: 'starts-with'}),
null, null,
'get with ${prefix} name and startsWith options should not reject'); 'get with ${prefix} name and starts-with options should not reject');
}, `cookieStore.get with ${prefix} name on non-secure origin`); }, `cookieStore.get with ${prefix} name on non-secure origin`);
promise_test(async testCase => { promise_test(async testCase => {
...@@ -55,9 +55,9 @@ ...@@ -55,9 +55,9 @@
[], [],
'getAll with ${prefix} prefix name option should not reject'); 'getAll with ${prefix} prefix name option should not reject');
assert_array_equals( assert_array_equals(
await cookieStore.getAll({name: prefix, matchType: 'startsWith'}), await cookieStore.getAll({name: prefix, matchType: 'starts-with'}),
[], [],
'getAll with ${prefix} name and startsWith options should not reject'); 'getAll with ${prefix} name and starts-with options should not reject');
}, `cookieStore.getAll with ${prefix} name on non-secure origin`); }, `cookieStore.getAll with ${prefix} name on non-secure origin`);
promise_test(async testCase => { promise_test(async testCase => {
......
...@@ -8,7 +8,6 @@ importScripts( ...@@ -8,7 +8,6 @@ importScripts(
"cookieStore_delete_arguments.tentative.window.js", "cookieStore_delete_arguments.tentative.window.js",
"cookieStore_get_arguments.tentative.window.js", "cookieStore_get_arguments.tentative.window.js",
"cookieStore_getAll_arguments.tentative.window.js", "cookieStore_getAll_arguments.tentative.window.js",
"cookieStore_has_arguments.tentative.window.js",
"cookieStore_set_arguments.tentative.window.js"); "cookieStore_set_arguments.tentative.window.js");
done(); done();
...@@ -7,7 +7,6 @@ importScripts("/resources/testharness.js"); ...@@ -7,7 +7,6 @@ importScripts("/resources/testharness.js");
importScripts( importScripts(
"cookieStore_get_delete_basic.tentative.window.js", "cookieStore_get_delete_basic.tentative.window.js",
"cookieStore_get_set_basic.tentative.window.js", "cookieStore_get_set_basic.tentative.window.js",
"cookieStore_getAll_set_basic.tentative.window.js", "cookieStore_getAll_set_basic.tentative.window.js");
"cookieStore_has_basic.tentative.window.js");
done(); done();
...@@ -13,7 +13,7 @@ self.addEventListener('install', (event) => { ...@@ -13,7 +13,7 @@ self.addEventListener('install', (event) => {
{ name: 'cookie-name1', matchType: 'equals', url: '/scope/path1' }]); { name: 'cookie-name1', matchType: 'equals', url: '/scope/path1' }]);
await cookieStore.subscribeToChanges([ await cookieStore.subscribeToChanges([
{ }, // Test the default values for subscription properties. { }, // Test the default values for subscription properties.
{ name: 'cookie-prefix', matchType: 'startsWith' }, { name: 'cookie-prefix', matchType: 'starts-with' },
]); ]);
})()); })());
}); });
...@@ -54,10 +54,10 @@ promise_test(async testCase => { ...@@ -54,10 +54,10 @@ promise_test(async testCase => {
assert_equals('equals', subscriptions[0].matchType); assert_equals('equals', subscriptions[0].matchType);
assert_equals(subscriptions[1].name, 'cookie-prefix'); assert_equals(subscriptions[1].name, 'cookie-prefix');
assert_equals('startsWith', subscriptions[1].matchType); assert_equals('starts-with', subscriptions[1].matchType);
assert_false('name' in subscriptions[2]); assert_false('name' in subscriptions[2]);
assert_equals('startsWith', subscriptions[2].matchType); assert_equals('starts-with', subscriptions[2].matchType);
}, 'getChangeSubscriptions returns subscriptions passed to subscribeToChanges'); }, 'getChangeSubscriptions returns subscriptions passed to subscribeToChanges');
promise_test(async testCase => { promise_test(async testCase => {
......
...@@ -3,6 +3,10 @@ ...@@ -3,6 +3,10 @@
dictionary CookieListItem { dictionary CookieListItem {
USVString name; USVString name;
USVString value; USVString value;
USVString? domain;
USVString path;
DOMTimeStamp? expires;
boolean secure;
}; };
typedef sequence<CookieListItem> CookieList; typedef sequence<CookieListItem> CookieList;
...@@ -35,7 +39,7 @@ dictionary ExtendableCookieChangeEventInit : ExtendableEventInit { ...@@ -35,7 +39,7 @@ dictionary ExtendableCookieChangeEventInit : ExtendableEventInit {
enum CookieMatchType { enum CookieMatchType {
"equals", "equals",
"startsWith" "starts-with"
}; };
dictionary CookieStoreGetOptions { dictionary CookieStoreGetOptions {
...@@ -63,9 +67,6 @@ dictionary CookieStoreSetOptions { ...@@ -63,9 +67,6 @@ dictionary CookieStoreSetOptions {
Promise<CookieListItem?> get(USVString name, optional CookieStoreGetOptions options); Promise<CookieListItem?> get(USVString name, optional CookieStoreGetOptions options);
Promise<CookieListItem?> get(optional CookieStoreGetOptions options); Promise<CookieListItem?> get(optional CookieStoreGetOptions options);
Promise<boolean> has(USVString name, optional CookieStoreGetOptions options);
Promise<boolean> has(optional CookieStoreGetOptions options);
Promise<void> set(USVString name, USVString value, optional CookieStoreSetOptions options); Promise<void> set(USVString name, USVString value, optional CookieStoreSetOptions options);
Promise<void> set(CookieStoreSetOptions options); Promise<void> set(CookieStoreSetOptions options);
......
...@@ -165,7 +165,6 @@ interface CookieStore : EventTarget ...@@ -165,7 +165,6 @@ interface CookieStore : EventTarget
method get method get
method getAll method getAll
method getChangeSubscriptions method getChangeSubscriptions
method has
method set method set
method subscribeToChanges method subscribeToChanges
interface CountQueuingStrategy interface CountQueuingStrategy
......
...@@ -1104,7 +1104,6 @@ interface CookieStore : EventTarget ...@@ -1104,7 +1104,6 @@ interface CookieStore : EventTarget
method delete method delete
method get method get
method getAll method getAll
method has
method set method set
setter onchange setter onchange
interface CountQueuingStrategy interface CountQueuingStrategy
......
...@@ -40,7 +40,7 @@ network::mojom::blink::CookieManagerGetOptionsPtr ToBackendOptions( ...@@ -40,7 +40,7 @@ network::mojom::blink::CookieManagerGetOptionsPtr ToBackendOptions(
// TODO(crbug.com/729800): Handle the url option. // TODO(crbug.com/729800): Handle the url option.
if (options.matchType() == "startsWith") { if (options.matchType() == "starts-with") {
backend_options->match_type = backend_options->match_type =
network::mojom::blink::CookieMatchType::STARTS_WITH; network::mojom::blink::CookieMatchType::STARTS_WITH;
} else { } else {
...@@ -205,7 +205,7 @@ blink::mojom::blink::CookieChangeSubscriptionPtr ToBackendSubscription( ...@@ -205,7 +205,7 @@ blink::mojom::blink::CookieChangeSubscriptionPtr ToBackendSubscription(
backend_subscription->url = default_cookie_url; backend_subscription->url = default_cookie_url;
} }
if (subscription.matchType() == "startsWith") { if (subscription.matchType() == "starts-with") {
backend_subscription->match_type = backend_subscription->match_type =
network::mojom::blink::CookieMatchType::STARTS_WITH; network::mojom::blink::CookieMatchType::STARTS_WITH;
} else { } else {
...@@ -240,7 +240,7 @@ void ToCookieChangeSubscription( ...@@ -240,7 +240,7 @@ void ToCookieChangeSubscription(
switch (backend_subscription.match_type) { switch (backend_subscription.match_type) {
case network::mojom::blink::CookieMatchType::STARTS_WITH: case network::mojom::blink::CookieMatchType::STARTS_WITH:
subscription.setMatchType(WTF::String("startsWith")); subscription.setMatchType(WTF::String("starts-with"));
break; break;
case network::mojom::blink::CookieMatchType::EQUALS: case network::mojom::blink::CookieMatchType::EQUALS:
subscription.setMatchType(WTF::String("equals")); subscription.setMatchType(WTF::String("equals"));
...@@ -310,20 +310,6 @@ ScriptPromise CookieStore::get(ScriptState* script_state, ...@@ -310,20 +310,6 @@ ScriptPromise CookieStore::get(ScriptState* script_state,
&CookieStore::GetAllForUrlToGetResult, exception_state); &CookieStore::GetAllForUrlToGetResult, exception_state);
} }
ScriptPromise CookieStore::has(ScriptState* script_state,
const CookieStoreGetOptions& options,
ExceptionState& exception_state) {
return has(script_state, WTF::String(), options, exception_state);
}
ScriptPromise CookieStore::has(ScriptState* script_state,
const String& name,
const CookieStoreGetOptions& options,
ExceptionState& exception_state) {
return DoRead(script_state, name, options,
&CookieStore::GetAllForUrlToHasResult, exception_state);
}
ScriptPromise CookieStore::set(ScriptState* script_state, ScriptPromise CookieStore::set(ScriptState* script_state,
const CookieStoreSetOptions& options, const CookieStoreSetOptions& options,
ExceptionState& exception_state) { ExceptionState& exception_state) {
...@@ -550,17 +536,6 @@ void CookieStore::GetAllForUrlToGetResult( ...@@ -550,17 +536,6 @@ void CookieStore::GetAllForUrlToGetResult(
resolver->Resolve(cookie); resolver->Resolve(cookie);
} }
// static
void CookieStore::GetAllForUrlToHasResult(
ScriptPromiseResolver* resolver,
const Vector<WebCanonicalCookie>& backend_cookies) {
ScriptState* script_state = resolver->GetScriptState();
if (!script_state->ContextIsValid())
return;
resolver->Resolve(!backend_cookies.IsEmpty());
}
ScriptPromise CookieStore::DoWrite(ScriptState* script_state, ScriptPromise CookieStore::DoWrite(ScriptState* script_state,
const String& name, const String& name,
const String& value, const String& value,
......
...@@ -57,13 +57,6 @@ class CookieStore final : public EventTargetWithInlineData, ...@@ -57,13 +57,6 @@ class CookieStore final : public EventTargetWithInlineData,
const String& name, const String& name,
const CookieStoreGetOptions&, const CookieStoreGetOptions&,
ExceptionState&); ExceptionState&);
ScriptPromise has(ScriptState*,
const CookieStoreGetOptions&,
ExceptionState&);
ScriptPromise has(ScriptState*,
const String& name,
const CookieStoreGetOptions&,
ExceptionState&);
ScriptPromise set(ScriptState*, ScriptPromise set(ScriptState*,
const CookieStoreSetOptions&, const CookieStoreSetOptions&,
...@@ -144,12 +137,6 @@ class CookieStore final : public EventTargetWithInlineData, ...@@ -144,12 +137,6 @@ class CookieStore final : public EventTargetWithInlineData,
ScriptPromiseResolver*, ScriptPromiseResolver*,
const Vector<WebCanonicalCookie>& backend_result); const Vector<WebCanonicalCookie>& backend_result);
// Converts the result of a RestrictedCookieManager::GetAllForUrl mojo call to
// the promise result expected by CookieStore.has.
static void GetAllForUrlToHasResult(
ScriptPromiseResolver*,
const Vector<WebCanonicalCookie>& backend_result);
// Common code in CookieStore::delete and CookieStore::set. // Common code in CookieStore::delete and CookieStore::set.
ScriptPromise DoWrite(ScriptState*, ScriptPromise DoWrite(ScriptState*,
const String& name, const String& name,
......
...@@ -17,10 +17,6 @@ ...@@ -17,10 +17,6 @@
USVString name, optional CookieStoreGetOptions options); USVString name, optional CookieStoreGetOptions options);
[CallWith=ScriptState, Measure, RaisesException] Promise<CookieListItem?> get( [CallWith=ScriptState, Measure, RaisesException] Promise<CookieListItem?> get(
optional CookieStoreGetOptions options); optional CookieStoreGetOptions options);
[CallWith=ScriptState, Measure, RaisesException] Promise<boolean> has(
USVString name, optional CookieStoreGetOptions options);
[CallWith=ScriptState, Measure, RaisesException] Promise<boolean> has(
optional CookieStoreGetOptions options);
// https://github.com/WICG/async-cookies-api/blob/gh-pages/explainer.md#writing // https://github.com/WICG/async-cookies-api/blob/gh-pages/explainer.md#writing
[CallWith=ScriptState, Measure, RaisesException] Promise<void> set( [CallWith=ScriptState, Measure, RaisesException] Promise<void> set(
......
...@@ -6,7 +6,7 @@ ...@@ -6,7 +6,7 @@
enum CookieMatchType { enum CookieMatchType {
"equals", "equals",
"startsWith" "starts-with"
}; };
dictionary CookieStoreGetOptions { dictionary CookieStoreGetOptions {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment