Changeset 229683 in webkit

Timestamp:

Mar 16, 2018 2:17:27 PM (6 years ago)

Author:

Brent Fulgham

Message:

Set a trap to catch an infrequent form-related nullptr crash
​https://bugs.webkit.org/show_bug.cgi?id=183704
<rdar://problem/37579354>

Reviewed by Ryosuke Niwa.

Source/WebCore:

Make FormState a FrameDestructionObserver. We expect all relevant FormState objects to have been
cleaned up prior to the frame being destroyed. If we find such a case, we'd like to see the
stack trace to see what's going on.

• loader/FormState.cpp:

(WebCore::FormState::FormState):
(WebCore::FormState::willDetachPage): RELEASE_ASSERT_NOT_REACHED if we ever get here.

• loader/FormState.h:

Source/WebKit:

Add a RELEASE_ASSERT to see if we ever encounter a nullptr WebCore frame.

• WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:

(WebKit::WebFrameLoaderClient::dispatchWillSubmitForm):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/loader/FormState.cpp (modified) (2 diffs)
• WebCore/loader/FormState.h (modified) (3 diffs)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-03-16  Brent Fulgham  <bfulgham@apple.com>
+
+        Set a trap to catch an infrequent form-related nullptr crash
+        https://bugs.webkit.org/show_bug.cgi?id=183704
+        <rdar://problem/37579354>
+
+        Reviewed by Ryosuke Niwa.
+
+        Make FormState a FrameDestructionObserver. We expect all relevant FormState objects to have been
+        cleaned up prior to the frame being destroyed. If we find such a case, we'd like to see the
+        stack trace to see what's going on.
+
+        * loader/FormState.cpp:
+        (WebCore::FormState::FormState):
+        (WebCore::FormState::willDetachPage): RELEASE_ASSERT_NOT_REACHED if we ever get here.
+        * loader/FormState.h:
+
 2018-03-16  Joanmarie Diggs  <jdiggs@igalia.com>
 

trunk/Source/WebCore/loader/FormState.cpp

@@ -36 +36 @@
 
 inline FormState::FormState(HTMLFormElement& form, StringPairVector&& textFieldValues, Document& sourceDocument, FormSubmissionTrigger formSubmissionTrigger)
-    : m_form(form)
+    : FrameDestructionObserver(sourceDocument.frame())
+    , m_form(form)
     , m_textFieldValues(WTFMove(textFieldValues))
     , m_sourceDocument(sourceDocument)
     , m_formSubmissionTrigger(formSubmissionTrigger)
 {
+    RELEASE_ASSERT(sourceDocument.frame());
 }
 
@@ -48 +50 @@
 }
 
+void FormState::willDetachPage()
+{
+    // Beartrap for <rdar://problem/37579354>
+    RELEASE_ASSERT_NOT_REACHED();
 }
+
+}

trunk/Source/WebCore/loader/FormState.h

@@ -29 +29 @@
 #pragma once
 
+#include "FrameDestructionObserver.h"
 #include <wtf/text/WTFString.h>
 
@@ -40 +41 @@
 using StringPairVector = Vector<std::pair<String, String>>;
 
-class FormState : public RefCounted<FormState> {
+class FormState : public RefCounted<FormState>, public FrameDestructionObserver {
 public:
     static Ref<FormState> create(HTMLFormElement&, StringPairVector&& textFieldValues, Document&, FormSubmissionTrigger);
@@ -51 +52 @@
 private:
     FormState(HTMLFormElement&, StringPairVector&& textFieldValues, Document&, FormSubmissionTrigger);
+    void willDetachPage() override;
 
     Ref<HTMLFormElement> m_form;

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-03-16  Brent Fulgham  <bfulgham@apple.com>
+
+        Set a trap to catch an infrequent form-related nullptr crash
+        https://bugs.webkit.org/show_bug.cgi?id=183704
+        <rdar://problem/37579354>
+
+        Reviewed by Ryosuke Niwa.
+
+        Add a RELEASE_ASSERT to see if we ever encounter a nullptr WebCore frame.
+
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::dispatchWillSubmitForm):
+
 2018-03-16  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp

@@ -940 +940 @@
     auto& form = formState.form();
 
-    auto* sourceFrame = WebFrame::fromCoreFrame(*formState.sourceDocument().frame());
+    auto* sourceCoreFrame = formState.sourceDocument().frame();
+    RELEASE_ASSERT(sourceCoreFrame);
+    auto* sourceFrame = WebFrame::fromCoreFrame(*sourceCoreFrame);
     ASSERT(sourceFrame);