Bug 97081 – [EFL][WK2] Check timeout on waitUntilLoadFinished() and waitUntilTitleChangedTo()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Byungwoo Lee]

add timeout to the functions and check return value of the functions will be better to prevent test timeout due to the infinite loop.

[Byungwoo Lee]

Created attachment 164737 [details]
Patch

[Chris Dumez]

Comment on attachment 164737 [details]
Patch

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

Makes sense, thanks.

> Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.h:45
> +    static const double timeOutAfter10Seconds = 10.0;

Should probably be named something like "defaultTimeoutSeconds".

[Gyuyoung Kim]

Comment on attachment 164737 [details]
Patch

Looks fine as well. Please land after fixing the nit Chrisophe pointed out.

[Byungwoo Lee]

(In reply to comment #3)
> (From update of attachment 164737 [details])
> Looks fine as well. Please land after fixing the nit Chrisophe pointed out.

Thanks~ I'll change it :)

[Byungwoo Lee]

Created attachment 164834 [details]
Patch

[Gyuyoung Kim]

Please verify if there is no unit test failure on both release and debug before landing. As you know, there were sometime problems on unit test.

[Byungwoo Lee]

(In reply to comment #6)
> Please verify if there is no unit test failure on both release and debug before landing. As you know, there were sometime problems on unit test.

Sure. I'll test and reply. Thanks.

[Byungwoo Lee]

(In reply to comment #7)
> (In reply to comment #6)
> > Please verify if there is no unit test failure on both release and debug before landing. As you know, there were sometime problems on unit test.
> 
> Sure. I'll test and reply. Thanks.

I tested this patch with the latest source again and it works find for both release and debug.
But I found that waitUntilURIChanged is added, so I'll apply this to the function also and upload it. :)

[Chris Dumez]

https://bugs.webkit.org/show_bug.cgi?id=97094 added a new waitForXXX() method. You should probably fix the default timeout value for that method as well.

[Byungwoo Lee]

(In reply to comment #9)
> https://bugs.webkit.org/show_bug.cgi?id=97094 added a new waitForXXX() method. You should probably fix the default timeout value for that method as well.

Yes, I'm preparing

[Byungwoo Lee]

Created attachment 164861 [details]
Patch

[Chris Dumez]

Comment on attachment 164861 [details]
Patch

LGTM. Thanks.

[Gyuyoung Kim]

Comment on attachment 164861 [details]
Patch

Please land this after passing unit test both release and debug.

[Byungwoo Lee]

Comment on attachment 164861 [details]
Patch

I tested the last patch again for release/debug, and both were ok. Thanks :)

[WebKit Review Bot]

Comment on attachment 164861 [details]
Patch

Clearing flags on attachment: 164861

Committed r129111: <http://trac.webkit.org/changeset/129111>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.