[webkit-reviews] review granted: [Bug 186045] webkit-test-runner: Add support for the reftest-wait class name : [Attachment 408201] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 186045: webkit-test-runner: Add support for the reftest-wait class name
https://bugs.webkit.org/show_bug.cgi?id=186045

Attachment 408201: Patch

https://bugs.webkit.org/attachment.cgi?id=408201&action=review




--- Comment #58 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 408201
  --> https://bugs.webkit.org/attachment.cgi?id=408201
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=408201&action=review

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:1995
> -    if (injectedBundle.pageCount())
> -	   injectedBundle.page()->dump();
> -    else
> +    if (!injectedBundle.pageCount()) {
>	   injectedBundle.done();
> +	   return;
> +    }
> +
> +    pollForRefTestWaitAttributeRemoval();

This reads poorly. Dumping is the important thing, reftest-wait is a minor
feature required by a few tests.

Maybe call it 'dumpRespectingRefTestWait' or similar? (same for the other
runners)

> LayoutTests/ChangeLog:9
> +	   * TestExpectations: Expect a pass on the test that directly tests
this feature.
> +	   There are multiple other tests that should now be passing.

What's the plan for enabling them?