Avoid keeping FormState alive longer than necessary
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 May 2018 05:23:00 +0000 (05:23 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 May 2018 05:23:00 +0000 (05:23 +0000)
commita6fb35432367a0db7830e94d016d4312dd1be569
tree91588fffa52bc70ab6ecaf3f401fba2415344728
parentdbc2f54bedff57c89e2336df1846dae0da8fd6cd
Avoid keeping FormState alive longer than necessary
https://bugs.webkit.org/show_bug.cgi?id=185877
<rdar://problem/39329219>

Reviewed by Ryosuke Niwa.

A number of crash fixes were done to prevent FormState objects from being
accessed after their relevant Frames had been destroyed. Unfortunately, this
could cause the FormState to persist after the owning Frame had been
destroyed, resulting in nullptr dereferences.

This patch does the following:

1. Uses WeakPtr's for FormState objects passed to completion handlers, rather
   than RefPtr, since those completion handlers might fire as part of the
   clean-up process during Frame destruction. This allows us to use the FormState
   if they are still valid, but gracefully handle cases where a form submission
   is cancelled in-flight.
2. Moves FormState object as they pass through the loader.
3. Removes some extraneous WTFMove() calls being made on bare FormState pointers.
4. Changes FormSubmission to hold a RefPtr so we can move the FormState to the
   loader in the code path that uses it (the FormSubmission is always destroyed
   shortly afterwards).
5. Changes the trap from Bug 183704 so that it only fires if the FormState object
   is being retained more than once.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::willSendRequest): Update for new CompletionHandler
signature.
* loader/FormState.cpp:
(WebCore::FormState::willDetachPage): Revise trap to check for retain counts
above one.
* loader/FormState.h:
(WebCore::FormState::weakPtrFactory const): Added.
* loader/FormSubmission.h:
(WebCore::FormSubmission::state const): Revised for change to RefPtr.
(WebCore::FormSubmission::takeState): Added.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::urlSelected): Update for new CompletionHandler signature.
(WebCore::FrameLoader::loadURLIntoChildFrame): Ditto.
(WebCore::FrameLoader::loadFrameRequest): Ditto.
(WebCore::FrameLoader::loadURL): Ditto.
(WebCore::FrameLoader::load): Ditto.
(WebCore::FrameLoader::loadWithNavigationAction): Ditto.
(WebCore::FrameLoader::loadWithDocumentLoader): Ditto.
(WebCore::FrameLoader::reloadWithOverrideEncoding): Ditto.
(WebCore::FrameLoader::reload): Ditto.
(WebCore::FrameLoader::loadPostRequest): Ditto.
(WebCore::FrameLoader::loadDifferentDocumentItem): Ditto.
* loader/FrameLoader.h:
* loader/NavigationScheduler.cpp:
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy):Revise to use WeakPtr for
FormState passed to the completion handler. Remove some extraneous WTFMove()
calls on bare pointers.
(WebCore::PolicyChecker::checkNewWindowPolicy): Ditto.
* loader/PolicyChecker.h:
* page/ContextMenuController.cpp:
(WebCore::openNewWindow): Revise for new signatures.
(WebCore::ContextMenuController::contextMenuItemSelected): Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@232147 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/FormState.cpp
Source/WebCore/loader/FormState.h
Source/WebCore/loader/FormSubmission.h
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/FrameLoader.h
Source/WebCore/loader/NavigationScheduler.cpp
Source/WebCore/loader/PolicyChecker.cpp
Source/WebCore/loader/PolicyChecker.h
Source/WebCore/page/ContextMenuController.cpp