Bug 190801

Summary:

Don't waste time under -setupInteraction under -initWithFrame for unparented WKWebViews

Product:

WebKit

Reporter:

Tim Horton <thorton>

Component:

New Bugs

Assignee:

Tim Horton <thorton>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bdakin, commit-queue, megan_gardner, ryanhaddad, webkit-bug-importer, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none

Description Tim Horton 2018-10-22 12:49:50 PDT

Don't waste time under -setupInteraction under -initWithFrame for unparented WKWebViews

Comment 1 Tim Horton 2018-10-22 12:50:11 PDT

Created attachment 352902 [details]
Patch

Comment 2 Tim Horton 2018-10-22 12:50:13 PDT

<rdar://problem/43674361>

Comment 3 Megan Gardner 2018-10-22 12:52:19 PDT

Comment on attachment 352902 [details]
Patch

Assuming all bots pass

Comment 4 WebKit Commit Bot 2018-10-22 14:22:45 PDT

Comment on attachment 352902 [details]
Patch

Clearing flags on attachment: 352902

Committed r237331: <https://trac.webkit.org/changeset/237331>

Comment 5 WebKit Commit Bot 2018-10-22 14:22:46 PDT

All reviewed patches have been landed.  Closing bug.

Comment 6 Ryan Haddad 2018-10-22 15:59:24 PDT

(In reply to WebKit Commit Bot from comment #4)
> Committed r237331: <https://trac.webkit.org/changeset/237331>

This appears to have caused API test failures on iOS:

Failed

    TestWebKitAPI.WebKit.InteractionDeadlockAfterCrash
        
        /Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/InteractionDeadlockAfterCrash.mm:70
        Value of: highlightLongPressRecognizer
          Actual: false
        Expected: true
        
        
        /Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/InteractionDeadlockAfterCrash.mm:74
        Value of: shouldBegin
          Actual: false
        Expected: true
        

Timeout

    TestWebKitAPI.ActionSheetTests.DismissingActionSheetShouldNotDismissPresentingViewController

https://build.webkit.org/builders/Apple%20iOS%2012%20Simulator%20Release%20WK2%20(Tests)/builds/491/steps/run-api-tests/logs/stdio

Comment 7 Tim Horton 2018-10-22 16:34:32 PDT

Technically a good change. I’ll fix it (by parenting the view).

Comment 8 Tim Horton 2018-10-22 18:52:26 PDT

(In reply to Tim Horton from comment #7)
> Technically a good change. I’ll fix it (by parenting the view).

Ryan, https://trac.webkit.org/changeset/237346/webkit should do the trick.

Comment 9 Ryan Haddad 2018-10-23 08:54:43 PDT

(In reply to Tim Horton from comment #8)
> (In reply to Tim Horton from comment #7)
> > Technically a good change. I’ll fix it (by parenting the view).
> 
> Ryan, https://trac.webkit.org/changeset/237346/webkit should do the trick.
That did indeed! We are still seeing TestWebKitAPI.ActionSheetTests.DismissingActionSheetShouldNotDismissPresentingViewController time out, though. Sorry for not making it clear that both were caused by this change.

Comment 10 Tim Horton 2018-10-23 11:52:58 PDT

(In reply to Ryan Haddad from comment #9)
> (In reply to Tim Horton from comment #8)
> > (In reply to Tim Horton from comment #7)
> > > Technically a good change. I’ll fix it (by parenting the view).
> > 
> > Ryan, https://trac.webkit.org/changeset/237346/webkit should do the trick.
> That did indeed! We are still seeing
> TestWebKitAPI.ActionSheetTests.
> DismissingActionSheetShouldNotDismissPresentingViewController time out,
> though. Sorry for not making it clear that both were caused by this change.

Interesting! I'll take a peek.

Comment 11 Tim Horton 2018-10-23 13:10:44 PDT

(In reply to Tim Horton from comment #10)
> (In reply to Ryan Haddad from comment #9)
> > (In reply to Tim Horton from comment #8)
> > > (In reply to Tim Horton from comment #7)
> > > > Technically a good change. I’ll fix it (by parenting the view).
> > > 
> > > Ryan, https://trac.webkit.org/changeset/237346/webkit should do the trick.
> > That did indeed! We are still seeing
> > TestWebKitAPI.ActionSheetTests.
> > DismissingActionSheetShouldNotDismissPresentingViewController time out,
> > though. Sorry for not making it clear that both were caused by this change.
> 
> Interesting! I'll take a peek.

Very similar problem, the view isn't in-window. Not sure yet exactly why it's not, since from code inspection it looks like it should be, but it's possible we just have to wait for a transition.