Bug 68426 – [WEBKIT2]Crash on mouse hover in MiniBrowser

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Nayan Kumar K]

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 ()

[Nayan Kumar K]

Created attachment 107967 [details]
Fix for MiniBrowser crash

Due to the mismatch in callback signature, clientInfo was becoming NULL, hence resulting in Minibrowser crash

[Carlos Garcia Campos]

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.

[Nayan Kumar K]

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.

[WebKit Review Bot]

Comment on attachment 107984 [details]
WKHitTestResults

Clearing flags on attachment: 107984

Committed r95565: <http://trac.webkit.org/changeset/95565>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

(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.

[Anders Carlsson]

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.

[Nayan Kumar K]

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.

[Nayan Kumar K]

Created attachment 108201 [details]
WKHitTestResult API and MiniBrowser crash fix

[Darin Adler]

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.

[Nayan Kumar K]

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.

[Nayan Kumar K]

(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)

[Collabora GTK+ EWS bot]

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

[Nayan Kumar K]

Created attachment 108277 [details]
WKHitTestResult API and Minibrowser crash fix

Oops.. Sorry, missed git adding newly created files in earlier patch

[Anders Carlsson]

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.

[Nayan Kumar K]

Created attachment 108339 [details]
MiniBrowserCrashFix

[Nayan Kumar K]

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!

[Anders Carlsson]

Comment on attachment 108339 [details]
MiniBrowserCrashFix

Much better. Will this break the mac build?

[Ravi Phaneendra Kasibhatla]

(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.

[WebKit Review Bot]

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

[Nayan Kumar K]

Created attachment 108630 [details]
WKHitTestResult API's

Rebase'ed the patch to latest trunk.

[WebKit Review Bot]

Comment on attachment 108630 [details]
WKHitTestResult API's

Clearing flags on attachment: 108630

Committed r95999: <http://trac.webkit.org/changeset/95999>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.