Bug 68426 - [WEBKIT2]Crash on mouse hover in MiniBrowser
Summary: [WEBKIT2]Crash on mouse hover in MiniBrowser
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Nayan Kumar K
URL:
Keywords:
Depends on: 68470
Blocks: 68369
  Show dependency treegraph
 
Reported: 2011-09-19 22:03 PDT by Nayan Kumar K
Modified: 2011-09-26 14:59 PDT (History)
9 users (show)

See Also:


Attachments
Fix for MiniBrowser crash (1.41 KB, patch)
2011-09-19 22:09 PDT, Nayan Kumar K
no flags Details | Formatted Diff | Diff
WKHitTestResults (9.46 KB, patch)
2011-09-20 04:50 PDT, Nayan Kumar K
no flags Details | Formatted Diff | Diff
WKHitTestResult API's added for gtk, qt, win and mac ports (16.14 KB, patch)
2011-09-21 06:36 PDT, Nayan Kumar K
no flags Details | Formatted Diff | Diff
WKHitTestResult API and MiniBrowser crash fix (11.60 KB, patch)
2011-09-21 12:02 PDT, Nayan Kumar K
gustavo.noronha: commit-queue-
Details | Formatted Diff | Diff
WKHitTestResult API and Minibrowser crash fix (16.59 KB, patch)
2011-09-21 22:27 PDT, Nayan Kumar K
andersca: review-
Details | Formatted Diff | Diff
MiniBrowserCrashFix (16.96 KB, patch)
2011-09-22 08:51 PDT, Nayan Kumar K
andersca: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
WKHitTestResult API's (16.82 KB, patch)
2011-09-26 00:06 PDT, Nayan Kumar K
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nayan Kumar K 2011-09-19 22:03:54 PDT
Minibrowser is crashing with below backtrace in latest WebKit2 trunk,


[Current thread is 1 (Thread 0x7ffff7fb49a0 (LWP 7875))]
(gdb) bt
#0  0x0000000000404ba7 in mouseDidMoveOverElement ()
#1  0x00007ffff6b59b35 in WebKit::WebUIClient::mouseDidMoveOverElement(WebKit::WebPageProxy*, WebKit::WebHitTestResult::Data const&, WebKit::WebEvent::Modifiers, WebKit::APIObject*) ()
   from /home/nayankk/Sources/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
#2  0x00007ffff6b4810a in WebKit::WebPageProxy::mouseDidMoveOverElement(WebKit::WebHitTestResult::Data const&, unsigned int, CoreIPC::ArgumentDecoder*) ()
   from /home/nayankk/Sources/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
#3  0x00007ffff6bcfe15 in void CoreIPC::handleMessageVariadic<Messages::WebPageProxy::MouseDidMoveOverElement, WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(WebKit::WebHitTestResult::Data const&, unsigned int, CoreIPC::ArgumentDecoder*)>(CoreIPC::ArgumentDecoder*, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(WebKit::WebHitTestResult::Data const&, unsigned int, CoreIPC::ArgumentDecoder*)) () from /home/nayankk/Sources/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
#4  0x00007ffff6bcfbde in WebKit::WebPageProxy::didReceiveWebPageProxyMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) ()
   from /home/nayankk/Sources/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
#5  0x00007ffff6adaa99 in CoreIPC::Connection::dispatchMessage(CoreIPC::Connection::Message<CoreIPC::ArgumentDecoder>&) ()
   from /home/nayankk/Sources/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
#6  0x00007ffff6adabd1 in CoreIPC::Connection::dispatchMessages() () from /home/nayankk/Sources/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
#7  0x00007ffff6ae08a3 in RunLoop::performWork() () from /home/nayankk/Sources/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
#8  0x00007ffff6adf219 in RunLoop::queueWork(RunLoop*) () from /home/nayankk/Sources/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
#9  0x00007ffff38e0bcd in g_main_dispatch (context=0x4365c0) at /build/buildd/glib2.0-2.28.6/./glib/gmain.c:2440
#10 g_main_context_dispatch (context=0x4365c0) at /build/buildd/glib2.0-2.28.6/./glib/gmain.c:3013
#11 0x00007ffff38e13a8 in g_main_context_iterate (context=0x4365c0, block=<value optimized out>, dispatch=1, self=<value optimized out>) at /build/buildd/glib2.0-2.28.6/./glib/gmain.c:3091
#12 0x00007ffff38e19f2 in g_main_loop_run (loop=0x575e90) at /build/buildd/glib2.0-2.28.6/./glib/gmain.c:3299
#13 0x00007ffff61bed9d in gtk_main () from /usr/lib/libgtk-3.so.0
#14 0x0000000000405121 in main ()
Comment 1 Nayan Kumar K 2011-09-19 22:09:15 PDT
Created attachment 107967 [details]
Fix for MiniBrowser crash

Due to the mismatch in callback signature, clientInfo was becoming NULL, hence resulting in Minibrowser crash
Comment 2 Carlos Garcia Campos 2011-09-19 23:45:34 PDT
Comment on attachment 107967 [details]
Fix for MiniBrowser crash

hmm, API changed and now it includes the hit test result. It changes everything, since we were using injected bundle in minibrowser to get the url of the hit test result. We don't need that anymore, we can just use the hit test result object given in mouseDidMoveOverElement.
Comment 3 Nayan Kumar K 2011-09-20 04:50:50 PDT
Created attachment 107984 [details]
WKHitTestResults

Updated patch.

This patch adds functions to expose WebHitTestResult via WKHitTestResult.h. Minibrowser uses this new API to get the hover'ed element URL.

I am not sure whether these new files need to be compiled for all ports. For now, it compiles only on GTK port.
Comment 4 WebKit Review Bot 2011-09-20 13:16:04 PDT
Comment on attachment 107984 [details]
WKHitTestResults

Clearing flags on attachment: 107984

Committed r95565: <http://trac.webkit.org/changeset/95565>
Comment 5 WebKit Review Bot 2011-09-20 13:16:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 mitz 2011-09-20 13:52:07 PDT
(In reply to comment #4)
> (From update of attachment 107984 [details])
> Clearing flags on attachment: 107984
> 
> Committed r95565: <http://trac.webkit.org/changeset/95565>

This appears to have broken the build. An #include directive was added to WebKit2.h, but the corresponding header was not added to the WebKit2 project.
Comment 7 Anders Carlsson 2011-09-20 14:05:15 PDT
I had to back this out because it broke builds, it seems like the WKHitTestResult files weren't moved from Shared, and the relevant project files weren't updated.
Comment 8 Nayan Kumar K 2011-09-21 06:36:18 PDT
Created attachment 108144 [details]
WKHitTestResult API's added for gtk, qt, win and mac ports

Now this patch includes the compilation of WKHitTestResult API's for gtk, qt, mac and win ports. I have personally tested the patch against mac and gtk port, I hope it will work on others as well.
Comment 9 Nayan Kumar K 2011-09-21 12:02:30 PDT
Created attachment 108201 [details]
WKHitTestResult API and MiniBrowser crash fix
Comment 10 Darin Adler 2011-09-21 12:06:49 PDT
Comment on attachment 108201 [details]
WKHitTestResult API and MiniBrowser crash fix

Why do we need to add API for this? Safari does hover feedback without this API.
Comment 11 Nayan Kumar K 2011-09-21 12:08:15 PDT
I am not sure which patch is supposed to go in first! If 68369 goes it first, I can modify this patch to include just the fix for MiniBrowser crash in WebKit2-GTK (or visa-versa?)

Also, learnt from 68369 that, I missed adding files to CMake system, hence updated the patch adding WKHitTestResult.cpp to CMake.
Comment 12 Nayan Kumar K 2011-09-21 12:13:05 PDT
(In reply to comment #10)
> (From update of attachment 108201 [details])
> Why do we need to add API for this? Safari does hover feedback without this API.

We can get the hover'ed link without this API (WebKit2-Minibrowser currently does that). But, now that mouseDidMoveOverElement is changed to include WKHitTestResults, we thought it would be nice to get the hoever'ed element URL via WKHitTestResult (See comment https://bugs.webkit.org/show_bug.cgi?id=68426#c2 above)
Comment 13 Collabora GTK+ EWS bot 2011-09-21 14:49:56 PDT
Comment on attachment 108201 [details]
WKHitTestResult API and MiniBrowser crash fix

Attachment 108201 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9772894
Comment 14 Nayan Kumar K 2011-09-21 22:27:40 PDT
Created attachment 108277 [details]
WKHitTestResult API and Minibrowser crash fix

Oops.. Sorry, missed git adding newly created files in earlier patch
Comment 15 Anders Carlsson 2011-09-22 07:25:42 PDT
Comment on attachment 108277 [details]
WKHitTestResult API and Minibrowser crash fix

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

I think WKHitTestResult should be in the UIProcess/API/c directory. Otherwise the patch looks good. Please upload a new patch so we can land it.

> Tools/MiniBrowser/gtk/BrowserWindow.c:600
> +    WKURLRef linkUrlRef = WKHitTestResultCopyAbsoluteLinkURL(hitTestResult);

This return value should be released.
Comment 16 Nayan Kumar K 2011-09-22 08:51:18 PDT
Created attachment 108339 [details]
MiniBrowserCrashFix
Comment 17 Nayan Kumar K 2011-09-22 09:01:22 PDT
Updated the patch incorporating review comments, WKHitTestResult[h|cpp] is now moved to UIProcess/API/C folder. Please note that, I have added "Public" attribute to WKHitTestResult.h in project.pbxproj for mac port, so that this file gets copied to framework. I hope its the right way to do!
Comment 18 Anders Carlsson 2011-09-22 09:34:58 PDT
Comment on attachment 108339 [details]
MiniBrowserCrashFix

Much better. Will this break the mac build?
Comment 19 Ravi Phaneendra Kasibhatla 2011-09-22 09:39:55 PDT
(In reply to comment #18)
> (From update of attachment 108339 [details])
> Much better. Will this break the mac build?
Nayan has tried the mac build and it was successful, but still it would be good if we can run a mac-EWS on the patch.
Comment 20 WebKit Review Bot 2011-09-23 08:05:54 PDT
Comment on attachment 108339 [details]
MiniBrowserCrashFix

Rejecting attachment 108339 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
bKit2/WebKit2API.pri
Hunk #1 succeeded at 34 (offset 1 line).
Hunk #2 succeeded at 109 (offset 1 line).
patching file Source/WebKit2/win/WebKit2.vcproj
Hunk #1 succeeded at 3257 (offset 8 lines).
patching file Source/WebKit2/win/WebKit2Generated.make
patching file Tools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Tools/MiniBrowser/gtk/BrowserWindow.c

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Anders Carlsson', u'--..." exit_code: 1

Full output: http://queues.webkit.org/results/9818133
Comment 21 Nayan Kumar K 2011-09-26 00:06:22 PDT
Created attachment 108630 [details]
WKHitTestResult API's

Rebase'ed the patch to latest trunk.
Comment 22 WebKit Review Bot 2011-09-26 14:59:12 PDT
Comment on attachment 108630 [details]
WKHitTestResult API's

Clearing flags on attachment: 108630

Committed r95999: <http://trac.webkit.org/changeset/95999>
Comment 23 WebKit Review Bot 2011-09-26 14:59:19 PDT
All reviewed patches have been landed.  Closing bug.