- Revision
- 131280
- Author
- [email protected]
- Date
- 2012-10-14 16:50:30 -0700 (Sun, 14 Oct 2012)
Log Message
Allow notification origin permission request when no js callback is provided
https://bugs.webkit.org/show_bug.cgi?id=63615
<rdar://problem/11059590>
Reviewed by Sam Weinig.
Source/WebCore:
Instead of throwing a type error when no callback is provided, we pass a null callback.
Test: http/tests/notifications/legacy/request-no-callback.html
* bindings/js/JSDesktopNotificationsCustom.cpp:
(WebCore::JSNotificationCenter::requestPermission):
Source/WebKit/mac:
Introduce a boolean to determine whether the request was using the legacy or standard API. This way,
we do not fall through to calling the standard API's callback if the legacy API's callback is null.
* WebCoreSupport/WebNotificationClient.mm:
(WebCore):
(-[WebNotificationPolicyListener initWithVoidCallback:]):
(-[WebNotificationPolicyListener allow]):
(-[WebNotificationPolicyListener deny]):
Source/WebKit2:
Null checks already exist for both standard and legacy API callbacks, so no changes are needed here
like there are in WebKit 1. The checks existed because the callbacks are held in a hash map used to keep
track of pending requests.
Also, add a check for a null callback when short circuiting.
* WebProcess/Notifications/NotificationPermissionRequestManager.cpp:
(WebKit::NotificationPermissionRequestManager::startRequest):
Tools:
Teach DRT to look at the existing entries in the permission hash map when permission is requested.
* DumpRenderTree/mac/MockWebNotificationProvider.h: Expose policyForOrigin.
* DumpRenderTree/mac/MockWebNotificationProvider.mm:
(-[MockWebNotificationProvider setWebNotificationOrigin:permission:]):
* DumpRenderTree/mac/UIDelegate.mm:
(-[UIDelegate webView:decidePolicyForNotificationRequestFromOrigin:listener:]): Look at whether a
policy for the origin already exists. If so, accept or deny the request as appropriate. Otherwise,
accept by default.
LayoutTests:
* http/tests/notifications/legacy/request-expected.txt:
* http/tests/notifications/legacy/request-no-callback-expected.txt: Added.
* http/tests/notifications/legacy/request-no-callback.html: Calls webkitNotifications.requestPermission()
with no callback, with default and denied permission. For WebKit2, if the permission is not default, it
will short circuit instead of creating a pending request.
* http/tests/notifications/legacy/request.html: The test mistakenly uses the standard API instead of
the legacy API. The results don't change, but we make sure that we call
webkitNotifications.requestPermission() with a callback to test full coverage of the legacy API. We also
expand the test to cover both default and denied permissions.
* http/tests/notifications/request-expected.txt:
* http/tests/notifications/request.html: Expand test to cover both default and denied permissions.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (131279 => 131280)
--- trunk/LayoutTests/ChangeLog 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/LayoutTests/ChangeLog 2012-10-14 23:50:30 UTC (rev 131280)
@@ -1,3 +1,23 @@
+2012-10-14 Jon Lee <[email protected]>
+
+ Allow notification origin permission request when no js callback is provided
+ https://bugs.webkit.org/show_bug.cgi?id=63615
+ <rdar://problem/11059590>
+
+ Reviewed by Sam Weinig.
+
+ * http/tests/notifications/legacy/request-expected.txt:
+ * http/tests/notifications/legacy/request-no-callback-expected.txt: Added.
+ * http/tests/notifications/legacy/request-no-callback.html: Calls webkitNotifications.requestPermission()
+ with no callback, with default and denied permission. For WebKit2, if the permission is not default, it
+ will short circuit instead of creating a pending request.
+ * http/tests/notifications/legacy/request.html: The test mistakenly uses the standard API instead of
+ the legacy API. The results don't change, but we make sure that we call
+ webkitNotifications.requestPermission() with a callback to test full coverage of the legacy API. We also
+ expand the test to cover both default and denied permissions.
+ * http/tests/notifications/request-expected.txt:
+ * http/tests/notifications/request.html: Expand test to cover both default and denied permissions.
+
2012-10-12 Adam Barth <[email protected]>
Unreviewed gardening.
Modified: trunk/LayoutTests/http/tests/notifications/legacy/request-expected.txt (131279 => 131280)
--- trunk/LayoutTests/http/tests/notifications/legacy/request-expected.txt 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/LayoutTests/http/tests/notifications/legacy/request-expected.txt 2012-10-14 23:50:30 UTC (rev 131280)
@@ -3,8 +3,11 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+Requesting permission with default permission
PASS window.webkitNotifications.checkPermission() is 1
PASS window.webkitNotifications.checkPermission() is 0
+Requesting permission with non-default permission
+PASS window.webkitNotifications.checkPermission() is 2
PASS successfullyParsed is true
TEST COMPLETE
Added: trunk/LayoutTests/http/tests/notifications/legacy/request-no-callback-expected.txt (0 => 131280)
--- trunk/LayoutTests/http/tests/notifications/legacy/request-no-callback-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/notifications/legacy/request-no-callback-expected.txt 2012-10-14 23:50:30 UTC (rev 131280)
@@ -0,0 +1,15 @@
+This test checks that requesting permission without a callback works. This supports legacy API.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Requesting permission with default permission
+PASS window.webkitNotifications.checkPermission() is 1
+PASS webkitNotifications.requestPermission() did not throw exception.
+Requesting permission with non-default permission
+PASS window.webkitNotifications.checkPermission() is 2
+PASS webkitNotifications.requestPermission() did not throw exception.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/http/tests/notifications/legacy/request-no-callback.html (0 => 131280)
--- trunk/LayoutTests/http/tests/notifications/legacy/request-no-callback.html (rev 0)
+++ trunk/LayoutTests/http/tests/notifications/legacy/request-no-callback.html 2012-10-14 23:50:30 UTC (rev 131280)
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<script src=""
+<script src=""
+<p id="description"></p>
+<div id="console"></div>
+<script>
+description("This test checks that requesting permission without a callback works. This supports legacy API.");
+
+function step1() {
+ debug("Requesting permission with default permission");
+ shouldBe("window.webkitNotifications.checkPermission()", "1");
+ shouldNotThrow("webkitNotifications.requestPermission()");
+ setTimeout(step2, 0);
+}
+
+function step2() {
+ testRunner.denyWebNotificationPermission(testURL);
+ debug("Requesting permission with non-default permission");
+ shouldBe("window.webkitNotifications.checkPermission()", "2");
+ shouldNotThrow("webkitNotifications.requestPermission()");
+ testCompleted();
+}
+
+if (window.testRunner) {
+ testRunner.waitUntilDone();
+ step1();
+}
+
+setTimeout(testCompleted, 100);
+</script>
Modified: trunk/LayoutTests/http/tests/notifications/legacy/request.html (131279 => 131280)
--- trunk/LayoutTests/http/tests/notifications/legacy/request.html 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/LayoutTests/http/tests/notifications/legacy/request.html 2012-10-14 23:50:30 UTC (rev 131280)
@@ -4,17 +4,29 @@
<p id="description"></p>
<div id="console"></div>
<script>
-if (window.testRunner)
- testRunner.waitUntilDone();
-
description("This test checks that a request for permission is made.");
+function step1() {
+ debug("Requesting permission with default permission");
+ shouldBe("window.webkitNotifications.checkPermission()", "1");
+ webkitNotifications.requestPermission(function() {
+ shouldBe("window.webkitNotifications.checkPermission()", "0");
+ setTimeout(step2, 0);
+ });
+}
+
+function step2() {
+ testRunner.denyWebNotificationPermission(testURL);
+ debug("Requesting permission with non-default permission");
+ webkitNotifications.requestPermission(function() {
+ shouldBe("window.webkitNotifications.checkPermission()", "2");
+ testCompleted();
+ });
+}
+
if (window.testRunner) {
- shouldBe("window.webkitNotifications.checkPermission()", "1");
- Notification.requestPermission(function() {
- shouldBe("window.webkitNotifications.checkPermission()", "0");
- testCompleted();
- });
+ testRunner.waitUntilDone();
+ step1();
}
setTimeout(testCompleted, 100);
Modified: trunk/LayoutTests/http/tests/notifications/request-expected.txt (131279 => 131280)
--- trunk/LayoutTests/http/tests/notifications/request-expected.txt 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/LayoutTests/http/tests/notifications/request-expected.txt 2012-10-14 23:50:30 UTC (rev 131280)
@@ -3,9 +3,13 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+Requesting permission with default permission
PASS Notification.permission is "default"
-PASS permission is granted.
+PASS permission is granted
PASS Notification.permission is "granted"
+Requesting permission with non-default permission
+PASS permission is denied
+PASS Notification.permission is "denied"
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/http/tests/notifications/request.html (131279 => 131280)
--- trunk/LayoutTests/http/tests/notifications/request.html 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/LayoutTests/http/tests/notifications/request.html 2012-10-14 23:50:30 UTC (rev 131280)
@@ -4,21 +4,37 @@
<p id="description"></p>
<div id="console"></div>
<script>
-if (window.testRunner)
- testRunner.waitUntilDone();
-
description("This test checks that a request for permission is made.");
+function step1() {
+ debug("Requesting permission with default permission");
+ shouldBeEqualToString("Notification.permission", "default");
+ Notification.requestPermission(function(permission) {
+ if (permission == 'granted')
+ testPassed("permission is granted");
+ else
+ testFailed("permission should be granted, but is " + permission);
+ shouldBeEqualToString("Notification.permission", "granted");
+ setTimeout(step2, 0);
+ });
+}
+
+function step2() {
+ debug("Requesting permission with non-default permission");
+ testRunner.denyWebNotificationPermission(testURL);
+ Notification.requestPermission(function(permission) {
+ if (permission == 'denied')
+ testPassed("permission is denied");
+ else
+ testFailed("permission should be denied, but is " + permission);
+ shouldBeEqualToString("Notification.permission", "denied");
+ testCompleted();
+ });
+}
+
if (window.testRunner) {
- shouldBeEqualToString("Notification.permission", "default");
- Notification.requestPermission(function(permission) {
- if (permission == 'granted')
- testPassed("permission is granted.");
- else
- testFailed("permission should be granted, but is " + permission + ".");
- shouldBeEqualToString("Notification.permission", "granted");
- testCompleted();
- });
+ testRunner.waitUntilDone();
+ step1();
}
setTimeout(testCompleted, 100);
Modified: trunk/Source/WebCore/ChangeLog (131279 => 131280)
--- trunk/Source/WebCore/ChangeLog 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/Source/WebCore/ChangeLog 2012-10-14 23:50:30 UTC (rev 131280)
@@ -1,3 +1,18 @@
+2012-10-14 Jon Lee <[email protected]>
+
+ Allow notification origin permission request when no js callback is provided
+ https://bugs.webkit.org/show_bug.cgi?id=63615
+ <rdar://problem/11059590>
+
+ Reviewed by Sam Weinig.
+
+ Instead of throwing a type error when no callback is provided, we pass a null callback.
+
+ Test: http/tests/notifications/legacy/request-no-callback.html
+
+ * bindings/js/JSDesktopNotificationsCustom.cpp:
+ (WebCore::JSNotificationCenter::requestPermission):
+
2012-10-12 Anders Carlsson <[email protected]>
Move QDataStream functions into HistoryItemQt.cpp
Modified: trunk/Source/WebCore/bindings/js/JSDesktopNotificationsCustom.cpp (131279 => 131280)
--- trunk/Source/WebCore/bindings/js/JSDesktopNotificationsCustom.cpp 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/Source/WebCore/bindings/js/JSDesktopNotificationsCustom.cpp 2012-10-14 23:50:30 UTC (rev 131280)
@@ -58,10 +58,14 @@
if (context->isWorkerContext())
return throwSyntaxError(exec);
- if (!exec->argument(0).isObject())
- return throwTypeError(exec);
-
- impl()->requestPermission(JSVoidCallback::create(exec->argument(0).getObject(), toJSDOMGlobalObject(static_cast<Document*>(context), exec)));
+ // If a callback function is provided as first argument, convert to a VoidCallback.
+ RefPtr<JSVoidCallback> callback;
+ if (exec->argument(0).isObject()) {
+ callback = JSVoidCallback::create(exec->argument(0).getObject(), toJSDOMGlobalObject(static_cast<Document*>(context), exec));
+ if (exec->hadException())
+ return jsUndefined();
+ }
+ impl()->requestPermission(callback.release());
return jsUndefined();
}
Modified: trunk/Source/WebKit/mac/ChangeLog (131279 => 131280)
--- trunk/Source/WebKit/mac/ChangeLog 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/Source/WebKit/mac/ChangeLog 2012-10-14 23:50:30 UTC (rev 131280)
@@ -1,3 +1,20 @@
+2012-10-14 Jon Lee <[email protected]>
+
+ Allow notification origin permission request when no js callback is provided
+ https://bugs.webkit.org/show_bug.cgi?id=63615
+ <rdar://problem/11059590>
+
+ Reviewed by Sam Weinig.
+
+ Introduce a boolean to determine whether the request was using the legacy or standard API. This way,
+ we do not fall through to calling the standard API's callback if the legacy API's callback is null.
+
+ * WebCoreSupport/WebNotificationClient.mm:
+ (WebCore):
+ (-[WebNotificationPolicyListener initWithVoidCallback:]):
+ (-[WebNotificationPolicyListener allow]):
+ (-[WebNotificationPolicyListener deny]):
+
2012-10-14 Sam Weinig <[email protected]>
Make UserScript and UserStyleSheet value objects that are copyable
Modified: trunk/Source/WebKit/mac/WebCoreSupport/WebNotificationClient.mm (131279 => 131280)
--- trunk/Source/WebKit/mac/WebCoreSupport/WebNotificationClient.mm 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/Source/WebKit/mac/WebCoreSupport/WebNotificationClient.mm 2012-10-14 23:50:30 UTC (rev 131280)
@@ -54,6 +54,7 @@
#endif
#if ENABLE(LEGACY_NOTIFICATIONS)
RefPtr<VoidCallback> _voidCallback;
+ bool _isLegacyRequest;
#endif
}
#if ENABLE(NOTIFICATIONS)
@@ -252,7 +253,7 @@
if (!(self = [super init]))
return nil;
- ASSERT(callback);
+ _isLegacyRequest = true;
_voidCallback = callback;
return self;
}
@@ -261,8 +262,9 @@
- (void)allow
{
#if ENABLE(LEGACY_NOTIFICATIONS)
- if (_voidCallback) {
- _voidCallback->handleEvent();
+ if (_isLegacyRequest) {
+ if (_voidCallback)
+ _voidCallback->handleEvent();
return;
}
#endif
@@ -274,8 +276,9 @@
- (void)deny
{
#if ENABLE(LEGACY_NOTIFICATIONS)
- if (_voidCallback) {
- _voidCallback->handleEvent();
+ if (_isLegacyRequest) {
+ if (_voidCallback)
+ _voidCallback->handleEvent();
return;
}
#endif
Modified: trunk/Source/WebKit2/ChangeLog (131279 => 131280)
--- trunk/Source/WebKit2/ChangeLog 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/Source/WebKit2/ChangeLog 2012-10-14 23:50:30 UTC (rev 131280)
@@ -1,3 +1,20 @@
+2012-10-14 Jon Lee <[email protected]>
+
+ Allow notification origin permission request when no js callback is provided
+ https://bugs.webkit.org/show_bug.cgi?id=63615
+ <rdar://problem/11059590>
+
+ Reviewed by Sam Weinig.
+
+ Null checks already exist for both standard and legacy API callbacks, so no changes are needed here
+ like there are in WebKit 1. The checks existed because the callbacks are held in a hash map used to keep
+ track of pending requests.
+
+ Also, add a check for a null callback when short circuiting.
+
+ * WebProcess/Notifications/NotificationPermissionRequestManager.cpp:
+ (WebKit::NotificationPermissionRequestManager::startRequest):
+
2012-10-14 Anders Carlsson <[email protected]>
Move QDataStream functions into HistoryItemQt.cpp
Modified: trunk/Source/WebKit2/WebProcess/Notifications/NotificationPermissionRequestManager.cpp (131279 => 131280)
--- trunk/Source/WebKit2/WebProcess/Notifications/NotificationPermissionRequestManager.cpp 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/Source/WebKit2/WebProcess/Notifications/NotificationPermissionRequestManager.cpp 2012-10-14 23:50:30 UTC (rev 131280)
@@ -84,7 +84,8 @@
{
NotificationClient::Permission permission = permissionLevel(origin);
if (permission != NotificationClient::PermissionNotAllowed) {
- callback->handleEvent();
+ if (callback)
+ callback->handleEvent();
return;
}
Modified: trunk/Tools/ChangeLog (131279 => 131280)
--- trunk/Tools/ChangeLog 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/Tools/ChangeLog 2012-10-14 23:50:30 UTC (rev 131280)
@@ -1,3 +1,21 @@
+2012-10-14 Jon Lee <[email protected]>
+
+ Allow notification origin permission request when no js callback is provided
+ https://bugs.webkit.org/show_bug.cgi?id=63615
+ <rdar://problem/11059590>
+
+ Reviewed by Sam Weinig.
+
+ Teach DRT to look at the existing entries in the permission hash map when permission is requested.
+
+ * DumpRenderTree/mac/MockWebNotificationProvider.h: Expose policyForOrigin.
+ * DumpRenderTree/mac/MockWebNotificationProvider.mm:
+ (-[MockWebNotificationProvider setWebNotificationOrigin:permission:]):
+ * DumpRenderTree/mac/UIDelegate.mm:
+ (-[UIDelegate webView:decidePolicyForNotificationRequestFromOrigin:listener:]): Look at whether a
+ policy for the origin already exists. If so, accept or deny the request as appropriate. Otherwise,
+ accept by default.
+
2012-10-13 Zan Dobersek <[email protected]>
[TestResultServer] Unit tests require an update after r131239
Modified: trunk/Tools/DumpRenderTree/mac/MockWebNotificationProvider.h (131279 => 131280)
--- trunk/Tools/DumpRenderTree/mac/MockWebNotificationProvider.h 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/Tools/DumpRenderTree/mac/MockWebNotificationProvider.h 2012-10-14 23:50:30 UTC (rev 131280)
@@ -48,7 +48,8 @@
+ (MockWebNotificationProvider *)shared;
- (void)simulateWebNotificationClick:(uint64_t)notificationID;
-- (void)setWebNotificationOrigin:(NSString*)origin permission:(BOOL)allowed;
+- (void)setWebNotificationOrigin:(NSString *)origin permission:(BOOL)allowed;
+- (WebNotificationPermission)policyForOrigin:(WebSecurityOrigin *)origin;
- (void)removeAllWebNotificationPermissions;
- (void)reset;
Modified: trunk/Tools/DumpRenderTree/mac/MockWebNotificationProvider.mm (131279 => 131280)
--- trunk/Tools/DumpRenderTree/mac/MockWebNotificationProvider.mm 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/Tools/DumpRenderTree/mac/MockWebNotificationProvider.mm 2012-10-14 23:50:30 UTC (rev 131280)
@@ -129,7 +129,7 @@
return WebNotificationPermissionDenied;
}
-- (void)setWebNotificationOrigin:(NSString*)origin permission:(BOOL)allowed
+- (void)setWebNotificationOrigin:(NSString *)origin permission:(BOOL)allowed
{
[_permissions.get() setObject:[NSNumber numberWithBool:allowed] forKey:origin];
}
Modified: trunk/Tools/DumpRenderTree/mac/UIDelegate.mm (131279 => 131280)
--- trunk/Tools/DumpRenderTree/mac/UIDelegate.mm 2012-10-14 23:39:58 UTC (rev 131279)
+++ trunk/Tools/DumpRenderTree/mac/UIDelegate.mm 2012-10-14 23:50:30 UTC (rev 131280)
@@ -293,8 +293,19 @@
- (void)webView:(WebView *)webView decidePolicyForNotificationRequestFromOrigin:(WebSecurityOrigin *)origin listener:(id<WebAllowDenyPolicyListener>)listener
{
- [(MockWebNotificationProvider *)[webView _notificationProvider] setWebNotificationOrigin:[origin stringValue] permission:YES];
- [listener allow];
+ MockWebNotificationProvider *provider = (MockWebNotificationProvider *)[webView _notificationProvider];
+ switch ([provider policyForOrigin:origin]) {
+ case WebNotificationPermissionAllowed:
+ [listener allow];
+ break;
+ case WebNotificationPermissionDenied:
+ [listener deny];
+ break;
+ case WebNotificationPermissionNotAllowed:
+ [provider setWebNotificationOrigin:[origin stringValue] permission:YES];
+ [listener allow];
+ break;
+ }
}
- (void)dealloc
