Bug 126401 – [iOS] Tapping any link crashes in WebCore::EventHandler::mouseMoved() (also crashes when scrolling certain sites)

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

[Daniel Bates]

Tapping a hyperlink or scrolling a page causes a crash.

[Daniel Bates]

<rdar://problem/15739334>

[Daniel Bates]

Created attachment 220249 [details]
Patch

[Daniel Bates]

Committed r161227: <http://trac.webkit.org/changeset/161227>

[Joseph Pecoraro]

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?

[Daniel Bates]

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