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:

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?

Daniel Bates

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

Attachments
Patch (1.85 KB, patch) 2020-03-24 15:07 PDT, Daniel Bates	dbates: review?	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2020-03-24 15:03:45 PDT

<rdar://problem/60843660>

Daniel Bates

Comment 2 2020-03-24 15:07:03 PDT

Created attachment 394419 [details] Patch

Darin Adler

Comment 3 2020-03-24 16:09:35 PDT

Comment on attachment 394419 [details] Patch Why no test case?

Daniel Bates

Comment 4 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.

Daniel Bates

Comment 5 2020-03-24 16:37:34 PDT

If there is an easier way to write a test I am all ears

Darin Adler

Comment 6 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.

Ahmad Saleem

Comment 7 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?

Note You need to log in before you can comment on or make changes to this bug.