Changeset 131280 in webkit


Ignore:
Timestamp:
Oct 14, 2012 4:50:30 PM (11 years ago)
Author:
jonlee@apple.com
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.
Location:
trunk
Files:
2 added
15 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r131253 r131280  
     12012-10-14  Jon Lee  <jonlee@apple.com>
     2
     3        Allow notification origin permission request when no js callback is provided
     4        https://bugs.webkit.org/show_bug.cgi?id=63615
     5        <rdar://problem/11059590>
     6
     7        Reviewed by Sam Weinig.
     8
     9        * http/tests/notifications/legacy/request-expected.txt:
     10        * http/tests/notifications/legacy/request-no-callback-expected.txt: Added.
     11        * http/tests/notifications/legacy/request-no-callback.html: Calls webkitNotifications.requestPermission()
     12        with no callback, with default and denied permission. For WebKit2, if the permission is not default, it
     13        will short circuit instead of creating a pending request.
     14        * http/tests/notifications/legacy/request.html: The test mistakenly uses the standard API instead of
     15        the legacy API. The results don't change, but we make sure that we call
     16        webkitNotifications.requestPermission() with a callback to test full coverage of the legacy API. We also
     17        expand the test to cover both default and denied permissions.
     18        * http/tests/notifications/request-expected.txt:
     19        * http/tests/notifications/request.html: Expand test to cover both default and denied permissions.
     20
    1212012-10-12  Adam Barth  <abarth@webkit.org>
    222
  • trunk/LayoutTests/http/tests/notifications/legacy/request-expected.txt

    r127299 r131280  
    44
    55
     6Requesting permission with default permission
    67PASS window.webkitNotifications.checkPermission() is 1
    78PASS window.webkitNotifications.checkPermission() is 0
     9Requesting permission with non-default permission
     10PASS window.webkitNotifications.checkPermission() is 2
    811PASS successfullyParsed is true
    912
  • trunk/LayoutTests/http/tests/notifications/legacy/request.html

    r127299 r131280  
    55<div id="console"></div>
    66<script>
    7 if (window.testRunner)
    8         testRunner.waitUntilDone();
    9 
    107description("This test checks that a request for permission is made.");
    118
     9function step1() {
     10    debug("Requesting permission with default permission");
     11    shouldBe("window.webkitNotifications.checkPermission()", "1");
     12    webkitNotifications.requestPermission(function() {
     13        shouldBe("window.webkitNotifications.checkPermission()", "0");
     14        setTimeout(step2, 0);
     15    });
     16}
     17
     18function step2() {
     19    testRunner.denyWebNotificationPermission(testURL);
     20    debug("Requesting permission with non-default permission");
     21    webkitNotifications.requestPermission(function() {
     22        shouldBe("window.webkitNotifications.checkPermission()", "2");
     23        testCompleted();
     24    });
     25}
     26
    1227if (window.testRunner) {
    13         shouldBe("window.webkitNotifications.checkPermission()", "1");
    14         Notification.requestPermission(function() {
    15                 shouldBe("window.webkitNotifications.checkPermission()", "0");
    16                 testCompleted();
    17         });
     28    testRunner.waitUntilDone();
     29    step1();
    1830}
    1931
  • trunk/LayoutTests/http/tests/notifications/request-expected.txt

    r127299 r131280  
    44
    55
     6Requesting permission with default permission
    67PASS Notification.permission is "default"
    7 PASS permission is granted.
     8PASS permission is granted
    89PASS Notification.permission is "granted"
     10Requesting permission with non-default permission
     11PASS permission is denied
     12PASS Notification.permission is "denied"
    913PASS successfullyParsed is true
    1014
  • trunk/LayoutTests/http/tests/notifications/request.html

    r127299 r131280  
    55<div id="console"></div>
    66<script>
    7 if (window.testRunner)
    8         testRunner.waitUntilDone();
    9 
    107description("This test checks that a request for permission is made.");
    118
     9function step1() {
     10    debug("Requesting permission with default permission");
     11    shouldBeEqualToString("Notification.permission", "default");
     12    Notification.requestPermission(function(permission) {
     13        if (permission == 'granted')
     14            testPassed("permission is granted");
     15        else
     16            testFailed("permission should be granted, but is " + permission);
     17        shouldBeEqualToString("Notification.permission", "granted");
     18        setTimeout(step2, 0);
     19    });
     20}
     21
     22function step2() {
     23    debug("Requesting permission with non-default permission");
     24    testRunner.denyWebNotificationPermission(testURL);
     25    Notification.requestPermission(function(permission) {
     26        if (permission == 'denied')
     27            testPassed("permission is denied");
     28        else
     29            testFailed("permission should be denied, but is " + permission);
     30        shouldBeEqualToString("Notification.permission", "denied");
     31        testCompleted();
     32    });
     33}
     34
    1235if (window.testRunner) {
    13         shouldBeEqualToString("Notification.permission", "default");
    14         Notification.requestPermission(function(permission) {
    15                 if (permission == 'granted')
    16                         testPassed("permission is granted.");
    17                 else
    18                         testFailed("permission should be granted, but is " + permission + ".");
    19                 shouldBeEqualToString("Notification.permission", "granted");
    20                 testCompleted();
    21         });
     36    testRunner.waitUntilDone();
     37    step1();
    2238}
    2339
  • trunk/Source/WebCore/ChangeLog

    r131279 r131280  
     12012-10-14  Jon Lee  <jonlee@apple.com>
     2
     3        Allow notification origin permission request when no js callback is provided
     4        https://bugs.webkit.org/show_bug.cgi?id=63615
     5        <rdar://problem/11059590>
     6
     7        Reviewed by Sam Weinig.
     8
     9        Instead of throwing a type error when no callback is provided, we pass a null callback.
     10
     11        Test: http/tests/notifications/legacy/request-no-callback.html
     12
     13        * bindings/js/JSDesktopNotificationsCustom.cpp:
     14        (WebCore::JSNotificationCenter::requestPermission):
     15
    1162012-10-12  Anders Carlsson  <andersca@apple.com>
    217
  • trunk/Source/WebCore/bindings/js/JSDesktopNotificationsCustom.cpp

    r125745 r131280  
    5959        return throwSyntaxError(exec);
    6060
    61     if (!exec->argument(0).isObject())
    62         return throwTypeError(exec);
    63 
    64     impl()->requestPermission(JSVoidCallback::create(exec->argument(0).getObject(), toJSDOMGlobalObject(static_cast<Document*>(context), exec)));
     61    // If a callback function is provided as first argument, convert to a VoidCallback.
     62    RefPtr<JSVoidCallback> callback;
     63    if (exec->argument(0).isObject()) {
     64        callback = JSVoidCallback::create(exec->argument(0).getObject(), toJSDOMGlobalObject(static_cast<Document*>(context), exec));
     65        if (exec->hadException())
     66            return jsUndefined();
     67    }
     68    impl()->requestPermission(callback.release());
    6569    return jsUndefined();
    6670}
  • trunk/Source/WebKit/mac/ChangeLog

    r131275 r131280  
     12012-10-14  Jon Lee  <jonlee@apple.com>
     2
     3        Allow notification origin permission request when no js callback is provided
     4        https://bugs.webkit.org/show_bug.cgi?id=63615
     5        <rdar://problem/11059590>
     6
     7        Reviewed by Sam Weinig.
     8
     9        Introduce a boolean to determine whether the request was using the legacy or standard API. This way,
     10        we do not fall through to calling the standard API's callback if the legacy API's callback is null.
     11
     12        * WebCoreSupport/WebNotificationClient.mm:
     13        (WebCore):
     14        (-[WebNotificationPolicyListener initWithVoidCallback:]):
     15        (-[WebNotificationPolicyListener allow]):
     16        (-[WebNotificationPolicyListener deny]):
     17
    1182012-10-14  Sam Weinig  <sam@webkit.org>
    219
  • trunk/Source/WebKit/mac/WebCoreSupport/WebNotificationClient.mm

    r130612 r131280  
    5555#if ENABLE(LEGACY_NOTIFICATIONS)
    5656    RefPtr<VoidCallback> _voidCallback;
     57    bool _isLegacyRequest;
    5758#endif
    5859}
     
    253254        return nil;
    254255
    255     ASSERT(callback);
     256    _isLegacyRequest = true;
    256257    _voidCallback = callback;
    257258    return self;
     
    262263{
    263264#if ENABLE(LEGACY_NOTIFICATIONS)
    264     if (_voidCallback) {
    265         _voidCallback->handleEvent();
     265    if (_isLegacyRequest) {
     266        if (_voidCallback)
     267            _voidCallback->handleEvent();
    266268        return;
    267269    }
     
    275277{
    276278#if ENABLE(LEGACY_NOTIFICATIONS)
    277     if (_voidCallback) {
    278         _voidCallback->handleEvent();
     279    if (_isLegacyRequest) {
     280        if (_voidCallback)
     281            _voidCallback->handleEvent();
    279282        return;
    280283    }
  • trunk/Source/WebKit2/ChangeLog

    r131279 r131280  
     12012-10-14  Jon Lee  <jonlee@apple.com>
     2
     3        Allow notification origin permission request when no js callback is provided
     4        https://bugs.webkit.org/show_bug.cgi?id=63615
     5        <rdar://problem/11059590>
     6
     7        Reviewed by Sam Weinig.
     8
     9        Null checks already exist for both standard and legacy API callbacks, so no changes are needed here
     10        like there are in WebKit 1. The checks existed because the callbacks are held in a hash map used to keep
     11        track of pending requests.
     12
     13        Also, add a check for a null callback when short circuiting.
     14
     15        * WebProcess/Notifications/NotificationPermissionRequestManager.cpp:
     16        (WebKit::NotificationPermissionRequestManager::startRequest):
     17
    1182012-10-14  Anders Carlsson  <andersca@apple.com>
    219
  • trunk/Source/WebKit2/WebProcess/Notifications/NotificationPermissionRequestManager.cpp

    r127299 r131280  
    8585    NotificationClient::Permission permission = permissionLevel(origin);
    8686    if (permission != NotificationClient::PermissionNotAllowed) {
    87         callback->handleEvent();
     87        if (callback)
     88            callback->handleEvent();
    8889        return;
    8990    }
  • trunk/Tools/ChangeLog

    r131259 r131280  
     12012-10-14  Jon Lee  <jonlee@apple.com>
     2
     3        Allow notification origin permission request when no js callback is provided
     4        https://bugs.webkit.org/show_bug.cgi?id=63615
     5        <rdar://problem/11059590>
     6
     7        Reviewed by Sam Weinig.
     8
     9        Teach DRT to look at the existing entries in the permission hash map when permission is requested.
     10
     11        * DumpRenderTree/mac/MockWebNotificationProvider.h: Expose policyForOrigin.
     12        * DumpRenderTree/mac/MockWebNotificationProvider.mm:
     13        (-[MockWebNotificationProvider setWebNotificationOrigin:permission:]):
     14        * DumpRenderTree/mac/UIDelegate.mm:
     15        (-[UIDelegate webView:decidePolicyForNotificationRequestFromOrigin:listener:]): Look at whether a
     16        policy for the origin already exists. If so, accept or deny the request as appropriate. Otherwise,
     17        accept by default.
     18
    1192012-10-13  Zan Dobersek  <zandobersek@gmail.com>
    220
  • trunk/Tools/DumpRenderTree/mac/MockWebNotificationProvider.h

    r127042 r131280  
    4949
    5050- (void)simulateWebNotificationClick:(uint64_t)notificationID;
    51 - (void)setWebNotificationOrigin:(NSString*)origin permission:(BOOL)allowed;
     51- (void)setWebNotificationOrigin:(NSString *)origin permission:(BOOL)allowed;
     52- (WebNotificationPermission)policyForOrigin:(WebSecurityOrigin *)origin;
    5253- (void)removeAllWebNotificationPermissions;
    5354
  • trunk/Tools/DumpRenderTree/mac/MockWebNotificationProvider.mm

    r130612 r131280  
    130130}
    131131
    132 - (void)setWebNotificationOrigin:(NSString*)origin permission:(BOOL)allowed
     132- (void)setWebNotificationOrigin:(NSString *)origin permission:(BOOL)allowed
    133133{
    134134    [_permissions.get() setObject:[NSNumber numberWithBool:allowed] forKey:origin];
  • trunk/Tools/DumpRenderTree/mac/UIDelegate.mm

    r129595 r131280  
    294294- (void)webView:(WebView *)webView decidePolicyForNotificationRequestFromOrigin:(WebSecurityOrigin *)origin listener:(id<WebAllowDenyPolicyListener>)listener
    295295{
    296     [(MockWebNotificationProvider *)[webView _notificationProvider] setWebNotificationOrigin:[origin stringValue] permission:YES];
    297     [listener allow];
     296    MockWebNotificationProvider *provider = (MockWebNotificationProvider *)[webView _notificationProvider];
     297    switch ([provider policyForOrigin:origin]) {
     298    case WebNotificationPermissionAllowed:
     299        [listener allow];
     300        break;
     301    case WebNotificationPermissionDenied:
     302        [listener deny];
     303        break;
     304    case WebNotificationPermissionNotAllowed:
     305        [provider setWebNotificationOrigin:[origin stringValue] permission:YES];
     306        [listener allow];
     307        break;
     308    }
    298309}
    299310
Note: See TracChangeset for help on using the changeset viewer.