Bug 114850

Summary: [EFL][WK2]: Fix WKViewClientWebProcessCallbacks WK2 API test
Product: WebKit Reporter: Sergio Correia (qrwteyrutiyoup) <sergio>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, benjamin, buildbot, commit-queue, gyuyoung.kim, kenneth, kling, mikhail.pozdnyakov, rakuco, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 none

Sergio Correia (qrwteyrutiyoup)
Reported 2013-04-18 21:51:08 PDT
[EFL][WK2]: Fix WKViewClientWebProcessCallbacks WK2 API test
Attachments
Patch (3.34 KB, patch)
2013-04-18 21:52 PDT, Sergio Correia (qrwteyrutiyoup)
no flags
Patch (6.89 KB, patch)
2013-04-22 21:39 PDT, Sergio Correia (qrwteyrutiyoup)
no flags
Patch (6.87 KB, patch)
2013-04-23 06:22 PDT, Sergio Correia (qrwteyrutiyoup)
no flags
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
Sergio Correia (qrwteyrutiyoup)
Comment 1 2013-04-18 21:52:41 PDT
Created attachment 198799 [details] Patch Proposed patch
Mikhail Pozdnyakov
Comment 2 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.
Sergio Correia (qrwteyrutiyoup)
Comment 3 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.
Sergio Correia (qrwteyrutiyoup)
Comment 4 2013-04-22 21:39:16 PDT
Created attachment 199148 [details] Patch Updated patch using InjectedBundle
Mikhail Pozdnyakov
Comment 5 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.
Sergio Correia (qrwteyrutiyoup)
Comment 6 2013-04-23 06:22:24 PDT
Created attachment 199225 [details] Patch Addressed Mikhail's comment.
Build Bot
Comment 7 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
Build Bot
Comment 8 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
Sergio Correia (qrwteyrutiyoup)
Comment 9 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?
Mikhail Pozdnyakov
Comment 10 2013-04-24 06:27:26 PDT
Comment on attachment 199225 [details] Patch LGTM.
Sergio Correia (qrwteyrutiyoup)
Comment 11 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?
Mikhail Pozdnyakov
Comment 12 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.
Sergio Correia (qrwteyrutiyoup)
Comment 13 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.
Bruno Abinader (history only)
Comment 14 2013-04-24 08:42:04 PDT
Comment on attachment 199225 [details] Patch The king has reviewed :)
WebKit Commit Bot
Comment 15 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>
WebKit Commit Bot
Comment 16 2013-04-24 09:11:02 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.