Expose more information about the exception in WKErrorJavaScriptExceptionOccurred errors
Created attachment 263970 [details] Patch
<rdar://problem/23245452>
Attachment 263970 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/WebPageProxy.cpp:2591: Extra space before ( in function call [whitespace/parens] [4] ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewEvaluateJavaScript.mm:97: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewEvaluateJavaScript.mm:116: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/WebPageProxy.h:766: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 4 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 263971 [details] Patch
Attachment 263971 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/WebPageProxy.cpp:2591: Extra space before ( in function call [whitespace/parens] [4] ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewEvaluateJavaScript.mm:112: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/WebPageProxy.h:766: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 263971 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263971&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKErrorPrivate.h:50 > + the exception source URL (as an NSString) for WKErrorJavaScriptExceptionOccurred errors. */ Why not as an NSURL? > Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:654 > + A14FC5831B89739100D107EB /* WKWebViewConfigurationExtras.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = WKWebViewConfigurationExtras.mm; path = ../../WKWebViewConfigurationExtras.mm; sourceTree = "<group>"; }; > + A14FC5841B89739100D107EB /* WKWebViewConfigurationExtras.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = WKWebViewConfigurationExtras.h; path = ../../WKWebViewConfigurationExtras.h; sourceTree = "<group>"; }; > A14FC5861B8991B600D107EB /* ContentFiltering.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ContentFiltering.mm; sourceTree = "<group>"; }; > A14FC5891B89927100D107EB /* ContentFilteringPlugIn.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ContentFilteringPlugIn.mm; sourceTree = "<group>"; }; > - A14FC58D1B8AE36500D107EB /* TestProtocol.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = TestProtocol.h; path = cocoa/TestProtocol.h; sourceTree = "<group>"; }; > - A14FC58E1B8AE36500D107EB /* TestProtocol.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = TestProtocol.mm; path = cocoa/TestProtocol.mm; sourceTree = "<group>"; }; > + A14FC58D1B8AE36500D107EB /* TestProtocol.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = TestProtocol.h; path = ../../cocoa/TestProtocol.h; sourceTree = "<group>"; }; > + A14FC58E1B8AE36500D107EB /* TestProtocol.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = TestProtocol.mm; path = ../../cocoa/TestProtocol.mm; sourceTree = "<group>"; }; ?? > Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:981 > - path = cocoa/WebProcessPlugIn; > + path = ../../cocoa/WebProcessPlugIn; ??
Comment on attachment 263971 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263971&action=review >> Source/WebKit2/UIProcess/API/Cocoa/WKErrorPrivate.h:50 >> + the exception source URL (as an NSString) for WKErrorJavaScriptExceptionOccurred errors. */ > > Why not as an NSURL? Excellent question. > Source/WebKit2/UIProcess/WebPageProxy.cpp:2594 > + callbackFunction(nullptr, false, ExceptionDetails(), CallbackBase::Error::Unknown); Sometimes I like to use { } instead of ExceptionDetails() at a call site like this one. Or possibly ExceptionDetails{ }. > Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:649 > + A14FC5831B89739100D107EB /* WKWebViewConfigurationExtras.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = WKWebViewConfigurationExtras.mm; path = ../../WKWebViewConfigurationExtras.mm; sourceTree = "<group>"; }; As Dan noted below, I see a lot of changes like this and it does not seem good.
(In reply to comment #7) > Comment on attachment 263971 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=263971&action=review > > >> Source/WebKit2/UIProcess/API/Cocoa/WKErrorPrivate.h:50 > >> + the exception source URL (as an NSString) for WKErrorJavaScriptExceptionOccurred errors. */ > > > > Why not as an NSURL? > > Excellent question. Will fix! > > Source/WebKit2/UIProcess/WebPageProxy.cpp:2594 > > + callbackFunction(nullptr, false, ExceptionDetails(), CallbackBase::Error::Unknown); > > Sometimes I like to use { } instead of ExceptionDetails() at a call site > like this one. Or possibly ExceptionDetails{ }. I hadn't picked up that trick yet, but I like it! > > Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:649 > > + A14FC5831B89739100D107EB /* WKWebViewConfigurationExtras.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = WKWebViewConfigurationExtras.mm; path = ../../WKWebViewConfigurationExtras.mm; sourceTree = "<group>"; }; > > As Dan noted below, I see a lot of changes like this and it does not seem > good. This was actually semi-intentional, but I'll do it in another patch. The XCode project has duplicate groups (cocoa and Cocoa) in one place.
http://trac.webkit.org/changeset/191543
Something bad is happening: https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/680/steps/run-api-tests/logs/stdio Building release to see if I can reproduce.
(In reply to comment #10) > Something bad is happening: > https://build.webkit.org/builders/ > Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/680/steps/run-api- > tests/logs/stdio > > Building release to see if I can reproduce. Weird? In a subsequent build it's complaining about something *else* I added to the API in another patch (http://trac.webkit.org/changeset/191545).
(In reply to comment #11) > (In reply to comment #10) > > Something bad is happening: > > https://build.webkit.org/builders/ > > Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/680/steps/run-api- > > tests/logs/stdio > > > > Building release to see if I can reproduce. > > Weird? In a subsequent build it's complaining about something *else* I added > to the API in another patch (http://trac.webkit.org/changeset/191545). And neither of these reproduce locally.
(In reply to comment #10) > Something bad is happening: > https://build.webkit.org/builders/ > Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/680/steps/run-api- > tests/logs/stdio > > Building release to see if I can reproduce. Also not happening on Yosemite bots :( I don't get it.
(In reply to comment #10) > Something bad is happening: > https://build.webkit.org/builders/ > Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/680/steps/run-api- > tests/logs/stdio > > Building release to see if I can reproduce. Is there any way to see the full build log?
Looks like System Integrity Protection is making TestWebKitAPI use WebKit from /System/Library/Frameworks instead of from the archive.
(In reply to comment #15) > Looks like System Integrity Protection is making TestWebKitAPI use WebKit > from /System/Library/Frameworks instead of from the archive. Wow! I'm surprised that hasn't come up before!
(In reply to comment #16) > (In reply to comment #15) > > Looks like System Integrity Protection is making TestWebKitAPI use WebKit > > from /System/Library/Frameworks instead of from the archive. > > Wow! I'm surprised that hasn't come up before! It has come up before and <http://trac.webkit.org/r190412> was supposed to take care of it.
There must be another location that I missed - it's failing when generating a list of tests to run, not when running the tests.