Bug 105142 – [EFL][WK1] Obsolete a case in test_ewk_frame api test to not make bot sick

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

[Kangil Han]

'load,finished' event comes before adding callback from test framework.
That's why test is stuck.

[Kangil Han]

Created attachment 179673 [details]
Patch

[Laszlo Gombos]

Comment on attachment 179673 [details]
Patch

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

> Source/WebKit/efl/ChangeLog:8
> +        Currently, test_ewk_frame api test is time out because 'load,finished' event comes before adding callback for it.

Currently the test_ewk_frame api test times out...

> Source/WebKit/efl/ChangeLog:13
> +        However, there has been a behavior change so blank page is returned.
> +        As a result, this patch removes a test that is not available at this moment for passing test.

Do you know when this behavior changed ? Was that change intentional ? 

Should we change the expected output instead of removing the test ? It seems the be a valid test to test load URL that points to a non-existing DNS record.

[Gyuyoung Kim]

Comment on attachment 179673 [details]
Patch

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

> Source/WebKit/efl/tests/test_ewk_frame.cpp:-53
> -    // Checking if function works properly when loading non-existing url.

When I only disable this test, there is no problem in API test. So, I don't know why you have to touch loadUrl(). I think you need to check why loadUrl() can't support non-existing url.

[Gyuyoung Kim]

Comment on attachment 179673 [details]
Patch

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

>> Source/WebKit/efl/tests/test_ewk_frame.cpp:-53
>> -    // Checking if function works properly when loading non-existing url.
> 
> When I only disable this test, there is no problem in API test. So, I don't know why you have to touch loadUrl(). I think you need to check why loadUrl() can't support non-existing url.

It seems to me that there is no timeout condition in loadUrl(). FYI, EFL WK2 unit test framework has a timeout threshold for loadUrlSync. So, if there is no response until the timeout value, waitUntilLoadFinished() will return false. Why not considering this threshold this as well ?

    static const double defaultTimeoutSeconds = 10.0;
    bool loadUrlSync(const char* url, double timeoutSeconds = defaultTimeoutSeconds);


If you need to have time to prepare a patch for this, you may be able to comment this test code out with new filed bug for my suggestion. Because, API test has been failed since last week.

[Kangil Han]

BUG 49246 has changed a specific load behavior.

Load finished callback is triggered directly when input has malformed url, i.e. 'http://www.abcdefg^^.com'.

Backtrace is like this.

3   0xb3edf3ce WebCore::FrameLoader::checkLoadCompleteForThisFrame()
4   0xb3edfeb0 WebCore::FrameLoader::checkLoadComplete()
5   0xb3eba5b3 WebCore::DocumentLoader::finishedLoading()
6   0xb3ebc565 WebCore::DocumentLoader::maybeLoadEmpty()
7   0xb3ebc74e WebCore::DocumentLoader::startLoadingMainResource()
8   0xb3edf57e WebCore::FrameLoader::continueLoadAfterWillSubmitForm()
9   0xb3ee1d6e WebCore::FrameLoader::continueLoadAfterNavigationPolicy(WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)
10  0xb3ee1560 WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)
11  0xb3ef823d WebCore::PolicyCallback::call(bool)
12  0xb3ef901e WebCore::PolicyChecker::continueAfterNavigationPolicy(WebCore::PolicyAction)
13  0xb76361cb
14  0xb763747b
15  0xb3ef8a90 WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, void (*)(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool), void*)
16  0xb3edc105 WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WTF::PassRefPtr<WebCore::FormState>)
17  0xb3edbc0c WebCore::FrameLoader::load(WebCore::DocumentLoader*)
18  0xb3edb7ea WebCore::FrameLoader::load(WebCore::FrameLoadRequest const&)
19  0xb76498f8 ewk_frame_uri_set
20  0xb7676aa6 ewk_view_uri_set
21  0x805092d EWKUnitTests::EWKTestBase::loadUrl(char const*)

[Kangil Han]

(In reply to comment #4)
> It seems to me that there is no timeout condition in loadUrl(). FYI, EFL WK2 unit test framework has a timeout threshold for loadUrlSync. So, if there is no response until the timeout value, waitUntilLoadFinished() will return false. Why not considering this threshold this as well ?
> 
>     static const double defaultTimeoutSeconds = 10.0;
>     bool loadUrlSync(const char* url, double timeoutSeconds = defaultTimeoutSeconds);
> 
> 
> If you need to have time to prepare a patch for this, you may be able to comment this test code out with new filed bug for my suggestion. Because, API test has been failed since last week.

As you mentioned, buildbot has been broken for several days.
I will disable second test case here and file a new bug for fixing it.

[Kangil Han]

Created attachment 180489 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 180489 [details]
Patch

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

> Source/WebKit/efl/tests/test_ewk_frame.cpp:-57
> -    initBuffer(&buffer);

You need to file a bug for this issue. Then, you should mention the bug url here, for example,

  // FIXME: BUG 49246 has changed load behavior when malformed url is inputed. Timer operation might be needed to sync with WK2.
  // See https://bugs.webkit.org/show_bug.cgi?id=xxxx for detail.

[Kangil Han]

Created attachment 180492 [details]
Patch

[WebKit Review Bot]

Comment on attachment 180492 [details]
Patch

Clearing flags on attachment: 180492

Committed r138363: <http://trac.webkit.org/changeset/138363>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.