Bug 126401 - [iOS] Tapping any link crashes in WebCore::EventHandler::mouseMoved() (also crashes when scrolling certain sites)
Summary: [iOS] Tapping any link crashes in WebCore::EventHandler::mouseMoved() (also c...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-01-02 14:12 PST by Daniel Bates
Modified: 2014-01-06 14:23 PST (History)
1 user (show)

See Also:

Attachments
Patch (1.28 KB, patch)
2014-01-02 14:16 PST, Daniel Bates 		thorton: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2014-01-02 14:12:35 PST 
Tapping a hyperlink or scrolling a page causes a crash.
Comment 1 Daniel Bates 2014-01-02 14:12:55 PST 
<rdar://problem/15739334>
Comment 2 Daniel Bates 2014-01-02 14:16:34 PST 
Created attachment 220249 [details]
Patch
Comment 3 Daniel Bates 2014-01-02 14:28:39 PST 
Committed r161227: <http://trac.webkit.org/changeset/161227>
Comment 4 Joseph Pecoraro 2014-01-06 10:10:02 PST 
Comment on attachment 220249 [details]
Patch

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

> Source/WebCore/page/ios/EventHandlerIOS.mm:57
>  static RetainPtr<WebEvent>& currentEventSlot()
>  {
> -    NeverDestroyed<RetainPtr<WebEvent>> event;
> +    static NeverDestroyed<RetainPtr<WebEvent>> event;
>      return event;

Isn't a NeverDestroyed with a RetainPtr<> a bit overkill? Can we drop the RetainPtr wrapping?
Comment 5 Daniel Bates 2014-01-06 14:23:12 PST 
(In reply to comment #4)
> (From update of attachment 220249 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=220249&action=review
> 
> > Source/WebCore/page/ios/EventHandlerIOS.mm:57
> >  static RetainPtr<WebEvent>& currentEventSlot()
> >  {
> > -    NeverDestroyed<RetainPtr<WebEvent>> event;
> > +    static NeverDestroyed<RetainPtr<WebEvent>> event;
> >      return event;
> 
> Isn't a NeverDestroyed with a RetainPtr<> a bit overkill? Can we drop the RetainPtr wrapping?

As mentioned in person today (01/06), we cannot drop the RetainPtr<> without having the caller retain the WebEvent before assigning to currentEventSlot(). It seems error prone to depend on the caller to explicitly retain the WebEvent as opposed to letting RetainPtr<> implicitly retain it on assignment. I am open to suggestions on how to clean up this code.