Commit 1d64fd34 authored by Jay Harris's avatar Jay Harris Committed by Commit Bot

Removes 'scope' from mojo interface, as it is ignored in the browser.

This is part of a series of CLs redesigning the Badging API based on
TPAC feedback:
https://github.com/WICG/badging/issues/55

In a followup CL, the scope member will no longer be exposed to the
web:
https://chromium-review.googlesource.com/c/chromium/src/+/1816002

Note: The scope parameter was ignored in a previous CL, which is
why there are no unit/browser test changes.

Bug: 1006665
Change-Id: I176dfc28abaf661aaeaa1622e059c2b8cc851e12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1818892
Commit-Queue: Jay Harris <harrisjay@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701015}
parent 55ed70b5
......@@ -99,8 +99,7 @@ void BadgeManager::UpdateBadge(const GURL& scope,
delegate_->OnBadgeUpdated(scope);
}
void BadgeManager::SetBadge(const GURL& /*scope*/,
blink::mojom::BadgeValuePtr mojo_value) {
void BadgeManager::SetBadge(blink::mojom::BadgeValuePtr mojo_value) {
if (mojo_value->is_number() && mojo_value->get_number() == 0) {
mojo::ReportBadMessage(
"|value| should not be zero when it is |number| (ClearBadge should be "
......@@ -120,7 +119,7 @@ void BadgeManager::SetBadge(const GURL& /*scope*/,
UpdateBadge(app_scope.value(), base::make_optional(value));
}
void BadgeManager::ClearBadge(const GURL& /*scope*/) {
void BadgeManager::ClearBadge() {
const base::Optional<GURL> app_scope =
GetAppScopeForContext(receivers_.current_context());
if (!app_scope)
......
......@@ -75,11 +75,8 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService {
// blink::mojom::BadgeService:
// Note: These are private to stop them being called outside of mojo as they
// require a mojo binding context.
// TODO(crbug.com/1006665): Remove scope from the mojo interface in SetBadge
// and ClearBadge.
void SetBadge(const GURL& /*scope*/,
blink::mojom::BadgeValuePtr value) override;
void ClearBadge(const GURL& /*scope*/) override;
void SetBadge(blink::mojom::BadgeValuePtr value) override;
void ClearBadge() override;
// Finds the scope URL of the most specific badge for |scope|. Returns
// GURL::EmptyGURL() if no match is found.
......
......@@ -20,12 +20,8 @@ union BadgeValue {
// Interface for handling badge messages from frames and subframes.
interface BadgeService {
// Asks the browser process to set a badge.
// |scope| specifies which badges to set. Note: This must be on the same
// origin as the caller.
SetBadge(url.mojom.Url scope, BadgeValue value);
SetBadge(BadgeValue value);
// Asks the browser to clear a badge.
// |scope| specifies the badges to clear. Note: This must be on the same
// origin as the caller.
ClearBadge(url.mojom.Url scope);
ClearBadge();
};
......@@ -79,7 +79,7 @@ void Badge::SetBadge(WTF::String scope,
if (!scope_url)
return;
badge_service_->SetBadge(*scope_url, std::move(value));
badge_service_->SetBadge(std::move(value));
}
void Badge::ClearBadge(WTF::String scope, ExceptionState& exception_state) {
......@@ -87,7 +87,7 @@ void Badge::ClearBadge(WTF::String scope, ExceptionState& exception_state) {
if (!scope_url)
return;
badge_service_->ClearBadge(*scope_url);
badge_service_->ClearBadge();
}
void Badge::Trace(blink::Visitor* visitor) {
......
......@@ -12,36 +12,21 @@
<script>
// Negative value not allowed.
badge_test(() => { ExperimentalBadge.set(-1); }, undefined, undefined,
'TypeError');
badge_test(() => { ExperimentalBadge.set(-1); }, undefined, 'TypeError');
// Value too large (2^53).
badge_test(() => { ExperimentalBadge.set(9007199254740992); }, undefined,
undefined, 'TypeError');
badge_test(() => { ExperimentalBadge.set(9007199254740992); },
undefined,
'TypeError');
// Illegal numeric values.
badge_test(() => { ExperimentalBadge.set(Infinity); }, undefined, undefined,
'TypeError');
badge_test(() => { ExperimentalBadge.set(-Infinity); }, undefined, undefined,
'TypeError');
badge_test(() => { ExperimentalBadge.set(NaN); }, undefined, undefined,
'TypeError');
badge_test(() => { ExperimentalBadge.set(Infinity); }, undefined, 'TypeError');
badge_test(() => { ExperimentalBadge.set(-Infinity); }, undefined, 'TypeError');
badge_test(() => { ExperimentalBadge.set(NaN); }, undefined, 'TypeError');
// Other values that can't convert to a long.
badge_test(() => { ExperimentalBadge.set("Foo"); }, undefined, undefined,
'TypeError');
badge_test(() => { ExperimentalBadge.set({}); }, undefined, undefined,
'TypeError');
// Wrong origin.
badge_test(() => {
ExperimentalBadge.set(1, {scope: 'https://wrongorigin.com/scope'});
}, undefined, undefined, 'SecurityError');
// Invalid URL.
badge_test(() => {
ExperimentalBadge.set(1, {scope: 'https://example.com:99999'});
}, undefined, undefined, 'TypeError');
badge_test(() => { ExperimentalBadge.set("Foo"); }, undefined, 'TypeError');
badge_test(() => { ExperimentalBadge.set({}); }, undefined, 'TypeError');
</script>
</body>
......
......@@ -11,51 +11,29 @@
<body>
<script>
// The default scope URL is the current origin.
const defaultScope = new URL('/', location).href;
badge_test(() => { ExperimentalBadge.set(); }, 'flag');
badge_test(() => { ExperimentalBadge.set(); }, 'flag', defaultScope);
badge_test(() => { ExperimentalBadge.set(undefined); }, 'flag');
badge_test(() => { ExperimentalBadge.set(undefined); }, 'flag', defaultScope);
badge_test(() => { ExperimentalBadge.set(1); }, 'number:1', defaultScope);
badge_test(() => { ExperimentalBadge.set(1); }, 'number:1');
// Non-whole number should round down to nearest integer.
badge_test(() => { ExperimentalBadge.set(10.6); }, 'number:10', defaultScope);
badge_test(() => { ExperimentalBadge.set(10.6); }, 'number:10');
// Maximum allowed value (2^53 - 1).
badge_test(() => { ExperimentalBadge.set(9007199254740991); },
'number:9007199254740991', defaultScope);
'number:9007199254740991');
// Setting the Badge to 0 should be equivalent to clearing the badge.
badge_test(() => { ExperimentalBadge.set(0); }, 'clear', defaultScope);
badge_test(() => { ExperimentalBadge.set(0); }, 'clear');
badge_test(() => { ExperimentalBadge.clear(); }, 'clear', defaultScope);
badge_test(() => { ExperimentalBadge.clear(); }, 'clear');
// Non-numeric values that convert to integer.
badge_test(() => { ExperimentalBadge.set(null); }, 'clear', defaultScope);
badge_test(() => { ExperimentalBadge.set(false); }, 'clear', defaultScope);
badge_test(() => { ExperimentalBadge.set(true); }, 'number:1', defaultScope);
badge_test(() => { ExperimentalBadge.set('3'); }, 'number:3', defaultScope);
// Test options dictionary.
badge_test(() => { ExperimentalBadge.set(1, {}); }, 'number:1', defaultScope);
badge_test(() => { ExperimentalBadge.set(1, {foo: 4}); }, 'number:1',
defaultScope);
badge_test(() => {
ExperimentalBadge.set(1, {scope: new URL('/scope', location).href});
}, 'number:1', new URL('/scope', location).href);
// Scope URL resolved against document.
badge_test(() => { ExperimentalBadge.set(1, {scope: '/scope'}); },
'number:1', new URL('/scope', location).href);
// Explicit undefined to set a flag with a scope.
// TODO(mgiuca): Currently fails with TypeError. https://crbug.com/1001411
// badge_test(() => {
// ExperimentalBadge.set(undefined, {scope: 'https://example.com/scope'});
// }, 'number:1', 'https://example.com/scope');
badge_test(() => { ExperimentalBadge.set(null); }, 'clear');
badge_test(() => { ExperimentalBadge.set(false); }, 'clear');
badge_test(() => { ExperimentalBadge.set(true); }, 'number:1');
badge_test(() => { ExperimentalBadge.set('3'); }, 'number:3');
</script>
</body>
......
......@@ -10,16 +10,15 @@ class MockBadgeService {
this.interceptor_.start();
}
init_(expectedAction, expectedScope) {
init_(expectedAction) {
this.expectedAction = expectedAction;
this.expectedScope = expectedScope;
return new Promise((resolve, reject) => {
this.reject_ = reject;
this.resolve_ = resolve;
});
}
setBadge(scope, value) {
setBadge(value) {
// Accessing number when the union is a flag will throw, so read the
// value in a try catch.
let number;
......@@ -32,17 +31,15 @@ class MockBadgeService {
try {
const action = number === undefined ? 'flag' : 'number:' + number;
assert_equals(action, this.expectedAction);
assert_equals(scope.url, this.expectedScope);
this.resolve_();
} catch (error) {
this.reject_(error);
}
}
clearBadge(scope) {
clearBadge() {
try {
assert_equals('clear', this.expectedAction);
assert_equals(scope.url, this.expectedScope);
this.resolve_();
} catch (error) {
this.reject_(error);
......@@ -67,9 +64,9 @@ function callAndObserveErrors(func, expectedErrorName) {
});
}
function badge_test(func, expectedAction, expectedScope, expectedError) {
function badge_test(func, expectedAction, expectedError) {
promise_test(() => {
let mockPromise = mockBadgeService.init_(expectedAction, expectedScope);
let mockPromise = mockBadgeService.init_(expectedAction);
return Promise.race(
[callAndObserveErrors(func, expectedError), mockPromise]);
});
......
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