Bug 194761 – Leak of CFErrorRef objects (1.92 Kbytes) in com.apple.WebKit.WebContent.Development running WebKit layout tests on iOS Simulator

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[David Kilzer (:ddkilzer)]

Leak of CFErrorRef objects (1.92 Kbytes) in com.apple.WebKit.WebContent.Development running WebKit layout tests on iOS Simulator.

NOTE: Requires patch in Bug 193772 to gather leaks for the com.apple.WebKit.WebContent.Development process.

STACK OF 3 INSTANCES OF 'ROOT LEAK: <CFError>':
[thread 0x10d8dc5c0]:
45  libdyld.dylib                         0x10cbadca1 start + 1
44  com.apple.WebKit                      0x10a04906d 0x109f4f000 + 1024109
43  libxpc.dylib                          0x10ceebca7 xpc_main + 143
42  libxpc.dylib                          0x10cee97d2 _xpc_objc_main + 555
41  com.apple.Foundation                  0x109a05200 -[NSRunLoop(NSRunLoop) run] + 76
40  com.apple.Foundation                  0x109a04fe0 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 211
39  com.apple.CoreFoundation              0x10bdbf9f2 CFRunLoopRunSpecific + 626
38  com.apple.CoreFoundation              0x10bdc024f __CFRunLoopRun + 1295
37  com.apple.CoreFoundation              0x10bdc5bcf __CFRunLoopDoSources0 + 255
36  com.apple.CoreFoundation              0x10bdc6391 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
35  JavaScriptCore                        0x27a4e0d12 WTF::RunLoop::performWork(void*) + 34  RunLoopCF.cpp:39
34  JavaScriptCore                        0x27a4e0a84 WTF::RunLoop::performWork() + 228  Function.h:0
33  com.apple.WebKit                      0x109f61c4b IPC::Connection::dispatchOneIncomingMessage() + 181  Connection.cpp:0
32  com.apple.WebKit                      0x109f5e728 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 108  memory:2603
31  com.apple.WebKit                      0x10a27cad1 WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 627  NetworkProcessConnection.cpp:0
30  com.apple.WebKit                      0x10a3629c6 WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::Decoder&) + 762  HandleMessage.h:0
29  com.apple.WebKit                      0x10a282a21 WebKit::WebResourceLoader::didFinishResourceLoad(WebCore::NetworkLoadMetrics const&) + 197  WebResourceLoader.cpp:0
28  com.apple.WebCore                     0x27ca94382 WebCore::SubresourceLoader::didFinishLoading(WebCore::NetworkLoadMetrics const&) + 706  ResourceLoader.h:161
27  com.apple.WebCore                     0x27cac445c WebCore::CachedRawResource::finishLoading(WebCore::SharedBuffer*) + 652  CachedRawResource.cpp:107
26  com.apple.WebCore                     0x27cac67bc WebCore::CachedResource::checkNotify() + 348  CachedResource.cpp:352
25  com.apple.WebCore                     0x27ca35215 WebCore::DocumentLoader::finishedLoading() + 485  RefPtr.h:81
24  com.apple.WebCore                     0x27c6299e9 WebCore::DecodedDataDocumentParser::flush(WebCore::DocumentWriter&) + 89  utility:918
23  com.apple.WebCore                     0x27c92975e WebCore::HTMLDocumentParser::append(WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> >&&) + 1006  DocumentParser.h:70
22  com.apple.WebCore                     0x27c928964 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) + 116  DocumentParser.h:69
21  com.apple.WebCore                     0x27c928fa7 WebCore::HTMLDocumentParser::pumpTokenizerLoop(WebCore::HTMLDocumentParser::SynchronousMode, bool, WebCore::PumpSession&) + 615  HTMLDocumentParser.cpp:254
20  com.apple.WebCore                     0x27c928cdf WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder() + 367  Ref.h:59
19  com.apple.WebCore                     0x27c9362b0 WebCore::HTMLScriptRunner::execute(WTF::Ref<WebCore::ScriptElement, WTF::DumbPtrTraits<WebCore::ScriptElement> >&&, WTF::TextPosition const&) + 48  RefPtr.h:81
18  com.apple.WebCore                     0x27c936422 WebCore::HTMLScriptRunner::runScript(WebCore::ScriptElement&, WTF::TextPosition const&) + 290  ScriptElement.h:61
17  com.apple.WebCore                     0x27c6d03f9 WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport) + 1241  CachedResourceHandle.h:61
16  com.apple.WebCore                     0x27c6d24a5 WebCore::ScriptElement::executeClassicScript(WebCore::ScriptSourceCode const&) + 581  CurrentScriptIncrementer.h:54
15  com.apple.WebCore                     0x27c3f1a4f WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&, WebCore::ExceptionDetails*) + 207  ScriptController.cpp:131
14  com.apple.WebCore                     0x27c3f1bf4 WebCore::JSExecState::profiledEvaluate(JSC::ExecState*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) + 84  JSExecState.h:80
13  JavaScriptCore                        0x27acb68b0 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) + 288  Completion.cpp:137
12  JavaScriptCore                        0x27aa35c3c JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*) + 11324  JITCodeInlines.h:39
11  JavaScriptCore                        0x27a66add9 vmEntryToJavaScript + 200  LowLevelInterpreter64.asm:293
10  JavaScriptCore                        0x27a67a079 llint_entry + 61686  LowLevelInterpreter.asm:879
9                                      0x51026980102d 0x510269801000 + 45
8   JavaScriptCore                        0x27a6a67ef long long JSC::APICallbackFunction::call<JSC::JSCallbackFunction>(JSC::ExecState*) + 495  APICallbackFunction.h:63
7   com.apple.WebKitTestRunner.InjectedBundle        0x2928fcbe9 WTR::JSTestRunner::installFakeHelvetica(OpaqueJSContext const*, OpaqueJSValue*, OpaqueJSValue*, unsigned long, OpaqueJSValue const* const*, OpaqueJSValue const**) + 89  JSTestRunner.cpp:2861
6   com.apple.WebKitTestRunner.InjectedBundle        0x2929084c8 WTR::TestRunner::installFakeHelvetica(OpaqueJSString*) + 26  WKRetainPtr.h:87
5   com.apple.WebKitTestRunner.InjectedBundle        0x2928e2532 WTR::installFakeHelvetica(OpaqueWKString const*) + 120  RetainPtr.h:169
4   com.apple.CoreText                    0x278de02d0 CTFontManagerRegisterFontsForURL + 140
3   com.apple.CoreText                    0x278de0544 CreateErrorForFailureToActOnFontURLs(Action, __CFArray const*, int) + 162
2   com.apple.CoreFoundation              0x10bd74d9a CFErrorCreateWithUserInfoKeysAndValues + 58
1   com.apple.CoreFoundation              0x10bdc730e _CFRuntimeCreateInstance + 446
0   libsystem_malloc.dylib                0x10ce2bd57 malloc_zone_calloc + 139 
====
    24 (1.92K) << TOTAL >>

      8 (656 bytes) ROOT LEAK: <CFError 0x7fd261c063b0> [48]
         7 (608 bytes) _userInfo --> <CFDictionary 0x7fd261c1a860> [64]
            5 (512 bytes) <CFDictionary (Value Storage) 0x7fd261c1aae0> [32]
               3 (416 bytes) <NSArray 0x7fd261c18590> [16]
                  2 (400 bytes) __strong _object --> <NSURL 0x7fd261c2c1c0> [96]
                     1 (304 bytes) _clients --> <CFString 0x7fd261c2bd00> [304]
               1 (64 bytes) <CFString 0x7fd261c2bfe0> [64]
            1 (32 bytes) <CFDictionary (Key Storage) 0x7fd261c19000> [32]

      8 (656 bytes) ROOT LEAK: <CFError 0x7fd261d03ba0> [48]
         7 (608 bytes) _userInfo --> <CFDictionary 0x7fd261d4e670> [64]
            5 (512 bytes) <CFDictionary (Value Storage) 0x7fd261d430a0> [32]
               3 (416 bytes) <NSArray 0x7fd261d1b1b0> [16]
                  2 (400 bytes) __strong _object --> <NSURL 0x7fd261d07510> [96]
                     1 (304 bytes) _clients --> <CFString 0x7fd261d47950> [304]
               1 (64 bytes) <CFString 0x7fd261d4e630> [64]
            1 (32 bytes) <CFDictionary (Key Storage) 0x7fd261d07570> [32]

      8 (656 bytes) ROOT LEAK: <CFError 0x7fd261d49230> [48]
         7 (608 bytes) _userInfo --> <CFDictionary 0x7fd261d49890> [64]
            5 (512 bytes) <CFDictionary (Value Storage) 0x7fd261d07b30> [32]
               3 (416 bytes) <NSArray 0x7fd261d49390> [16]
                  2 (400 bytes) __strong _object --> <NSURL 0x7fd261ee3000> [96]
                     1 (304 bytes) _clients --> <CFString 0x7fd261ee6bd0> [304]
               1 (64 bytes) <CFString 0x7fd261d34fc0> [64]
            1 (32 bytes) <CFDictionary (Key Storage) 0x7fd261d2daa0> [32]

[David Kilzer (:ddkilzer)]

Created attachment 362234 [details]
Patch v1

[Radar WebKit Bug Importer]

<rdar://problem/48147429>

[Alexey Proskuryakov]

Comment on attachment 362234 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=362234&action=review

> Tools/ChangeLog:13
> +        CTFontManagerRegisterFontsForURL() to ignore any errors, thereby
> +        fixing the leaks.

Tangentially, what are the errors? Doesn't seem like we should be expecting any, so it may be an indication of something pretty terrible.

[David Kilzer (:ddkilzer)]

Created attachment 362303 [details]
Patch v2

Now prints out error message when registration fails.

[David Kilzer (:ddkilzer)]

(In reply to Alexey Proskuryakov from comment #3)
> Comment on attachment 362234 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=362234&action=review
> 
> > Tools/ChangeLog:13
> > +        CTFontManagerRegisterFontsForURL() to ignore any errors, thereby
> > +        fixing the leaks.
> 
> Tangentially, what are the errors? Doesn't seem like we should be expecting
> any, so it may be an indication of something pretty terrible.

Only these four tests trigger the error:

$BUILD_ROOT/layout-test-results/fast/text/user-installed-fonts/shadow-family-stderr.txt

2019-02-18 10:29:46.488 com.apple.WebKit.WebContent.Development[62401:23370792] Failed to activate fake Helvetica: Error Domain=com.apple.CoreText.CTFontManagerErrorDomain Code=105 "Could not register the font file(s)" UserInfo={NSDescription=Could not register the font file(s), CTFontManagerErrorFontURLs=(
    "file://$HOME/Library/Developer/CoreSimulator/Devices/453C9A70-C13B-4D78-AE6E-FE1A9F04643A/data/Containers/Bundle/Application/8D8B896F-A06F-4ED2-B157-C87593F9B230/WebKitTestRunnerApp.app/PlugIns/WebKitTestRunnerInjectedBundle.bundle/FakeHelvetica-Helvetica-500.ttf"
)}

$BUILD_ROOT/layout-test-results/fast/text/user-installed-fonts/shadow-stderr.txt

2019-02-18 10:29:48.071 com.apple.WebKit.WebContent.Development[62401:23370792] Failed to activate fake Helvetica: Error Domain=com.apple.CoreText.CTFontManagerErrorDomain Code=105 "Could not register the font file(s)" UserInfo={NSDescription=Could not register the font file(s), CTFontManagerErrorFontURLs=(
    "file://$HOME/Library/Developer/CoreSimulator/Devices/453C9A70-C13B-4D78-AE6E-FE1A9F04643A/data/Containers/Bundle/Application/8D8B896F-A06F-4ED2-B157-C87593F9B230/WebKitTestRunnerApp.app/PlugIns/WebKitTestRunnerInjectedBundle.bundle/FakeHelvetica-Helvetica-400.ttf"
)}

$BUILD_ROOT/layout-test-results/fast/text/user-installed-fonts/shadow-family-disable-stderr.txt

2019-02-18 10:29:46.240 com.apple.WebKit.WebContent.Development[62401:23370792] Failed to activate fake Helvetica: Error Domain=com.apple.CoreText.CTFontManagerErrorDomain Code=105 "Could not register the font file(s)" UserInfo={NSDescription=Could not register the font file(s), CTFontManagerErrorFontURLs=(
    "file://$HOME/Library/Developer/CoreSimulator/Devices/453C9A70-C13B-4D78-AE6E-FE1A9F04643A/data/Containers/Bundle/Application/8D8B896F-A06F-4ED2-B157-C87593F9B230/WebKitTestRunnerApp.app/PlugIns/WebKitTestRunnerInjectedBundle.bundle/FakeHelvetica-Helvetica-500.ttf"
)}

$BUILD_ROOT/layout-test-results/fast/text/user-installed-fonts/shadow-disable-stderr.txt

2019-02-18 10:29:45.988 com.apple.WebKit.WebContent.Development[62401:23370792] Failed to activate fake Helvetica: Error Domain=com.apple.CoreText.CTFontManagerErrorDomain Code=105 "Could not register the font file(s)" UserInfo={NSDescription=Could not register the font file(s), CTFontManagerErrorFontURLs=(
    "file://$HOME/Library/Developer/CoreSimulator/Devices/453C9A70-C13B-4D78-AE6E-FE1A9F04643A/data/Containers/Bundle/Application/8D8B896F-A06F-4ED2-B157-C87593F9B230/WebKitTestRunnerApp.app/PlugIns/WebKitTestRunnerInjectedBundle.bundle/FakeHelvetica-Helvetica-400.ttf"
)}

Note that these files DO exist at this path, too.

Are they being registered more than once?

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #5)
> (In reply to Alexey Proskuryakov from comment #3)
> > Comment on attachment 362234 [details]
> > Patch v1
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=362234&action=review
> > 
> > > Tools/ChangeLog:13
> > > +        CTFontManagerRegisterFontsForURL() to ignore any errors, thereby
> > > +        fixing the leaks.
> > 
> > Tangentially, what are the errors? Doesn't seem like we should be expecting
> > any, so it may be an indication of something pretty terrible.
> 
> Only these four tests trigger the error:
> 
> $BUILD_ROOT/layout-test-results/fast/text/user-installed-fonts/shadow-family-
> stderr.txt
> 
> 2019-02-18 10:29:46.488
> com.apple.WebKit.WebContent.Development[62401:23370792] Failed to activate
> fake Helvetica: Error Domain=com.apple.CoreText.CTFontManagerErrorDomain
> Code=105 "Could not register the font file(s)" UserInfo={NSDescription=Could
> not register the font file(s), CTFontManagerErrorFontURLs=(
>    
> "file://$HOME/Library/Developer/CoreSimulator/Devices/453C9A70-C13B-4D78-
> AE6E-FE1A9F04643A/data/Containers/Bundle/Application/8D8B896F-A06F-4ED2-B157-
> C87593F9B230/WebKitTestRunnerApp.app/PlugIns/WebKitTestRunnerInjectedBundle.
> bundle/FakeHelvetica-Helvetica-500.ttf"
> )}
> 
> $BUILD_ROOT/layout-test-results/fast/text/user-installed-fonts/shadow-stderr.
> txt
> 
> 2019-02-18 10:29:48.071
> com.apple.WebKit.WebContent.Development[62401:23370792] Failed to activate
> fake Helvetica: Error Domain=com.apple.CoreText.CTFontManagerErrorDomain
> Code=105 "Could not register the font file(s)" UserInfo={NSDescription=Could
> not register the font file(s), CTFontManagerErrorFontURLs=(
>    
> "file://$HOME/Library/Developer/CoreSimulator/Devices/453C9A70-C13B-4D78-
> AE6E-FE1A9F04643A/data/Containers/Bundle/Application/8D8B896F-A06F-4ED2-B157-
> C87593F9B230/WebKitTestRunnerApp.app/PlugIns/WebKitTestRunnerInjectedBundle.
> bundle/FakeHelvetica-Helvetica-400.ttf"
> )}
> 
> $BUILD_ROOT/layout-test-results/fast/text/user-installed-fonts/shadow-family-
> disable-stderr.txt
> 
> 2019-02-18 10:29:46.240
> com.apple.WebKit.WebContent.Development[62401:23370792] Failed to activate
> fake Helvetica: Error Domain=com.apple.CoreText.CTFontManagerErrorDomain
> Code=105 "Could not register the font file(s)" UserInfo={NSDescription=Could
> not register the font file(s), CTFontManagerErrorFontURLs=(
>    
> "file://$HOME/Library/Developer/CoreSimulator/Devices/453C9A70-C13B-4D78-
> AE6E-FE1A9F04643A/data/Containers/Bundle/Application/8D8B896F-A06F-4ED2-B157-
> C87593F9B230/WebKitTestRunnerApp.app/PlugIns/WebKitTestRunnerInjectedBundle.
> bundle/FakeHelvetica-Helvetica-500.ttf"
> )}
> 
> $BUILD_ROOT/layout-test-results/fast/text/user-installed-fonts/shadow-
> disable-stderr.txt
> 
> 2019-02-18 10:29:45.988
> com.apple.WebKit.WebContent.Development[62401:23370792] Failed to activate
> fake Helvetica: Error Domain=com.apple.CoreText.CTFontManagerErrorDomain
> Code=105 "Could not register the font file(s)" UserInfo={NSDescription=Could
> not register the font file(s), CTFontManagerErrorFontURLs=(
>    
> "file://$HOME/Library/Developer/CoreSimulator/Devices/453C9A70-C13B-4D78-
> AE6E-FE1A9F04643A/data/Containers/Bundle/Application/8D8B896F-A06F-4ED2-B157-
> C87593F9B230/WebKitTestRunnerApp.app/PlugIns/WebKitTestRunnerInjectedBundle.
> bundle/FakeHelvetica-Helvetica-400.ttf"
> )}
> 
> Note that these files DO exist at this path, too.
> 
> Are they being registered more than once?

Myles, is this expected?

[Myles C. Maxfield]

(In reply to David Kilzer (:ddkilzer) from comment #6)
> Myles, is this expected?

Nope.

[David Kilzer (:ddkilzer)]

(In reply to Myles C. Maxfield from comment #7)
> (In reply to David Kilzer (:ddkilzer) from comment #6)
> > Myles, is this expected?
> 
> Nope.

Should this block landing this patch since this fixes the leak, and adds stderr output to these tests?

[Alexey Proskuryakov]

Seems like the right behavior here would probably be to RELEASE_ASSERT. But first the issue has to be fixed of course.

[David Kilzer (:ddkilzer)]

(In reply to Alexey Proskuryakov from comment #9)
> Seems like the right behavior here would probably be to RELEASE_ASSERT. But
> first the issue has to be fixed of course.

According to LayoutTests/platform/ios/TestExpectations which was added for Bug 180062 in r225641, this is expected behavior on iOS:

+# User-installed font infrastructure is ony present on certain OSes.
+webkit.org/b/180062 fast/text/user-installed-fonts/disable.html [ ImageOnlyFailure ]
+webkit.org/b/180062 fast/text/user-installed-fonts/shadow-family.html [ ImageOnlyFailure ]
+webkit.org/b/180062 fast/text/user-installed-fonts/shadow-postscript.html [ ImageOnlyFailure ]
+webkit.org/b/180062 fast/text/user-installed-fonts/shadow.html [ ImageOnlyFailure ]

In fact, it's only supposed to work on Mojave and above on WebKit2:

LayoutTests/platform/mac-wk1/TestExpectations

+# User-installed fonts test infrastructure is not present in WK1
+webkit.org/b/180062 fast/text/user-installed-fonts [ ImageOnlyFailure ]

LayoutTests/platform/mac/TestExpectations

+# User-installed font infrastructure is ony present on certain OSes.
+webkit.org/b/180062 [ ElCapitan Sierra HighSierra ] fast/text/user-installed-fonts/disable.html [ ImageOnlyFailure ]
+webkit.org/b/180062 [ ElCapitan Sierra HighSierra ] fast/text/user-installed-fonts/shadow-postscript.html [ ImageOnlyFailure ]
+webkit.org/b/180062 [ ElCapitan Sierra HighSierra ] fast/text/user-installed-fonts/shadow.html [ ImageOnlyFailure ]

So do we change the code to prevent the CFErrorRef objects from being created (going back to Patch v1 / Attachment #362234 [details])?  Or get the errors, but only report them on macOS Mojave and newer?

[Alexey Proskuryakov]

Another alternative is to ifdef out the code where it's not going to work.

[David Kilzer (:ddkilzer)]

Created attachment 362389 [details]
Patch v3

Only log errors in installFakeHelvetica() if supported platform.

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #12)
> Created attachment 362389 [details]
> Patch v3
> 
> Only log errors in installFakeHelvetica() if supported platform.

Patch v3 only logs on supported platforms.  Should we just make the whole WTR::installFakeHelvetica() method do nothing except on macOS Mojave or newer?

[EWS Watchlist]

Attachment 362389 [details] did not pass style-queue:


ERROR: Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:146:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[David Kilzer (:ddkilzer)]

(In reply to Build Bot from comment #14)
> Attachment 362389 [details] did not pass style-queue:
> 
> 
> ERROR:
> Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:146: 
> Misplaced OS version check. Please use a named macro in wtf/Platform.h,
> wtf/FeatureDefines.h, or an appropriate internal file. 
> [build/version_check] [5]
> Total errors found: 1 in 2 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

+ #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400

So this warning means we should make up a brand new macro for wtf/Platform.h for this specific case?

Or should we just change the macro to `#if USE(APPKIT)` because eventually this code will work on all macOS versions, and the expectations in LayoutTests/platform/mac/TestExpectations (see Comment #10) will eventually be removed?

[Alexey Proskuryakov]

Perhaps we already have a feature macro for this? Adding one for testing code would be silly, but testing code features usually correspond to real features.

[Myles C. Maxfield]

Comment on attachment 362389 [details]
Patch v3

View in context: https://bugs.webkit.org/attachment.cgi?id=362389&action=review

> Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:92
> +                if (!CTFontManagerRegisterFontsForURL((CFURLRef)fontURL, kCTFontManagerScopeProcess, &error)) {

Shouldn't we be using C++-style casts?

> Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:147
> +        // Registering shadow fonts is only supported on macOS Mojave or newer. See Bugs 180062 & 194761.

I think it's supposed to work on iOS, but we shouldn't block this patch on that. We can open a new bug to investigate why it doesn't work on iOS.

[David Kilzer (:ddkilzer)]

Comment on attachment 362389 [details]
Patch v3

View in context: https://bugs.webkit.org/attachment.cgi?id=362389&action=review

>> Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:92
>> +                if (!CTFontManagerRegisterFontsForURL((CFURLRef)fontURL, kCTFontManagerScopeProcess, &error)) {
> 
> Shouldn't we be using C++-style casts?

Will fix.

>> Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:147
>> +        // Registering shadow fonts is only supported on macOS Mojave or newer. See Bugs 180062 & 194761.
> 
> I think it's supposed to work on iOS, but we shouldn't block this patch on that. We can open a new bug to investigate why it doesn't work on iOS.

It appears to work on iOS, but not iOS Simulator.  I will file a radar to track this internally.

[David Kilzer (:ddkilzer)]

Comment on attachment 362389 [details]
Patch v3

View in context: https://bugs.webkit.org/attachment.cgi?id=362389&action=review

>>> Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:92
>>> +                if (!CTFontManagerRegisterFontsForURL((CFURLRef)fontURL, kCTFontManagerScopeProcess, &error)) {
>> 
>> Shouldn't we be using C++-style casts?
> 
> Will fix.

Actually, to make these ARC-compliant (for a future switch to use ARC in this project), we have to use C-style casts:  (__bridge CFTypeRef).

Using static_cast<__bridge CFTypeRef>() doesn't work.  :(

[David Kilzer (:ddkilzer)]

Committed r241920: <https://trac.webkit.org/changeset/241920>

[Myles C. Maxfield]

(In reply to David Kilzer (:ddkilzer) from comment #19)
> Comment on attachment 362389 [details]
> Patch v3
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=362389&action=review
> 
> >>> Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:92
> >>> +                if (!CTFontManagerRegisterFontsForURL((CFURLRef)fontURL, kCTFontManagerScopeProcess, &error)) {
> >> 
> >> Shouldn't we be using C++-style casts?
> > 
> > Will fix.
> 
> Actually, to make these ARC-compliant (for a future switch to use ARC in
> this project), we have to use C-style casts:  (__bridge CFTypeRef).
> 
> Using static_cast<__bridge CFTypeRef>() doesn't work.  :(

👎