Teach WebKitTestRunner and DumpRenderTree about detecting world leaks

• Component: New Bugs
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Teach WebKitTestRunner and DumpRenderTree about detecting world leaks

[Simon Fraser (smfr)]

Created attachment 348176 [details]
Patch

[EWS Watchlist]

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)]

Created attachment 348201 [details]
Patch

[EWS Watchlist]

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)]

https://trac.webkit.org/r235408

[Radar WebKit Bug Importer]

<rdar://problem/43775591>

[Jonathan Bedard]

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)]

(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]

(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)]

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]

(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.