Bug 97081 - [EFL][WK2] Check timeout on waitUntilLoadFinished() and waitUntilTitleChangedTo()
Summary: [EFL][WK2] Check timeout on waitUntilLoadFinished() and waitUntilTitleChanged...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Byungwoo Lee
URL:
Keywords:
Depends on: 97094
Blocks:
  Show dependency tree / graph
 
Reported: 2012-09-19 00:58 PDT by Byungwoo Lee
Modified: 2012-09-20 03:27 PDT (History)
5 users (show)

See Also:

Attachments
Patch (17.58 KB, patch)
2012-09-19 07:47 PDT, Byungwoo Lee 		no flags Details | Formatted Diff | Diff
Patch (17.56 KB, patch)
2012-09-19 21:20 PDT, Byungwoo Lee 		no flags Details | Formatted Diff | Diff
Patch (18.13 KB, patch)
2012-09-20 01:50 PDT, Byungwoo Lee 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.