Bug 188329 - navigator.sendBeacon does not work in pagehide callbacks
Summary: navigator.sendBeacon does not work in pagehide callbacks
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Safari Technology Preview
Hardware: Macintosh macOS 10.13
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-04 09:57 PDT by Philip Walton
Modified: 2019-04-23 09:11 PDT (History)
11 users (show)

See Also:


Attachments
Patch (6.92 KB, patch)
2018-08-07 16:03 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Walton 2018-08-04 09:57:27 PDT
Initial support for navigator.sendBeacon was added here: https://bugs.webkit.org/show_bug.cgi?id=175007,
but if you try to invoke navigator.sendBeacon() in a pagehide event listener, it fails and you get a warning in the console:

> Beacon API cannot load ... due to access control checks

Note that this is the same error you'd get if you tried to do a fetch() or xhr in a pagehide event listener, so I suspect the same rules are being enforced. However, beacons should not be subject to the same restrictions since they're designed to not block or require waiting for a response. (Note, this is also true of any fetch() with the keepalive flag set to true and should be considered when webkit implements keepalive: https://bugs.webkit.org/show_bug.cgi?id=168865).

All other browsers that support the Beacon API or fetch keepalive allow these requests to be sent in the pagehide event listener.
Comment 1 Chris Dumez 2018-08-06 08:47:41 PDT
Likely caused:
    // Prevent new loads if we are in the PageCache or being added to the PageCache.
    // We query the top document because new frames may be created in pagehide event handlers
    // and their pageCacheState will not reflect the fact that they are about to enter page
    // cache.
    if (auto* topDocument = frame.mainFrame().document()) {
        if (topDocument->pageCacheState() != Document::NotInPageCache) {
            RELEASE_LOG_IF_ALLOWED("load: Already in page cache or being added to it (frame = %p)", &frame);
            failBeforeStarting();
            return;
        }
    }

in CachedResource::load(CachedResourceLoader&).

Seems like a bad bug indeed.
Comment 2 Chris Dumez 2018-08-07 16:03:51 PDT
Created attachment 346740 [details]
Patch
Comment 3 WebKit Commit Bot 2018-08-07 18:41:52 PDT
Comment on attachment 346740 [details]
Patch

Clearing flags on attachment: 346740

Committed r234684: <https://trac.webkit.org/changeset/234684>
Comment 4 WebKit Commit Bot 2018-08-07 18:41:54 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Radar WebKit Bug Importer 2018-08-07 18:42:19 PDT
<rdar://problem/43030256>
Comment 6 Martin Kuba 2018-10-17 17:38:22 PDT
I am still seeing this in Safari 12.  Has this patch been included in Safari 12?
Comment 7 Chris Dumez 2018-10-17 18:47:58 PDT
(In reply to martin from comment #6)
> I am still seeing this in Safari 12.  Has this patch been included in Safari
> 12?

No, the change missed the cut-off date for iOS 12. It will be fixed in a later release but cannot comment on when.
Comment 8 Ryosuke Niwa 2018-11-12 16:04:47 PST
You can try out Safari Technology Preview, which has this fix: https://developer.apple.com/safari/technology-preview/
Comment 9 Chris Dumez 2019-04-23 09:11:24 PDT
This fix shipped in iOS 12.2 and macOS 10.14.4.