-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Cherry-pick e938617. rdar://problem/105112595
Cherry-pick 263868@main (e938617). rdar://105112595 The Document object is leaked on some pages using media (like YouTube.com) https://bugs.webkit.org/show_bug.cgi?id=251835 rdar://105112595 Reviewed by Chris Dumez. Re-land of 263660@main (and 263715@main) fixing crashes due to prematurely garbage collected MediaSessionActionHandler JS wrappers. By default a callback holds a Strong<> reference to the JS Function object. This has the effect of making the callback a GC root. Another option is to annotate the callback with the IsWeakCallback extended attribute which will hold the callback object as a Weak reference and keep it alive via the visitJSFunction mechanism instead of making it a root. In the case of MediaSessionActionHandler the strong reference will prevent an HTMLDocument from being garbage collected even after navigating away and clearing the caches (after a low memory warning, for example). This change adds the IsWeakCallback attribute and the necessary virtual function to the MediaSessionActionHandler base class and makes changes to allow the MediaSession to mark any action handlers that have been added to it. LayoutTests: Add a test to check that action handlers installed by the page are not leaked. Use an iframe to install and exercise the action handlers before the iframe is navigated away and a garbage collection is triggered (repeatedly). If after 500 attempts at GC the document containing the action handlers still exists we consider the document leaked. Also add a test to check that action handlers survive garbage collection and can be called when appropriate. * LayoutTests/media/media-session/actionHandler-lifetime-expected.txt: Added. * LayoutTests/media/media-session/actionHandler-lifetime.html: Added. * LayoutTests/media/media-session/actionHandler-no-document-leak-expected.txt: Added. * LayoutTests/media/media-session/actionHandler-no-document-leak.html: Added. * LayoutTests/media/media-session/resources/media-session-action-handler-document-leak-frame.html: Added. * Source/WebCore/Modules/mediasession/MediaSession.cpp: (WebCore::MediaSession::virtualHasPendingActivity const): (WebCore::MediaSession::setActionHandler): (WebCore::MediaSession::callActionHandler): * Source/WebCore/Modules/mediasession/MediaSession.h: (WebCore::MediaSession::hasActiveActionHandlers const): (WebCore::MediaSession::visitActionHandlers const): * Source/WebCore/Modules/mediasession/MediaSession.idl: * Source/WebCore/Modules/mediasession/MediaSessionActionHandler.h: * Source/WebCore/Modules/mediasession/MediaSessionActionHandler.idl: * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/bindings/js/JSMediaSessionCustom.cpp: Added. (WebCore::JSMediaSession::visitAdditionalChildren): Canonical link: https://commits.webkit.org/263868@main Identifier: 259548.810@safari-7615-branch
- Loading branch information
Showing
9 changed files
with
152 additions
and
8 deletions.
There are no files selected for viewing
13 changes: 13 additions & 0 deletions
13
LayoutTests/media/media-session/actionHandler-lifetime-expected.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| Tests media session action handlers are not prematurely garbage collected. Test passes if it doesn't crash. | ||
|
|
||
| On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". | ||
|
|
||
|
|
||
| PASS () => areObjectsEqual(window.actionDetails, {action: "play"}) is true | ||
| PASS () => areObjectsEqual(window.actionDetails, {action: "pause"}) is true | ||
| PASS () => areObjectsEqual(window.actionDetails, {action: "play"}) is true | ||
| PASS () => areObjectsEqual(window.actionDetails, {action: "pause"}) is true | ||
| PASS successfullyParsed is true | ||
|
|
||
| TEST COMPLETE | ||
|
|
44 changes: 44 additions & 0 deletions
44
LayoutTests/media/media-session/actionHandler-lifetime.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| <!DOCTYPE html> | ||
| <body> | ||
| <script src="../../resources/js-test.js"></script> | ||
| <script> | ||
| description("Tests media session action handlers are not prematurely garbage collected. Test passes if it doesn't crash."); | ||
| jsTestIsAsync = true; | ||
|
|
||
| function runTest() { | ||
| internals.sendMediaSessionAction(navigator.mediaSession, {action: "play"}); | ||
| shouldBeTrue(() => areObjectsEqual(window.actionDetails, {action: "play"})); | ||
|
|
||
| internals.sendMediaSessionAction(navigator.mediaSession, {action: "pause"}); | ||
| shouldBeTrue(() => areObjectsEqual(window.actionDetails, {action: "pause"})); | ||
| } | ||
|
|
||
| function forceGCAndRunTest() { | ||
| gc(); | ||
| requestAnimationFrame(() => { | ||
| runTest(); | ||
| finishJSTest(); | ||
| }); | ||
| } | ||
|
|
||
| onload = () => { | ||
| if (!window.internals) { | ||
| testFailed("Test requires internals."); | ||
| finishJSTest(); | ||
| return; | ||
| } | ||
|
|
||
| function callback(actionDetails) { | ||
| window.actionDetails = actionDetails; | ||
| }; | ||
|
|
||
| let actions = ["play", "pause"]; | ||
| for (action of actions) | ||
| navigator.mediaSession.setActionHandler(action, callback); | ||
|
|
||
| runTest(); | ||
| requestAnimationFrame(() => forceGCAndRunTest()); | ||
|
|
||
| }; | ||
| </script> | ||
| </body> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| /* | ||
| * Copyright (C) 2023 Apple, Inc. All rights reserved. | ||
| * | ||
| * Redistribution and use in source and binary forms, with or without | ||
| * modification, are permitted provided that the following conditions | ||
| * are met: | ||
| * 1. Redistributions of source code must retain the above copyright | ||
| * notice, this list of conditions and the following disclaimer. | ||
| * 2. Redistributions in binary form must reproduce the above copyright | ||
| * notice, this list of conditions and the following disclaimer in the | ||
| * documentation and/or other materials provided with the distribution. | ||
| * | ||
| * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY | ||
| * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE | ||
| * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR | ||
| * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR | ||
| * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, | ||
| * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, | ||
| * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR | ||
| * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY | ||
| * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT | ||
| * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE | ||
| * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | ||
| */ | ||
|
|
||
| #if ENABLE(MEDIA_SESSION) | ||
|
|
||
| #include "config.h" | ||
| #include "JSMediaSession.h" | ||
|
|
||
| #include <JavaScriptCore/JSCInlines.h> | ||
|
|
||
| namespace WebCore { | ||
|
|
||
| template <typename Visitor> | ||
| void JSMediaSession::visitAdditionalChildren(Visitor& visitor) | ||
| { | ||
| wrapped().visitActionHandlers(visitor); | ||
| } | ||
|
|
||
| DEFINE_VISIT_ADDITIONAL_CHILDREN(JSMediaSession); | ||
|
|
||
| } | ||
|
|
||
| #endif |