Skip to content

Commit 9948a5b

Browse files
cola119eric-carlson
authored andcommitted
Ensuring initialization of CaptionUserPreferencesTestingModeToken before setting caption mode
https://bugs.webkit.org/show_bug.cgi?id=262680 Reviewed by Eric Carlson. The `captionDisplayMode` option was introduced in https://bugs.webkit.org/show_bug.cgi?id=261460 (PR: WebKit#17689). However, when I attempted to import WPT for webvtt/parsing with `captionDisplayMode` setting enabled, I found that the tests were failing intermittently on ios-wk2 (example: WebKit#17852 (comment)). The flakiness of these tests appears to be due to the fact that the `CaptionUserPreferencesTestingModeToken` instance may not always be initialized before the caption mode is set (https://github.com/WebKit/WebKit/blob/659f85098cf4322282a775cbd51eb58a4ed34022/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp#L583). This instance is usually initialized on the `Internals` class's constructor during the WebKitTestRunner setup (https://github.com/WebKit/WebKit/blob/659f85098cf4322282a775cbd51eb58a4ed34022/Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp#L888). In most cases, the `CaptionUserPreferencesTestingModeToken` instance is created in the `Internals` class constructor before `WKBundlePageSetCaptionDisplayMode` is called, but there are instances where it is not, particularly when tests are run concurrently. This leads to the tests failing. To address this issue, the WebKitTestRunner needs to ensure that the `CaptionUserPreferencesTestingModeToken` instance is always initialized before `WKBundlePageSetCaptionDisplayMode` is called, thus eliminating the flakiness of the tests. * Source/WebKit/Shared/API/APICaptionUserPreferencesTestingModeToken.h: Added. (API::CaptionUserPreferencesTestingModeToken::create): (API::CaptionUserPreferencesTestingModeToken::CaptionUserPreferencesTestingModeToken): * Source/WebKit/Shared/API/APIObject.h: * Source/WebKit/Shared/API/c/WKBase.h: * Source/WebKit/UIProcess/API/C/WKAPICast.h: * Source/WebKit/WebKit.xcodeproj/project.pbxproj: * Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp: (WKBundlePageSetCaptionDisplayMode): (WKBundlePageCreateCaptionUserPreferencesTestingModeToken): (WKBundleSetCaptionDisplayMode): Deleted. * Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePagePrivate.h: * Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp: (WTR::InjectedBundle::beginTesting): * Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.h: Canonical link: https://commits.webkit.org/269455@main
1 parent afcea26 commit 9948a5b

File tree

9 files changed

+78
-4
lines changed

9 files changed

+78
-4
lines changed
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright (C) 2010-2023 Apple Inc. All rights reserved.
3+
*
4+
* Redistribution and use in source and binary forms, with or without
5+
* modification, are permitted provided that the following conditions
6+
* are met:
7+
* 1. Redistributions of source code must retain the above copyright
8+
* notice, this list of conditions and the following disclaimer.
9+
* 2. Redistributions in binary form must reproduce the above copyright
10+
* notice, this list of conditions and the following disclaimer in the
11+
* documentation and/or other materials provided with the distribution.
12+
*
13+
* THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
14+
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
15+
* THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
16+
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
17+
* BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
18+
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
19+
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
20+
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
21+
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
22+
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
23+
* THE POSSIBILITY OF SUCH DAMAGE.
24+
*/
25+
26+
#pragma once
27+
28+
#include "APIObject.h"
29+
#include <WebCore/CaptionUserPreferences.h>
30+
#include <wtf/Ref.h>
31+
32+
namespace API {
33+
34+
class CaptionUserPreferencesTestingModeToken : public API::ObjectImpl<API::Object::Type::CaptionUserPreferencesTestingModeToken> {
35+
public:
36+
static Ref<CaptionUserPreferencesTestingModeToken> create(WebCore::CaptionUserPreferences& preferences)
37+
{
38+
return adoptRef(*new CaptionUserPreferencesTestingModeToken(*new WebCore::CaptionUserPreferencesTestingModeToken(preferences)));
39+
}
40+
private:
41+
CaptionUserPreferencesTestingModeToken(WebCore::CaptionUserPreferencesTestingModeToken& token)
42+
: m_token(token)
43+
{
44+
}
45+
46+
WebCore::CaptionUserPreferencesTestingModeToken m_token;
47+
};
48+
49+
}

Source/WebKit/Shared/API/APIObject.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ class Object
5454
Array,
5555
AuthenticationChallenge,
5656
AuthenticationDecisionListener,
57+
CaptionUserPreferencesTestingModeToken,
5758
CertificateInfo,
5859
Connection,
5960
ContextMenuItem,
@@ -318,6 +319,7 @@ template<> struct EnumTraits<API::Object::Type> {
318319
API::Object::Type::Array,
319320
API::Object::Type::AuthenticationChallenge,
320321
API::Object::Type::AuthenticationDecisionListener,
322+
API::Object::Type::CaptionUserPreferencesTestingModeToken,
321323
API::Object::Type::CertificateInfo,
322324
API::Object::Type::Connection,
323325
API::Object::Type::ContextMenuItem,

Source/WebKit/Shared/API/c/WKBase.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ typedef const struct OpaqueWKAuthenticationDecisionListener* WKAuthenticationDec
8484
typedef const struct OpaqueWKBackForwardList* WKBackForwardListRef;
8585
typedef const struct OpaqueWKBackForwardListItem* WKBackForwardListItemRef;
8686
typedef const struct OpaqueWKResourceCacheManager* WKResourceCacheManagerRef;
87+
typedef const struct OpaqueWKCaptionUserPreferencesTestingModeToken* WKCaptionUserPreferencesTestingModeTokenRef;
8788
typedef const struct OpaqueWKColorPickerResultListener* WKColorPickerResultListenerRef;
8889
typedef const struct OpaqueWKContext* WKContextRef;
8990
typedef const struct OpaqueWKContextConfiguration* WKContextConfigurationRef;

Source/WebKit/UIProcess/API/C/WKAPICast.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
#include <WebCore/Settings.h>
5050

5151
namespace API {
52+
class CaptionUserPreferencesTestingModeToken;
5253
class ContentRuleList;
5354
class ContentRuleListStore;
5455
class Feature;
@@ -115,6 +116,7 @@ WK_ADD_API_MAPPING(WKAuthenticationDecisionListenerRef, AuthenticationDecisionLi
115116
WK_ADD_API_MAPPING(WKBackForwardListItemRef, WebBackForwardListItem)
116117
WK_ADD_API_MAPPING(WKBackForwardListRef, WebBackForwardList)
117118
WK_ADD_API_MAPPING(WKBundleHitTestResultMediaType, BundleHitTestResultMediaType)
119+
WK_ADD_API_MAPPING(WKCaptionUserPreferencesTestingModeTokenRef, API::CaptionUserPreferencesTestingModeToken)
118120
WK_ADD_API_MAPPING(WKColorPickerResultListenerRef, WebColorPickerResultListenerProxy)
119121
WK_ADD_API_MAPPING(WKContextRef, WebProcessPool)
120122
WK_ADD_API_MAPPING(WKContextConfigurationRef, API::ProcessPoolConfiguration)

Source/WebKit/WebKit.xcodeproj/project.pbxproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6974,6 +6974,7 @@
69746974
C5BCE5DB1C50761D00CDE3FA /* InteractionInformationAtPosition.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = InteractionInformationAtPosition.mm; path = ios/InteractionInformationAtPosition.mm; sourceTree = "<group>"; };
69756975
C5FA1ED118E1062200B3F402 /* WKAirPlayRoutePicker.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = WKAirPlayRoutePicker.h; path = ios/forms/WKAirPlayRoutePicker.h; sourceTree = "<group>"; };
69766976
C5FA1ED218E1062200B3F402 /* WKAirPlayRoutePicker.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = WKAirPlayRoutePicker.mm; path = ios/forms/WKAirPlayRoutePicker.mm; sourceTree = "<group>"; };
6977+
C68BA2622ADBA6EC00770791 /* APICaptionUserPreferencesTestingModeToken.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = APICaptionUserPreferencesTestingModeToken.h; sourceTree = "<group>"; };
69776978
C6A4CA092252899800169289 /* WKBundlePageMac.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = WKBundlePageMac.h; sourceTree = "<group>"; };
69786979
C6A4CA0A2252899800169289 /* WKBundlePageMac.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = WKBundlePageMac.mm; sourceTree = "<group>"; };
69796980
CA05397823EE324400A553DC /* ContentAsStringIncludesChildFrames.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ContentAsStringIncludesChildFrames.h; sourceTree = "<group>"; };
@@ -13509,6 +13510,7 @@
1350913510
37DFA6FE1810BB2D001F4A9F /* Cocoa */,
1351013511
BC64696D11DBE603006455B0 /* APIArray.cpp */,
1351113512
BC64696E11DBE603006455B0 /* APIArray.h */,
13513+
C68BA2622ADBA6EC00770791 /* APICaptionUserPreferencesTestingModeToken.h */,
1351213514
1A3DD205125E5A2F004515E6 /* APIClient.h */,
1351313515
1AAB037B185F99D800EDF501 /* APIData.cpp */,
1351413516
51578B821209ECEF00A37C4A /* APIData.h */,

Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include "WKBundlePagePrivate.h"
2929

3030
#include "APIArray.h"
31+
#include "APICaptionUserPreferencesTestingModeToken.h"
3132
#include "APIDictionary.h"
3233
#include "APIFrameHandle.h"
3334
#include "APIInjectedBundlePageContextMenuClient.h"
@@ -849,7 +850,7 @@ void WKBundlePageClearApplicationCache(WKBundlePageRef page)
849850
WebKit::toImpl(page)->corePage()->applicationCacheStorage().deleteAllEntries();
850851
}
851852

852-
void WKBundleSetCaptionDisplayMode(WKBundlePageRef page, WKStringRef mode)
853+
void WKBundlePageSetCaptionDisplayMode(WKBundlePageRef page, WKStringRef mode)
853854
{
854855
#if ENABLE(VIDEO)
855856
auto& captionPreferences = WebKit::toImpl(page)->corePage()->group().ensureCaptionPreferences();
@@ -858,7 +859,17 @@ void WKBundleSetCaptionDisplayMode(WKBundlePageRef page, WKStringRef mode)
858859
captionPreferences.setCaptionDisplayMode(displayMode.value());
859860
#else
860861
UNUSED_PARAM(page);
861-
UNUSED_PARAM(modeString);
862+
UNUSED_PARAM(mode);
863+
#endif
864+
}
865+
866+
WKCaptionUserPreferencesTestingModeTokenRef WKBundlePageCreateCaptionUserPreferencesTestingModeToken(WKBundlePageRef page)
867+
{
868+
#if ENABLE(VIDEO)
869+
auto& captionPreferences = WebKit::toImpl(page)->corePage()->group().ensureCaptionPreferences();
870+
return WebKit::toAPI(&API::CaptionUserPreferencesTestingModeToken::create(captionPreferences).leakRef());
871+
#else
872+
UNUSED_PARAM(page);
862873
#endif
863874
}
864875

Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePagePrivate.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,8 @@ WK_EXPORT void WKBundlePageSetApplicationCacheOriginQuota(WKBundlePageRef page,
130130
WK_EXPORT void WKBundlePageResetApplicationCacheOriginQuota(WKBundlePageRef page, WKStringRef origin);
131131
WK_EXPORT WKArrayRef WKBundlePageCopyOriginsWithApplicationCache(WKBundlePageRef page);
132132

133-
WK_EXPORT void WKBundleSetCaptionDisplayMode(WKBundlePageRef page, WKStringRef mode);
133+
WK_EXPORT void WKBundlePageSetCaptionDisplayMode(WKBundlePageRef page, WKStringRef mode);
134+
WK_EXPORT WKCaptionUserPreferencesTestingModeTokenRef WKBundlePageCreateCaptionUserPreferencesTestingModeToken(WKBundlePageRef page);
134135

135136
enum {
136137
kWKEventThrottlingBehaviorResponsive = 0,

Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,10 @@ void InjectedBundle::beginTesting(WKDictionaryRef settings, BegingTestingMode te
552552
m_accessibilityController->setIsolatedTreeMode(m_accessibilityIsolatedTreeMode);
553553
#endif
554554
#endif
555+
#if ENABLE(VIDEO)
556+
if (!m_captionUserPreferencesTestingModeToken)
557+
m_captionUserPreferencesTestingModeToken = WKBundlePageCreateCaptionUserPreferencesTestingModeToken(page()->page());
558+
#endif
555559

556560
#if PLATFORM(IOS_FAMILY)
557561
WKBundlePageSetUseTestingViewportConfiguration(page()->page(), !booleanValue(settings, "UseFlexibleViewport"));
@@ -580,7 +584,7 @@ void InjectedBundle::beginTesting(WKDictionaryRef settings, BegingTestingMode te
580584
clearResourceLoadStatistics();
581585

582586
#if ENABLE(VIDEO)
583-
WKBundleSetCaptionDisplayMode(page()->page(), stringValue(settings, "CaptionDisplayMode"));
587+
WKBundlePageSetCaptionDisplayMode(page()->page(), stringValue(settings, "CaptionDisplayMode"));
584588
#endif
585589
// [WK2] REGRESSION(r128623): It made layout tests extremely slow
586590
// https://bugs.webkit.org/show_bug.cgi?id=96862

Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,8 @@ class InjectedBundle {
221221
Vector<String> m_allowedHosts;
222222

223223
size_t m_userScriptInjectedCount { 0 };
224+
225+
WKRetainPtr<WKCaptionUserPreferencesTestingModeTokenRef> m_captionUserPreferencesTestingModeToken;
224226
};
225227

226228
void postMessage(const char* name);

0 commit comments

Comments
 (0)