For asserts (and manually called CRASH-es) we print the backtrace twice, once with WTFReportBacktrace - which just prints adresses - and once with the signal handler. Also the logic of generating the backtrace is duplicated in DRT and WTR.
Created attachment 120321 [details] Patch
Attachment 120321 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/JavaScriptCore/Change..." exit_code: 1 Tools/DumpRenderTree/qt/main.cpp:57: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #2) > Attachment 120321 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/JavaScriptCore/Change..." exit_code: 1 > > Tools/DumpRenderTree/qt/main.cpp:57: Alphabetical sorting problem. [build/include_order] [4] > Total errors found: 1 in 8 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. This could only be fixed by including config.h in DRT's main.cpp (because of header cross-dependencies) but I don't think we should do that.
Comment on attachment 120321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120321&action=review > Source/JavaScriptCore/wtf/Assertions.cpp:260 > + globalHook(); As ironic as it may sound in the context of crash handling, but shouldn't we do a null pointer check here? :)
(In reply to comment #4) > (From update of attachment 120321 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=120321&action=review > > > Source/JavaScriptCore/wtf/Assertions.cpp:260 > > + globalHook(); > > As ironic as it may sound in the context of crash handling, but shouldn't we do a null pointer check here? :) Yep, definitely we should.
Created attachment 122190 [details] null-check added
Attachment 122190 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/JavaScriptCore/Change..." exit_code: 1 Tools/DumpRenderTree/qt/main.cpp:57: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 122190 [details] null-check added I think the patch is a good idea, but it doesn't seem to build on mac and win yet - according to the EWS :)
Created attachment 122449 [details] speculatively fixed mac&win build
Attachment 122449 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/JavaScriptCore/Change..." exit_code: 1 Tools/DumpRenderTree/qt/main.cpp:57: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 122449 [details] speculatively fixed mac&win build View in context: https://bugs.webkit.org/attachment.cgi?id=122449&action=review > Source/JavaScriptCore/wtf/Assertions.cpp:317 > #endif > +#endif > const int frameNumber = i - framesToSkip + 1; > if (mangledName || cxaDemangled) The second #endif closes the #if OS(DARWIN) || OS(LINUX) and results in the line below to be compiled on windows, causing the win-ews failure.
Created attachment 122743 [details] Patch
Attachment 122743 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/JavaScriptCore/Change..." exit_code: 1 Tools/DumpRenderTree/qt/main.cpp:57: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 122743 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=122743&action=review r=me, but please fix the style issue and add the missing "static" before landing. > Source/JavaScriptCore/wtf/Assertions.cpp:328 > +WTFCrashHookFunction globalHook = 0; Missing static?
(In reply to comment #14) > (From update of attachment 122743 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=122743&action=review > > r=me, but please fix the style issue and add the missing "static" before landing. Unfortunately the includes do not work in alphabetical order because wtf/Assertions.h use some definitions from wtf/ExportMacros.h. Normally this is not an issue because config.h provides such headers but since this is the main.cpp of DRT we don't include config.h and I guess this is intentional. Do you think it's ok to change on that?
(In reply to comment #15) > (In reply to comment #14) > > (From update of attachment 122743 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=122743&action=review > > > > r=me, but please fix the style issue and add the missing "static" before landing. > > Unfortunately the includes do not work in alphabetical order because wtf/Assertions.h use some definitions from wtf/ExportMacros.h. Normally this is not an issue because config.h provides such headers but since this is the main.cpp of DRT we don't include config.h and I guess this is intentional. Do you think it's ok to change on that? Ah right, ok :) So only "static" is left.
Committed r105267: <http://trac.webkit.org/changeset/105267>