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

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.