Bug 114850 - [EFL][WK2]: Fix WKViewClientWebProcessCallbacks WK2 API test
Summary: [EFL][WK2]: Fix WKViewClientWebProcessCallbacks WK2 API test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-18 21:51 PDT by Sergio Correia (qrwteyrutiyoup)
Modified: 2013-04-24 09:11 PDT (History)
10 users (show)

See Also:


Attachments
Patch (3.34 KB, patch)
2013-04-18 21:52 PDT, Sergio Correia (qrwteyrutiyoup)
no flags Details | Formatted Diff | Diff
Patch (6.89 KB, patch)
2013-04-22 21:39 PDT, Sergio Correia (qrwteyrutiyoup)
no flags Details | Formatted Diff | Diff
Patch (6.87 KB, patch)
2013-04-23 06:22 PDT, Sergio Correia (qrwteyrutiyoup)
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (914.33 KB, application/zip)
2013-04-23 17:11 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sergio Correia (qrwteyrutiyoup) 2013-04-18 21:51:08 PDT
[EFL][WK2]: Fix WKViewClientWebProcessCallbacks WK2 API test
Comment 1 Sergio Correia (qrwteyrutiyoup) 2013-04-18 21:52:41 PDT
Created attachment 198799 [details]
Patch

Proposed patch
Comment 2 Mikhail Pozdnyakov 2013-04-19 03:48:46 PDT
Comment on attachment 198799 [details]
Patch

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

Could we found another way to simulate a crash? maybe just send a signal since this is efl-specific test meaning that it runs on unix?

> Tools/TestWebKitAPI/Tests/WebKit2/efl/WKViewClientWebProcessCallbacks.cpp:-96
> -    viewClient.webProcessCrashed = webProcessCrashed;

that is removing the part of functionality that is to be tested.
Comment 3 Sergio Correia (qrwteyrutiyoup) 2013-04-22 20:48:55 PDT
(In reply to comment #2)
> (From update of attachment 198799 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=198799&action=review
> 
> Could we found another way to simulate a crash? maybe just send a signal since this is efl-specific test meaning that it runs on unix?

I'm testing an approach using an InjectedBundle to simulate the crash, so that we will still be able to test the crash callback. I am going to upload it soon, once I finish testing.
Comment 4 Sergio Correia (qrwteyrutiyoup) 2013-04-22 21:39:16 PDT
Created attachment 199148 [details]
Patch

Updated patch using InjectedBundle
Comment 5 Mikhail Pozdnyakov 2013-04-23 00:28:43 PDT
Comment on attachment 199148 [details]
Patch

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

Looks good!

> Tools/TestWebKitAPI/Tests/WebKit2/efl/WKViewClientWebProcessCallbacks_Bundle.cpp:47
> +void WKViewClientWebProcessCallbacksTest::didReceiveMessage(WKBundleRef bundle, WKStringRef messageName, WKTypeRef messageBody)

bundle and messageBody can be omitted.
Comment 6 Sergio Correia (qrwteyrutiyoup) 2013-04-23 06:22:24 PDT
Created attachment 199225 [details]
Patch

Addressed Mikhail's comment.
Comment 7 Build Bot 2013-04-23 17:11:35 PDT
Comment on attachment 199225 [details]
Patch

Attachment 199225 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/77252

New failing tests:
fast/repaint/japanese-rl-selection-repaint-in-regions.html
Comment 8 Build Bot 2013-04-23 17:11:39 PDT
Created attachment 199338 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2
Comment 9 Sergio Correia (qrwteyrutiyoup) 2013-04-24 06:20:37 PDT
(In reply to comment #8)
> Created an attachment (id=199338) [details]
> Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
> 
> The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
> Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2

Hmm. This does not seem to be related with the patch itself.

@Mikhail: Do you have any more remarks on this patch?
Comment 10 Mikhail Pozdnyakov 2013-04-24 06:27:26 PDT
Comment on attachment 199225 [details]
Patch

LGTM.
Comment 11 Sergio Correia (qrwteyrutiyoup) 2013-04-24 08:05:16 PDT
(In reply to comment #10)
> (From update of attachment 199225 [details])
> LGTM.

Thanks for your review. Do I need to get a r+ from a WK2 owner or something like that?
Comment 12 Mikhail Pozdnyakov 2013-04-24 08:21:38 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > (From update of attachment 199225 [details] [details])
> > LGTM.
> 
> Thanks for your review. Do I need to get a r+ from a WK2 owner or something like that?

yeah, I guess so.
Comment 13 Sergio Correia (qrwteyrutiyoup) 2013-04-24 08:29:59 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > (From update of attachment 199225 [details] [details] [details])
> > > LGTM.
> > 
> > Thanks for your review. Do I need to get a r+ from a WK2 owner or something like that?
> 
> yeah, I guess so.

Alright.
Comment 14 Bruno Abinader (history only) 2013-04-24 08:42:04 PDT
Comment on attachment 199225 [details]
Patch

The king has reviewed :)
Comment 15 WebKit Commit Bot 2013-04-24 09:10:57 PDT
Comment on attachment 199225 [details]
Patch

Clearing flags on attachment: 199225

Committed r149045: <http://trac.webkit.org/changeset/149045>
Comment 16 WebKit Commit Bot 2013-04-24 09:11:02 PDT
All reviewed patches have been landed.  Closing bug.