Bug 85956 - [EFL] EFL's DRT does not print didFailProvisionalLoadWithError messages
Summary: [EFL] EFL's DRT does not print didFailProvisionalLoadWithError messages
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: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-08 22:55 PDT by Chris Dumez
Modified: 2012-05-23 00:42 PDT (History)
5 users (show)

See Also:


Attachments
Patch (15.24 KB, patch)
2012-05-09 01:04 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (16.79 KB, patch)
2012-05-09 01:56 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (16.87 KB, patch)
2012-05-10 00:26 PDT, Chris Dumez
gustavo: review+
gustavo: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (17.12 KB, patch)
2012-05-22 23:04 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2012-05-08 22:55:32 PDT
EFL's DRT does not print didFailProvisionalLoadWithError messages.

This causes the following test to fail:
http/tests/loading/pdf-commit-load-callbacks.html
Comment 1 Chris Dumez 2012-05-09 01:04:34 PDT
Created attachment 140879 [details]
Patch
Comment 2 Gyuyoung Kim 2012-05-09 01:12:57 PDT
Comment on attachment 140879 [details]
Patch

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

> Source/WebKit/efl/ewk/ewk_view.cpp:2970
> +    evas_object_smart_callback_call(ewkView, "load,provisional,failed", (void*)error);

It looks you are missing to write this signal description in ewk_view.h.
Comment 3 Chris Dumez 2012-05-09 01:56:28 PDT
Created attachment 140887 [details]
Patch

Take feedback into consideration.
Comment 4 Raphael Kubo da Costa (:rakuco) 2012-05-09 18:49:34 PDT
Comment on attachment 140887 [details]
Patch

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

Do you have any idea why the expectation varies for the PDF test across ports? Shouldn't our version match GTK's?

> Source/WebKit/efl/ewk/ewk_frame.cpp:1380
> +    evas_object_smart_callback_call(ewkFrame, "load,provisional,failed", (void*)error);

Please don't use C-style casts.

> Source/WebKit/efl/ewk/ewk_view.cpp:2970
> +    evas_object_smart_callback_call(ewkView, "load,provisional,failed", (void*)error);

Ditto.
Comment 5 Chris Dumez 2012-05-10 00:00:23 PDT
Here is the diff between our expectations and GTK's:
--- LayoutTests/platform/efl/http/tests/loading/pdf-commit-load-callbacks-expected.txt	2012-05-10 09:55:07.259467379 +0300
+++ LayoutTests/platform/gtk/http/tests/loading/pdf-commit-load-callbacks-expected.txt	2012-05-09 11:14:37.317981072 +0300
@@ -3,6 +3,5 @@
 frame "<!--framePath //<!--frame0-->-->" - didStartProvisionalLoadForFrame
 main frame - didFinishDocumentLoadForFrame
 main frame - didHandleOnloadEventsForFrame
-frame "<!--framePath //<!--frame0-->-->" - didFailProvisionalLoadWithError
 main frame - didFinishLoadForFrame

Basically, the GTK port does not print out "didFailProvisionalLoadWithError" messages yet.

For this reason, we are using the same expectations as Qt, which has support for "didFailProvisionalLoadWithError" messages.

I'll reupload a patch with the casting issues fixed.
Comment 6 Chris Dumez 2012-05-10 00:26:46 PDT
Created attachment 141097 [details]
Patch

Removed C-style casts.
Comment 7 Raphael Kubo da Costa (:rakuco) 2012-05-10 07:10:04 PDT
Comment on attachment 141097 [details]
Patch

Looks OK.
Comment 8 Chris Dumez 2012-05-10 08:27:36 PDT
For reference, the platform-specific expected results were introduced by:
https://bugs.webkit.org/show_bug.cgi?id=85275
Comment 9 Gustavo Noronha (kov) 2012-05-22 07:58:06 PDT
Comment on attachment 141097 [details]
Patch

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

cq- because of the nit

> Tools/DumpRenderTree/efl/DumpRenderTreeChrome.cpp:583
> +    // signals. m_provisionalLoadFailedFrame is used to avoid printing twice the load error in

Adding a colon after 'error' here would make this clearer I believe.
Comment 10 Chris Dumez 2012-05-22 23:04:24 PDT
Created attachment 143466 [details]
Patch for landing
Comment 11 WebKit Review Bot 2012-05-23 00:42:19 PDT
Comment on attachment 143466 [details]
Patch for landing

Clearing flags on attachment: 143466

Committed r118142: <http://trac.webkit.org/changeset/118142>
Comment 12 WebKit Review Bot 2012-05-23 00:42:28 PDT
All reviewed patches have been landed.  Closing bug.