Bug 126401

Summary: [iOS] Tapping any link crashes in WebCore::EventHandler::mouseMoved() (also crashes when scrolling certain sites)
Product: WebKit Reporter: Daniel Bates <dbates>
Component: UI EventsAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: adele
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch thorton: review+

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.