Bug 209506 - requestDocumentEditingContext() should bail out if the focused frame does not have a view
Summary: requestDocumentEditingContext() should bail out if the focused frame does not...
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad iOS 13
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-24 15:03 PDT by Daniel Bates
Modified: 2024-03-11 17:26 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.85 KB, patch)
2020-03-24 15:07 PDT, Daniel Bates
dbates: 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 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?