Bug 188191

Created attachment 346150 [details] Patch

Attachment 346150 [details] did not pass style-queue: ERROR: Source/WebCore/html/FTPDirectoryDocument.cpp:170: Missing spaces around / [whitespace/operators] [3] ERROR: Source/WebCore/html/FTPDirectoryDocument.cpp:173: Missing spaces around / [whitespace/operators] [3] ERROR: Source/WebCore/html/FTPDirectoryDocument.cpp:175: Missing spaces around / [whitespace/operators] [3] Total errors found: 3 in 116 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 346150 [details] Patch Attachment 346150 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/8709061 Number of test failures exceeded the failure limit.

Created attachment 346154 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4

Comment on attachment 346150 [details] Patch Attachment 346150 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8709327 New failing tests: legacy-animation-engine/imported/blink/transitions/unprefixed-transform.html

Created attachment 346164 [details] Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

Do we understand what happened with the iOS simulator tests?

(In reply to Build Bot from comment #2) > ERROR: Source/WebCore/html/FTPDirectoryDocument.cpp:170: Missing spaces > around / [whitespace/operators] [3] > ERROR: Source/WebCore/html/FTPDirectoryDocument.cpp:173: Missing spaces > around / [whitespace/operators] [3] > ERROR: Source/WebCore/html/FTPDirectoryDocument.cpp:175: Missing spaces > around / [whitespace/operators] [3] > Total errors found: 3 in 116 files Should probably just add the spaces since we are touching these lines.

Created attachment 346269 [details] Patch

Comment on attachment 346269 [details] Patch Attachment 346269 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/8722562 Number of test failures exceeded the failure limit.

Created attachment 346274 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4

Comment on attachment 346269 [details] Patch Clearing flags on attachment: 346269 Committed r234489: <https://trac.webkit.org/changeset/234489>

All reviewed patches have been landed. Closing bug.

<rdar://problem/42840464>

(In reply to WebKit Commit Bot from comment #12) > Comment on attachment 346269 [details] > Patch > > Clearing flags on attachment: 346269 > > Committed r234489: <https://trac.webkit.org/changeset/234489> after this revision I'm seeing 50 Layout test crashes and 63 unit test failures on iOS Layout Test output: https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Release%20WK2%20%28Tests%29/builds/6557/steps/layout-test/logs/stdio API test output: https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Release%20WK2%20%28Tests%29/builds/6557/steps/run-api-tests/logs/stdio

here's output from a debug crash: ASSERTION FAILED: dlopen(/System/Library/Frameworks/QuickLook.framework/QuickLook, 2): Symbol not found: __ZN3WTF6String6formatEPKcz Referenced from: /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/Library/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot/System/Library/PrivateFrameworks/WebInspector.framework/WebInspector Expected in: /Volumes/Data/slave/ios-simulator-11-debug-tests-wk2/build/WebKitBuild/Debug-iphonesimulator/JavaScriptCore.framework/JavaScriptCore in /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/Library/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot/System/Library/PrivateFrameworks/WebInspector.framework/WebInspector frameworkLibrary ./platform/ios/QuickLookSoftLink.mm(32) : void *WebCore::QuickLookLibrary(bool)_block_invoke 1 0x11c4b7e89 WTFCrash 2 0x12087f6e8 invocation function for block in WebCore::QuickLookLibrary(bool) 3 0x11481a779 _dispatch_client_callout 4 0x11481bc0a dispatch_once_f 5 0x12087f635 WebCore::QuickLookLibrary(bool) 6 0x120889799 invocation function for block in WebCore::initQuickLookQLTypeCopyBestMimeTypeForFileNameAndMimeType(NSString*, NSString*) 7 0x11481a779 _dispatch_client_callout 8 0x11481bc0a dispatch_once_f 9 0x12087f7a1 WebCore::initQuickLookQLTypeCopyBestMimeTypeForFileNameAndMimeType(NSString*, NSString*) 10 0x120a31904 WebCore::softLink_QuickLook_QLTypeCopyBestMimeTypeForFileNameAndMimeType(NSString*, NSString*) 11 0x120a31308 WebCore::adjustMIMETypeIfNecessary(_CFURLResponse*, bool) 12 0x10e3c95c4 -[WKNetworkSessionDelegate URLSession:dataTask:didReceiveResponse:completionHandler:] 13 0x10d955363 __NSBLOCKOPERATION_IS_CALLING_OUT_TO_A_BLOCK__ 14 0x10d9551ca -[NSBlockOperation main] 15 0x10d9536b2 -[__NSOperationInternal _start:] 16 0x11481a779 _dispatch_client_callout 17 0x11481f931 _dispatch_block_invoke_direct 18 0x11481a779 _dispatch_client_callout 19 0x11481f931 _dispatch_block_invoke_direct 20 0x11481f7d4 dispatch_block_perform 21 0x10d94f75b __NSOQSchedule_f 22 0x11481a779 _dispatch_client_callout 23 0x114824778 _dispatch_main_queue_callback_4CF 24 0x11305bc99 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ 25 0x11301fea6 __CFRunLoopRun 26 0x11301f30b CFRunLoopRunSpecific 27 0x10d93ab4a -[NSRunLoop(NSRunLoop) runMode:beforeDate:] 28 0x10d93aa25 -[NSRunLoop(NSRunLoop) run] 29 0x114c478c9 _xpc_objc_main 30 0x114c49d73 xpc_main 31 0x10d88ed9e main

Reverted r234489 for reason: Caused 50+ crashes and 60+ API failures on iOS Committed r234501: <https://trac.webkit.org/changeset/234501>

I guess there are other processes on iOS that depend on linking to the WTF::String::Format function!? We need to fix that.

Wait, no, it’s WebInspector. The problem here is that when WebKit calls out to QuickLook, QuickLook loads the *system* version of the WebInspector framework, not the one that was just built.

Who’s an expert on how this is supposed to work in when testing builds on the iOS Simulator that can help us figure out how to fix this?

(In reply to Darin Adler from comment #20) > Who’s an expert on how this is supposed to work in when testing builds on > the iOS Simulator that can help us figure out how to fix this? This sounds like something has gone wrong with our DYLD paths. Perhaps we don't have a WebInspector framework in the build directory? Looking into it.

(In reply to Darin Adler from comment #19) > Wait, no, it’s WebInspector. > > The problem here is that when WebKit calls out to QuickLook, QuickLook loads > the *system* version of the WebInspector framework, not the one that was > just built. WebInspector.framework is not built from the WebKit tree, it's from Safari tree. So for open source builds, this is expected. I couldn't find any explicit usages of String::format in the WebInspector.framework code base, but there could be implicit usages via ASSERT() or other macros that are used.

(In reply to Brian Burg from comment #22) > (In reply to Darin Adler from comment #19) > > Wait, no, it’s WebInspector. > > > > The problem here is that when WebKit calls out to QuickLook, QuickLook loads > > the *system* version of the WebInspector framework, not the one that was > > just built. > > WebInspector.framework is not built from the WebKit tree, it's from Safari > tree. So for open source builds, this is expected. > > I couldn't find any explicit usages of String::format in the > WebInspector.framework code base, but there could be implicit usages via > ASSERT() or other macros that are used. Maybe we need both String::format and String::deprecatedFormat for the next few months.

Hmm, yes there are files such as: Source/WTF/wtf/mac/DeprecatedSymbolsUsedBySafari.mm Source/JavaScriptCore/inspector/cocoa/DeprecatedInspectorValues.cpp These files play tricks to add symbols back so that older Apple binaries (such as non-trunk iOS Simulator frameworks, or non-trunk SafariForWebKitDevelopment) can continue to find the expected symbols in trunk built WebKit frameworks. I'm guessing the best fix would be to add a String::format to: Source/WTF/wtf/mac/DeprecatedSymbolsUsedBySafari.mm And include a suitable warning around its definition. Then, when the supported iOS Simulator builds have updated appropriately we can remove the DeprecatedSymbols definition.

(In reply to Brian Burg from comment #22) > > I couldn't find any explicit usages of String::format in the > WebInspector.framework code base, but there could be implicit usages via > ASSERT() or other macros that are used. I also couldn't quickly figure out where this is coming from either. WebInspector.framework does link with JavaScriptCore and use some WTF features, but we try to keep it to a minimum (mostly WTF::JSON, which does use WTF::String).

This symbol is not referenced by WebInspector.framework in trunk: midnight:OpenSource bburg$ nm /Users/bburg/repos/webkit/OpenSource/WebKitBuild/Release/WebInspector.framework/WebInspector | grep ZN3WTF6String U __ZN3WTF6StringC1ENS_12ASCIILiteralE U __ZN3WTF6StringC1EP8NSString It used to be referenced in the error code path, but this was simplified by Darin (heh): diff --git a/Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_backend_dispatcher_implementation.py b/Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_backend_dispatcher_implementation.py index a164c028f1c..2a24718ee3a 100755 --- a/Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_backend_dispatcher_implementation.py +++ b/Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_backend_dispatcher_implementation.py @@ -1,6 +1,6 @@ #!/usr/bin/env python # -# Copyright (c) 2014-2016 Apple Inc. All rights reserved. +# Copyright (c) 2014-2018 Apple Inc. All rights reserved. # Copyright (c) 2014 University of Washington. All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -177,7 +177,7 @@ class ObjCBackendDispatcherImplementationGenerator(ObjCGenerator): if isinstance(parameter.type, EnumType): lines.append(' if (!%s) {' % objc_in_param_name) - lines.append(' backendDispatcher()->reportProtocolError(BackendDispatcher::InvalidParams, String::format("Parameter \'%%s\' of method \'%%s\' cannot be processed", "%s", "%s.%s"));' % (parameter.parameter_name, domain.domain_name, command.command_name)) + lines.append(' backendDispatcher()->reportProtocolError(BackendDispatcher::InvalidParams, ASCIILiteral { "Parameter \'%s\' of method \'%s.%s\' cannot be processed" });' % (parameter.parameter_name, domain.domain_name, command.command_name)) lines.append(' return;') lines.append(' }')

Not necessary now, because I removed String::format entirely.