Bug 188994

Summary: Teach WebKitTestRunner and DumpRenderTree about detecting world leaks
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, cdumez, dbates, esprehn+autocc, ews-watchlist, jbedard, kangil.han, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 186214    
Attachments:
Description Flags
Patch
none
Patch thorton: review+

Simon Fraser (smfr)
Reported 2018-08-27 11:20:18 PDT
Teach WebKitTestRunner and DumpRenderTree about detecting world leaks
Attachments
Patch (40.19 KB, patch)
2018-08-27 11:29 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Patch (40.11 KB, patch)
2018-08-27 14:33 PDT, Simon Fraser (smfr) 		thorton: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2018-08-27 11:29:11 PDT
Created attachment 348176 [details] Patch
EWS Watchlist
Comment 2 2018-08-27 11:31:56 PDT
Attachment 348176 [details] did not pass style-queue: ERROR: Tools/WebKitTestRunner/TestController.h:53: Extra space before ( in function call [whitespace/parens] [4] ERROR: Tools/WebKitTestRunner/TestController.h:73: Extra space before ( in function call [whitespace/parens] [4] ERROR: Tools/WebKitTestRunner/TestController.h:266: Extra space before ( in function call [whitespace/parens] [4] ERROR: Tools/WebKitTestRunner/TestController.cpp:1378: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 4 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.
Simon Fraser (smfr)
Comment 3 2018-08-27 14:33:53 PDT
Created attachment 348201 [details] Patch
EWS Watchlist
Comment 4 2018-08-27 14:36:58 PDT
Attachment 348201 [details] did not pass style-queue: ERROR: Tools/WebKitTestRunner/TestController.h:53: Extra space before ( in function call [whitespace/parens] [4] ERROR: Tools/WebKitTestRunner/TestController.h:73: Extra space before ( in function call [whitespace/parens] [4] ERROR: Tools/WebKitTestRunner/TestController.h:266: Extra space before ( in function call [whitespace/parens] [4] ERROR: Tools/WebKitTestRunner/TestController.cpp:1379: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 4 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.
Simon Fraser (smfr)
Comment 5 2018-08-27 16:31:53 PDT
https://trac.webkit.org/r235408
Radar WebKit Bug Importer
Comment 6 2018-08-27 16:33:27 PDT
<rdar://problem/43775591>
Jonathan Bedard
Comment 7 2018-08-28 08:23:57 PDT
I hate to bring this up after this patch has landed, but, any reason we didn't simply add a new option on the input line? (Grep for '--pixel-test' in TestController.cpp)
Simon Fraser (smfr)
Comment 8 2018-08-28 08:27:39 PDT
(In reply to Jonathan Bedard from comment #7) > I hate to bring this up after this patch has landed, but, any reason we > didn't simply add a new option on the input line? (Grep for '--pixel-test' > in TestController.cpp) There is: optionList.append(Option("--check-for-world-leaks", "Check for leaks of world objects (currently, documents)", handleOptionCheckForWorldLeaks)); or did you mean something else?
Jonathan Bedard
Comment 9 2018-08-28 08:57:21 PDT
(In reply to Simon Fraser (smfr) from comment #8) > (In reply to Jonathan Bedard from comment #7) > > I hate to bring this up after this patch has landed, but, any reason we > > didn't simply add a new option on the input line? (Grep for '--pixel-test' > > in TestController.cpp) > > There is: > optionList.append(Option("--check-for-world-leaks", "Check for leaks of > world objects (currently, documents)", handleOptionCheckForWorldLeaks)); > > or did you mean something else? I'm talking about the ability of WebKitTestRunner to accept options on the stdin line that contains the layout test to be run. line 1321 in TestController.cpp contains a function named 'parseInputLine' which takes the line read from WebKitTestRunner's stdin and extracts a few options from it. Is there a reason that world leaks can't use this? It seems that this offers basically the same functionality as handleControlCommand().
Simon Fraser (smfr)
Comment 10 2018-08-28 10:56:08 PDT
parseInputLine() expects the first part to be a test URL, and would have to be changed to not run a test, but get the leaks results. I think my way is a bit clearer.
Jonathan Bedard
Comment 11 2018-08-28 11:21:33 PDT
(In reply to Simon Fraser (smfr) from comment #10) > parseInputLine() expects the first part to be a test URL, and would have to > be changed to not run a test, but get the leaks results. I think my way is a > bit clearer. Or we could set it to run after the test finishes, but I agree now that I understand this better, your way is clearer.
Note You need to log in before you can comment on or make changes to this bug.