Bug 97081

Summary: [EFL][WK2] Check timeout on waitUntilLoadFinished() and waitUntilTitleChangedTo()
Product: WebKit Reporter: Byungwoo Lee <bw80.lee>
Component: WebKit EFLAssignee: Byungwoo Lee <bw80.lee>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, gyuyoung.kim, lucas.de.marchi, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 97094    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Byungwoo Lee 2012-09-19 00:58:02 PDT 
add timeout to the functions and check return value of the functions will be better to prevent test timeout due to the infinite loop.
Comment 1 Byungwoo Lee 2012-09-19 07:47:30 PDT 
Created attachment 164737 [details]
Patch
Comment 2 Chris Dumez 2012-09-19 08:21:03 PDT 
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".
Comment 3 Gyuyoung Kim 2012-09-19 18:01:22 PDT 
Comment on attachment 164737 [details]
Patch

Looks fine as well. Please land after fixing the nit Chrisophe pointed out.
Comment 4 Byungwoo Lee 2012-09-19 18:13:29 PDT 
(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 :)
Comment 5 Byungwoo Lee 2012-09-19 21:20:40 PDT 
Created attachment 164834 [details]
Patch
Comment 6 Gyuyoung Kim 2012-09-19 21:38:47 PDT 
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.
Comment 7 Byungwoo Lee 2012-09-19 21:40:56 PDT 
(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.
Comment 8 Byungwoo Lee 2012-09-20 01:30:48 PDT 
(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. :)
Comment 9 Chris Dumez 2012-09-20 01:34:04 PDT 
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.
Comment 10 Byungwoo Lee 2012-09-20 01:35:44 PDT 
(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
Comment 11 Byungwoo Lee 2012-09-20 01:50:23 PDT 
Created attachment 164861 [details]
Patch
Comment 12 Chris Dumez 2012-09-20 01:54:08 PDT 
Comment on attachment 164861 [details]
Patch

LGTM. Thanks.
Comment 13 Gyuyoung Kim 2012-09-20 01:55:04 PDT 
Comment on attachment 164861 [details]
Patch

Please land this after passing unit test both release and debug.
Comment 14 Byungwoo Lee 2012-09-20 03:01:01 PDT 
Comment on attachment 164861 [details]
Patch

I tested the last patch again for release/debug, and both were ok. Thanks :)
Comment 15 WebKit Review Bot 2012-09-20 03:27:01 PDT 
Comment on attachment 164861 [details]
Patch

Clearing flags on attachment: 164861

Committed r129111: <http://trac.webkit.org/changeset/129111>
Comment 16 WebKit Review Bot 2012-09-20 03:27:05 PDT 
All reviewed patches have been landed.  Closing bug.