Changeset 280718 in webkit

Timestamp:

Aug 5, 2021, 8:10:40 PM (5 years ago)

Author:

Cameron McCormack

Message:

Stop tracking form elements with FormController
​https://bugs.webkit.org/show_bug.cgi?id=228724
<rdar://problem/81435095>

Reviewed by Darin Adler.

Source/WebCore:

FormController currently tracks the insertion order of
HTMLFormElementWithState objects in the document. But we don't need
to know this list of form controls until the time we need to save
form state for the document (e.g. on pagehide). So we instead
traverse the document to find those elements at the point we need
them, rather than maintain FormController::m_formElementsWithState.

This is a small speedup (1-2%) on a few of the Speedometer subtests
that insert and remove many input elements.

A future optimization could record on the Document whether there are
any input elements that have had their value changed, since it's
probably common for pages with form controls to never be changed.

• dom/Document.cpp:

(WebCore::Document::formElementsState const): Traverse the document to
find all the HTMLFormElementWithState objects.

• dom/Element.h:

(WebCore::Element::isFormControlElementWithState const):

• html/FormController.cpp:

(WebCore::FormController::createSavedFormStateMap):
(WebCore::FormController::formElementsState const):

• html/FormController.h:
• html/HTMLFormControlElementWithState.cpp:

(WebCore::HTMLFormControlElementWithState::insertedIntoAncestor):
Track the order that HTMLFormControlElementWithState objects are
inserted into the document.
(WebCore::HTMLFormControlElementWithState::removedFromAncestor):

• html/HTMLFormControlElementWithState.h:

(WebCore::HTMLFormControlElementWithState::insertionIndex const):
(isType):

• page/Frame.h: Remove unused formElementsCharacterCount.
• page/ios/FrameIOS.mm:

Source/WebKitLegacy/mac:

formElementsCharacterCount is unused and can be removed.

• WebView/WebFrame.mm:
• WebView/WebFramePrivate.h:

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/dom/Document.cpp (modified) (1 diff)
• WebCore/dom/Element.h (modified) (1 diff)
• WebCore/html/FormController.cpp (modified) (3 diffs)
• WebCore/html/FormController.h (modified) (2 diffs)
• WebCore/html/HTMLFormControlElementWithState.cpp (modified) (3 diffs)
• WebCore/html/HTMLFormControlElementWithState.h (modified) (3 diffs)
• WebCore/page/Frame.h (modified) (1 diff)
• WebCore/page/ios/FrameIOS.mm (modified) (1 diff)
• WebKitLegacy/mac/ChangeLog (modified) (1 diff)
• WebKitLegacy/mac/WebView/WebFrame.mm (modified) (1 diff)
• WebKitLegacy/mac/WebView/WebFramePrivate.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-08-05  Cameron McCormack  <heycam@apple.com>
+
+        Stop tracking form elements with FormController
+        https://bugs.webkit.org/show_bug.cgi?id=228724
+        <rdar://problem/81435095>
+
+        Reviewed by Darin Adler.
+
+        FormController currently tracks the insertion order of
+        HTMLFormElementWithState objects in the document.  But we don't need
+        to know this list of form controls until the time we need to save
+        form state for the document (e.g. on pagehide).  So we instead
+        traverse the document to find those elements at the point we need
+        them, rather than maintain FormController::m_formElementsWithState.
+
+        This is a small speedup (1-2%) on a few of the Speedometer subtests
+        that insert and remove many input elements.
+
+        A future optimization could record on the Document whether there are
+        any input elements that have had their value changed, since it's
+        probably common for pages with form controls to never be changed.
+
+        * dom/Document.cpp:
+        (WebCore::Document::formElementsState const): Traverse the document to
+        find all the HTMLFormElementWithState objects.
+        * dom/Element.h:
+        (WebCore::Element::isFormControlElementWithState const):
+        * html/FormController.cpp:
+        (WebCore::FormController::createSavedFormStateMap):
+        (WebCore::FormController::formElementsState const):
+        * html/FormController.h:
+        * html/HTMLFormControlElementWithState.cpp:
+        (WebCore::HTMLFormControlElementWithState::insertedIntoAncestor):
+        Track the order that HTMLFormControlElementWithState objects are
+        inserted into the document.
+        (WebCore::HTMLFormControlElementWithState::removedFromAncestor):
+        * html/HTMLFormControlElementWithState.h:
+        (WebCore::HTMLFormControlElementWithState::insertionIndex const):
+        (isType):
+        * page/Frame.h: Remove unused formElementsCharacterCount.
+        * page/ios/FrameIOS.mm:
+
 2021-08-05  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -1910 +1910 @@
     if (!m_formController)
         return Vector<String>();
-    return m_formController->formElementsState();
+    return m_formController->formElementsState(*this);
 }
 

trunk/Source/WebCore/dom/Element.h

@@ -469 +469 @@
 
     virtual bool isFormControlElement() const { return false; }
+    virtual bool isFormControlElementWithState() const { return false; }
     virtual bool isSpinButtonElement() const { return false; }
     virtual bool isTextFormControlElement() const { return false; }

trunk/Source/WebCore/html/FormController.cpp

@@ -343 +343 @@
 FormController::~FormController() = default;
 
-unsigned FormController::formElementsCharacterCount() const
-{
-    unsigned count = 0;
-    for (auto& element : m_formElementsWithState) {
-        if (element->isTextField())
-            count += element->saveFormControlState()[0].length();
-    }
-    return count;
-}
-
 static String formStateSignature()
 {
@@ -362 +352 @@
 }
 
-std::unique_ptr<FormController::SavedFormStateMap> FormController::createSavedFormStateMap(const FormElementListHashSet& controlList)
+std::unique_ptr<FormController::SavedFormStateMap> FormController::createSavedFormStateMap(const FormControlVector& controlList)
 {
     FormKeyGenerator keyGenerator;
     auto stateMap = makeUnique<SavedFormStateMap>();
-    for (auto& control : controlList) {
-        if (!control->shouldSaveAndRestoreFormControlState())
-            continue;
-        auto& formState = stateMap->add(keyGenerator.formKey(*control).impl(), nullptr).iterator->value;
+    for (const HTMLFormControlElementWithState& control : controlList) {
+        if (!control.shouldSaveAndRestoreFormControlState())
+            continue;
+        auto& formState = stateMap->add(keyGenerator.formKey(control).impl(), nullptr).iterator->value;
         if (!formState)
             formState = makeUnique<SavedFormState>();
-        formState->appendControlState(control->name(), control->type(), control->saveFormControlState());
+        formState->appendControlState(control.name(), control.type(), control.saveFormControlState());
     }
     return stateMap;
 }
 
-Vector<String> FormController::formElementsState() const
-{
-    std::unique_ptr<SavedFormStateMap> stateMap = createSavedFormStateMap(m_formElementsWithState);
+Vector<String> FormController::formElementsState(const Document& document) const
+{
+    // FIXME: We should be saving the state of form controls in shadow trees, too.
+    FormControlVector controls;
+    for (auto& control : descendantsOfType<HTMLFormControlElementWithState>(document)) {
+        ASSERT(control.insertionIndex());
+        controls.append(control);
+    }
+
+    std::sort(controls.begin(), controls.end(), [](auto a, auto b) {
+        return a.get().insertionIndex() < b.get().insertionIndex();
+    });
+
+    auto stateMap = createSavedFormStateMap(controls);
     Vector<String> stateVector;
-    stateVector.reserveInitialCapacity(m_formElementsWithState.size() * 4);
+    stateVector.reserveInitialCapacity(controls.size() * 4);
     stateVector.append(formStateSignature());
     for (auto& state : *stateMap) {
@@ -485 +486 @@
 }
 
-void FormController::registerFormElementWithState(HTMLFormControlElementWithState& control)
-{
-    ASSERT(!m_formElementsWithState.contains(&control));
-    m_formElementsWithState.add(&control);
-}
-
-void FormController::unregisterFormElementWithState(HTMLFormControlElementWithState& control)
-{
-    ASSERT(m_formElementsWithState.contains(&control));
-    m_formElementsWithState.remove(&control);
-}
-
 } // namespace WebCore

trunk/Source/WebCore/html/FormController.h

@@ -43 +43 @@
     ~FormController();
 
-    void registerFormElementWithState(HTMLFormControlElementWithState&);
-    void unregisterFormElementWithState(HTMLFormControlElementWithState&);
-
-    unsigned formElementsCharacterCount() const;
-
-    Vector<String> formElementsState() const;
+    Vector<String> formElementsState(const Document&) const;
     void setStateForNewFormElements(const Vector<String>&);
 
@@ -59 +54 @@
 
 private:
-    typedef ListHashSet<RefPtr<HTMLFormControlElementWithState>> FormElementListHashSet;
-    typedef HashMap<RefPtr<AtomStringImpl>, std::unique_ptr<SavedFormState>> SavedFormStateMap;
+    using FormControlVector = Vector<std::reference_wrapper<const HTMLFormControlElementWithState>>;
+    using SavedFormStateMap = HashMap<RefPtr<AtomStringImpl>, std::unique_ptr<SavedFormState>>;
 
-    static std::unique_ptr<SavedFormStateMap> createSavedFormStateMap(const FormElementListHashSet&);
+    static std::unique_ptr<SavedFormStateMap> createSavedFormStateMap(const FormControlVector&);
     FormControlState takeStateForFormElement(const HTMLFormControlElementWithState&);
     static void formStatesFromStateVector(const Vector<String>&, SavedFormStateMap&);
 
-    FormElementListHashSet m_formElementsWithState;
     SavedFormStateMap m_savedFormStateMap;
     std::unique_ptr<FormKeyGenerator> m_formKeyGenerator;

trunk/Source/WebCore/html/HTMLFormControlElementWithState.cpp

@@ -44 +44 @@
 Node::InsertedIntoAncestorResult HTMLFormControlElementWithState::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
 {
-    if (insertionType.connectedToDocument && !containingShadowRoot())
-        document().formController().registerFormElementWithState(*this);
+    m_insertionIndex = ++lastInsertionIndex;
     return HTMLFormControlElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
 }
@@ -51 +50 @@
 void HTMLFormControlElementWithState::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
 {
-    if (removalType.disconnectedFromDocument && !containingShadowRoot() && !oldParentOfRemovedTree.containingShadowRoot())
-        document().formController().unregisterFormElementWithState(*this);
+    m_insertionIndex = 0;
     HTMLFormControlElement::removedFromAncestor(removalType, oldParentOfRemovedTree);
 }
@@ -85 +83 @@
 }
 
+uint64_t HTMLFormControlElementWithState::lastInsertionIndex { 0 };
+
 } // namespace Webcore

trunk/Source/WebCore/html/HTMLFormControlElementWithState.h

@@ -39 +39 @@
     virtual void restoreFormControlState(const FormControlState&) { } // Called only if state is not empty.
 
+    uint64_t insertionIndex() const { return m_insertionIndex; }
+
 protected:
     HTMLFormControlElementWithState(const QualifiedName& tagName, Document&, HTMLFormElement*);
@@ -51 +53 @@
 private:
     bool isFormControlElementWithState() const final;
+
+    uint64_t m_insertionIndex { 0 };
+    static uint64_t lastInsertionIndex;
 };
 
@@ -56 +61 @@
 
 SPECIALIZE_TYPE_TRAITS_BEGIN(WebCore::HTMLFormControlElementWithState)
+    static bool isType(const WebCore::Element& element) { return element.isFormControlElementWithState(); }
+    static bool isType(const WebCore::Node& node) { return is<WebCore::Element>(node) && isType(downcast<WebCore::Element>(node)); }
     static bool isType(const WebCore::FormAssociatedElement& element) { return element.isFormControlElementWithState(); }
 SPECIALIZE_TYPE_TRAITS_END()

trunk/Source/WebCore/page/Frame.h

@@ -283 +283 @@
     WEBCORE_EXPORT NSRect caretRect();
     WEBCORE_EXPORT NSRect rectForScrollToVisible();
-    WEBCORE_EXPORT unsigned formElementsCharacterCount() const;
 
     // This function is used by Legacy WebKit.

trunk/Source/WebCore/page/ios/FrameIOS.mm

@@ -628 +628 @@
 }
 
-unsigned Frame::formElementsCharacterCount() const
-{
-    Document* document = this->document();
-    if (!document)
-        return 0;
-    return document->formController().formElementsCharacterCount();
-}
-
 void Frame::setTimersPaused(bool paused)
 {

trunk/Source/WebKitLegacy/mac/ChangeLog

@@ +1 @@
+2021-08-05  Cameron McCormack  <heycam@apple.com>
+
+        Stop tracking form elements with FormController
+        https://bugs.webkit.org/show_bug.cgi?id=228724
+        <rdar://problem/81435095>
+
+        Reviewed by Darin Adler.
+
+        formElementsCharacterCount is unused and can be removed.
+
+        * WebView/WebFrame.mm:
+        * WebView/WebFramePrivate.h:
+
 2021-07-29  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebKitLegacy/mac/WebView/WebFrame.mm

@@ -1164 +1164 @@
 #if PLATFORM(IOS_FAMILY)
 
-- (unsigned)formElementsCharacterCount
-{
-    return core(self)->formElementsCharacterCount();
-}
-
 - (void)setTimeoutsPaused:(BOOL)flag
 {

trunk/Source/WebKitLegacy/mac/WebView/WebFramePrivate.h

@@ -96 +96 @@
 - (void)_setLoadsSynchronously:(BOOL)flag;
 - (BOOL)_loadsSynchronously;
-- (unsigned)formElementsCharacterCount;
 - (void)setTimeoutsPaused:(BOOL)flag;