Bug 56645 - Assertion fires when hidden Find-on-Page matches are encountered in WebKit2
Summary: Assertion fires when hidden Find-on-Page matches are encountered in WebKit2
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: John Sullivan
URL: http://amazon.com
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-03-18 07:55 PDT by John Sullivan
Modified: 2011-03-18 08:31 PDT (History)
2 users (show)

See Also:


Attachments
Patch to bail out earlier for empty selection rect (1.58 KB, patch)
2011-03-18 07:59 PDT, John Sullivan
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Sullivan 2011-03-18 07:55:32 PDT
To reproduce:


1. visit amazon.com in a client using a debug build of WebKit2
2. use the Find-on-page UI to search for "the"
3. (If necessary), do Find Again a few times

An assertion fires:


ASSERTION FAILED: size

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   com.apple.WebKit2             	0x0000000100312b39 WebKit::SharedMemory::create(unsigned long) + 65 (SharedMemoryMac.cpp:94)
1   com.apple.WebKit2             	0x00000001003b58c7 WebKit::ShareableBitmap::createShareable(WebCore::IntSize const&) + 47 (ShareableBitmap.cpp:51)
2   com.apple.WebKit2             	0x000000010035b397 WebKit::FindController::updateFindIndicator(WebCore::Frame*, bool) + 323 (FindController.cpp:164)
3   com.apple.WebKit2             	0x000000010035b942 WebKit::FindController::findString(WTF::String const&, WebKit::FindOptions, unsigned int) + 378 (FindController.cpp:116)
4   com.apple.WebKit2             	0x000000010027d47c WebKit::WebPage::findString(WTF::String const&, unsigned int, unsigned int) + 48 (WebPage.cpp:1590)
<etc>

updateFindIndicator is computing an empty selection rect, and then trying to create a zero-sized ShareableBitmap, which hits the assertion in SharedMemory::create().

I've got a patch forthcoming.
Comment 1 John Sullivan 2011-03-18 07:59:31 PDT
Created attachment 86164 [details]
Patch to bail out earlier for empty selection rect
Comment 2 John Sullivan 2011-03-18 08:01:44 PDT
In radar as 9154276.
Comment 3 mitz 2011-03-18 08:03:45 PDT
I seem to remember that Jeff Miller fixed a bug like this. Was his fix Windows-specific? Can his change be reverted?
Comment 4 John Sullivan 2011-03-18 08:16:05 PDT
I found Jeff's fix -- it is <http://trac.webkit.org/changeset/77091>.

Jeff's fix prevented a crash after creating a zero-sized ShareableBitmap in this same updateFindIndicator function. This fix prevents an assertion while trying to create a zero-sized ShareableBitmap. svn blame reveals that the assertion was added after Jeff's fix, in 77968.

I think both tests are valid. It's good to bail out as soon as possible by testing for the empty selection rect (my fix). It's also good to bail out if the attempt to create a ShareableBitmap fails (perhaps there is some failure possibility other than zero-size-ness).
Comment 5 John Sullivan 2011-03-18 08:31:00 PDT
Fixed in <http://trac.webkit.org/changeset/81472>.