Bug 209506

Summary: requestDocumentEditingContext() should bail out if the focused frame does not have a view
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebKit Misc.Assignee: Daniel Bates <dbates>
Status: ASSIGNED ---    
Severity: Normal CC: achristensen, ahmad.saleem792, darin, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: iPhone / iPad   
OS: iOS 13   
See Also: https://bugs.webkit.org/show_bug.cgi?id=209493
Attachments:
Description Flags
Patch dbates: review?

Description Daniel Bates 2020-03-24 15:03:35 PDT 
requestDocumentEditingContext() performs a layout, which can do anything, including document destruction. Ensure that the focused frame has a view before computing the editing context.
Comment 1 Radar WebKit Bug Importer 2020-03-24 15:03:45 PDT 
<rdar://problem/60843660>
Comment 2 Daniel Bates 2020-03-24 15:07:03 PDT 
Created attachment 394419 [details]
Patch
Comment 3 Darin Adler 2020-03-24 16:09:35 PDT 
Comment on attachment 394419 [details]
Patch

Why no test case?
Comment 4 Daniel Bates 2020-03-24 16:36:56 PDT 
(In reply to Darin Adler from comment #3)
> Comment on attachment 394419 [details]
> Patch
> 
> Why no test case?

It is hard to write one.

I don't even know how to write such a test. I mean, here's what I need to do:

1. Write a test that will run JavaScript on layout (so, probably need a resize observer) that performs a synchronous navigation.

2. Write an API test that loads that page, get the page into a state where layout is needed, but **not** performed, then call -requestDocumentContext: (again, hoping that layout timer has not run before message gets to web process; otherwise, need to disable that).

Then -requestDocumentContext will message to requestDocumentEditingContext(), which will update layout => triggers navigation => view destroyed and the test will crash without the patch and not crash with it.
Comment 5 Daniel Bates 2020-03-24 16:37:34 PDT 
If there is an easier way to write a test I am all ears
Comment 6 Darin Adler 2020-03-24 17:10:08 PDT 
That sounds like the right way to write the test. We should really write it. Otherwise we are just making code changes and crossing our fingers.
Comment 7 Ahmad Saleem 2024-03-11 17:26:44 PDT 
This patch was modifying:

https://searchfox.org/wubkat/rev/711120e7edec012527620d07bf63d85713a180fd/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm#4935

Which is:



    RefPtr frame = m_page->checkedFocusController()->focusedOrMainFrame();
    if (!frame)
        return completionHandler({ });

__

So I think 275171@main addressed this in bug 269761 and rdar://116201648.

@Alex - above commit was from your side, do you think we need to do anything more here or we can tag this as 'Duplicate' to above bug?