Bug 57294 - Crash when closing "Add Bookmark" Dialog using the Enter Key
Summary: Crash when closing "Add Bookmark" Dialog using the Enter Key
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Brian Weinstein
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-03-28 19:31 PDT by Brian Weinstein
Modified: 2011-03-30 12:58 PDT (History)
2 users (show)

See Also:


Attachments
[PATCH] Fix (1.32 KB, patch)
2011-03-28 19:33 PDT, Brian Weinstein
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Weinstein 2011-03-28 19:31:13 PDT
If a WebView is set to close using the Enter Key (if it was a dialog that closes when you press enter), it can crash when closed this way.

The FrameView isn't protected in EventHandler::keyEvent, like it is in EventHandler::handleMousePressEvent, EventHandler::handleMouseDoubleClickEvent, EventHandler::handleMouseMoveEvent, EventHandler::handleMouseReleaseEvent, and EventHandler::handleWheelEvent.

<rdar://problem/9044756>
Comment 1 Brian Weinstein 2011-03-28 19:33:39 PDT
Created attachment 87249 [details]
[PATCH] Fix
Comment 2 Darin Adler 2011-03-28 20:05:50 PDT
Comment on attachment 87249 [details]
[PATCH] Fix

It’s not at all clear why we protect the FrameView and not some other reference-counted object.
Comment 3 Brian Weinstein 2011-03-28 20:15:01 PDT
(In reply to comment #2)
> (From update of attachment 87249 [details])
> It’s not at all clear why we protect the FrameView and not some other reference-counted object.

I'm not sure either. I was going by existing style w/ other functions in the class + it fixed the crash. I can talk to someone about it later tonight or in the morning before landing.
Comment 4 Adam Roben (:aroben) 2011-03-29 04:49:51 PDT
Comment on attachment 87249 [details]
[PATCH] Fix

Is it possible to make a test for this? If not a test for run-webkit-tests, then maybe one for run-api-tests?
Comment 5 Brian Weinstein 2011-03-29 14:01:54 PDT
(In reply to comment #4)
> (From update of attachment 87249 [details])
> Is it possible to make a test for this? If not a test for run-webkit-tests, then maybe one for run-api-tests?

I tried making a test that opens a window using window.open, and that window is closed by any key-press, but that didn't reproduce the crash.
Comment 6 Alexey Proskuryakov 2011-03-29 14:05:46 PDT
> I tried making a test that opens a window using window.open, and that window is closed by any key-press, but that didn't reproduce the crash.

Not sure if it's worth spending more time on this, but the inability to make a test suggests that there is some general difference between normal web views and those the problem was observed with, and that difference would be good to find and eliminate.
Comment 7 Brian Weinstein 2011-03-30 12:45:37 PDT
Adding some additional steps to reproduce:

1) Go to any page (http://reddit.com - was used for testing).
2) Click the + button in the URL field, to add a bookmark
3) Press Enter to close the "Add Bookmark" Dialog
Comment 8 Brian Weinstein 2011-03-30 12:58:49 PDT
Landed in r82487.