- Revision
- 282615
- Author
- wenson_hs...@apple.com
- Date
- 2021-09-16 16:05:14 -0700 (Thu, 16 Sep 2021)
Log Message
Throttle a couple of editing-related timers in cases where the user has not interacted with subframes
https://bugs.webkit.org/show_bug.cgi?id=230326
Reviewed by Tim Horton.
This patch (1) delays the firing of a WebCore::Timer that is responsible for notifying the injected bundle about
newly inserted form controls, and (2) limits spellchecking and automatic text replacement to editing contexts in
documents that are either the main document, or have had user interaction or editing.
This gives us a small but measurable performance boost on Speedometer, where the DOM manipulations that occur
during the synchronous script execution phase currently schedule zero-delay WebCore::Timers via these two
codepaths, which then fire right after we begin counting time in the subsequent asynchronous phase, but before
that asynchronous phase has ended. This means that we're effectively penalized during the second async phase,
for timers that are scheduled during the first sync phase.
While most timers that are scheduled simply trigger work that we would've performed anyways when ensuring layout
near the end of the async phase (e.g. zero-delay style recalc timers and layout timers), these two timers -
`Editor::m_editorUIUpdateTimer` and `Document::m_didAssociateFormControlsTimer` - cause us to occasionally do
nontrivial work (that we would otherwise not have done) before ending the async phase.
Since these two timers are only used for AutoFill and text checking (respectively), it's likely that we can just
defer and avoid this work in these (relatively) narrow scenarios.
* dom/Document.cpp:
(WebCore::Document::commonTeardown):
When tearing down the document, additionally avoid triggering the associated form control timer by stopping the
timer and clearing out all the elements in the weak set.
(WebCore::Document::didAssociateFormControl):
(WebCore::Document::didAssociateFormControlsTimerFired):
Extend the delay to a second in the case of non-top-level documents that have not had any form of user
interaction.
* dom/Document.h:
* editing/Editor.cpp:
(WebCore::Editor::willApplyEditing):
Right before we're about to apply any edit command, set a bit indicating that the Editor has handled editing.
The editor UI update timer then consults this bit below, when determining whether or not it should schedule any
work.
(WebCore::Editor::respondToChangedSelection):
Additionally avoid repeatedly stopping and restarting `m_telephoneNumberDetectionUpdateTimer` when the DOM
selection changes, by making it a DeferrableOneShotTimer instead; this allows us to just set a bit on the timer
to reschedule it, instead of having to stop and restart every time.
(WebCore::Editor::willApplyEditing const): Deleted.
* editing/Editor.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (282614 => 282615)
--- trunk/Source/WebCore/ChangeLog 2021-09-16 23:04:30 UTC (rev 282614)
+++ trunk/Source/WebCore/ChangeLog 2021-09-16 23:05:14 UTC (rev 282615)
@@ -1,3 +1,57 @@
+2021-09-16 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Throttle a couple of editing-related timers in cases where the user has not interacted with subframes
+ https://bugs.webkit.org/show_bug.cgi?id=230326
+
+ Reviewed by Tim Horton.
+
+ This patch (1) delays the firing of a WebCore::Timer that is responsible for notifying the injected bundle about
+ newly inserted form controls, and (2) limits spellchecking and automatic text replacement to editing contexts in
+ documents that are either the main document, or have had user interaction or editing.
+
+ This gives us a small but measurable performance boost on Speedometer, where the DOM manipulations that occur
+ during the synchronous script execution phase currently schedule zero-delay WebCore::Timers via these two
+ codepaths, which then fire right after we begin counting time in the subsequent asynchronous phase, but before
+ that asynchronous phase has ended. This means that we're effectively penalized during the second async phase,
+ for timers that are scheduled during the first sync phase.
+
+ While most timers that are scheduled simply trigger work that we would've performed anyways when ensuring layout
+ near the end of the async phase (e.g. zero-delay style recalc timers and layout timers), these two timers -
+ `Editor::m_editorUIUpdateTimer` and `Document::m_didAssociateFormControlsTimer` - cause us to occasionally do
+ nontrivial work (that we would otherwise not have done) before ending the async phase.
+
+ Since these two timers are only used for AutoFill and text checking (respectively), it's likely that we can just
+ defer and avoid this work in these (relatively) narrow scenarios.
+
+ * dom/Document.cpp:
+ (WebCore::Document::commonTeardown):
+
+ When tearing down the document, additionally avoid triggering the associated form control timer by stopping the
+ timer and clearing out all the elements in the weak set.
+
+ (WebCore::Document::didAssociateFormControl):
+ (WebCore::Document::didAssociateFormControlsTimerFired):
+
+ Extend the delay to a second in the case of non-top-level documents that have not had any form of user
+ interaction.
+
+ * dom/Document.h:
+ * editing/Editor.cpp:
+ (WebCore::Editor::willApplyEditing):
+
+ Right before we're about to apply any edit command, set a bit indicating that the Editor has handled editing.
+ The editor UI update timer then consults this bit below, when determining whether or not it should schedule any
+ work.
+
+ (WebCore::Editor::respondToChangedSelection):
+
+ Additionally avoid repeatedly stopping and restarting `m_telephoneNumberDetectionUpdateTimer` when the DOM
+ selection changes, by making it a DeferrableOneShotTimer instead; this allows us to just set a bit on the timer
+ to reschedule it, instead of having to stop and restart every time.
+
+ (WebCore::Editor::willApplyEditing const): Deleted.
+ * editing/Editor.h:
+
2021-09-16 Chris Dumez <cdu...@apple.com>
ASSERTION FAILED: nsData under DataURLResourceMediaLoader::DataURLResourceMediaLoader()
Modified: trunk/Source/WebCore/dom/Document.cpp (282614 => 282615)
--- trunk/Source/WebCore/dom/Document.cpp 2021-09-16 23:04:30 UTC (rev 282614)
+++ trunk/Source/WebCore/dom/Document.cpp 2021-09-16 23:05:14 UTC (rev 282615)
@@ -870,6 +870,8 @@
m_timelinesController->detachFromDocument();
m_timeline = nullptr;
+ m_associatedFormControls.clear();
+ m_didAssociateFormControlsTimer.stop();
}
Element* Document::elementForAccessKey(const String& key)
@@ -7535,18 +7537,24 @@
auto* page = this->page();
if (!page || !page->chrome().client().shouldNotifyOnFormChanges())
return;
- m_associatedFormControls.add(&element);
- if (!m_didAssociateFormControlsTimer.isActive())
- m_didAssociateFormControlsTimer.startOneShot(0_s);
+
+ auto isNewEntry = m_associatedFormControls.add(&element).isNewEntry;
+ if (isNewEntry && !m_didAssociateFormControlsTimer.isActive())
+ m_didAssociateFormControlsTimer.startOneShot(isTopDocument() || hasHadUserInteraction() ? 0_s : 1_s);
}
void Document::didAssociateFormControlsTimerFired()
{
- auto vector = copyToVector(m_associatedFormControls);
- m_associatedFormControls.clear();
- if (auto* page = this->page()) {
+ Vector<RefPtr<Element>> controls;
+ controls.reserveInitialCapacity(m_associatedFormControls.computeSize());
+ for (auto& element : std::exchange(m_associatedFormControls, { })) {
+ if (element.isConnected())
+ controls.uncheckedAppend(&element);
+ }
+
+ if (auto page = this->page(); page && !controls.isEmpty()) {
ASSERT(m_frame);
- page->chrome().client().didAssociateFormControls(vector, *m_frame);
+ page->chrome().client().didAssociateFormControls(controls, *m_frame);
}
}
Modified: trunk/Source/WebCore/dom/Document.h (282614 => 282615)
--- trunk/Source/WebCore/dom/Document.h 2021-09-16 23:04:30 UTC (rev 282614)
+++ trunk/Source/WebCore/dom/Document.h 2021-09-16 23:05:14 UTC (rev 282615)
@@ -2040,7 +2040,7 @@
std::optional<WallTime> m_overrideLastModified;
- HashSet<RefPtr<Element>> m_associatedFormControls;
+ WeakHashSet<Element> m_associatedFormControls;
unsigned m_disabledFieldsetElementsCount { 0 };
unsigned m_dataListElementCount { 0 };
Modified: trunk/Source/WebCore/editing/Editor.cpp (282614 => 282615)
--- trunk/Source/WebCore/editing/Editor.cpp 2021-09-16 23:04:30 UTC (rev 282614)
+++ trunk/Source/WebCore/editing/Editor.cpp 2021-09-16 23:05:14 UTC (rev 282615)
@@ -1105,8 +1105,10 @@
dispatchInputEvent(*endRoot, inputTypeName, data, WTFMove(dataTransfer), targetRanges);
}
-bool Editor::willApplyEditing(CompositeEditCommand& command, Vector<RefPtr<StaticRange>>&& targetRanges) const
+bool Editor::willApplyEditing(CompositeEditCommand& command, Vector<RefPtr<StaticRange>>&& targetRanges)
{
+ m_hasHandledAnyEditing = true;
+
if (!command.shouldDispatchInputEvents())
return true;
@@ -1225,7 +1227,7 @@
, m_alternativeTextController(makeUnique<AlternativeTextController>(document))
, m_editorUIUpdateTimer(*this, &Editor::editorUIUpdateTimerFired)
#if ENABLE(TELEPHONE_NUMBER_DETECTION) && !PLATFORM(IOS_FAMILY)
- , m_telephoneNumberDetectionUpdateTimer(*this, &Editor::scanSelectionForTelephoneNumbers)
+ , m_telephoneNumberDetectionUpdateTimer(*this, &Editor::scanSelectionForTelephoneNumbers, 0_s)
#endif
{
}
@@ -3654,12 +3656,15 @@
#if ENABLE(TELEPHONE_NUMBER_DETECTION) && !PLATFORM(IOS_FAMILY)
if (shouldDetectTelephoneNumbers())
- m_telephoneNumberDetectionUpdateTimer.startOneShot(0_s);
+ m_telephoneNumberDetectionUpdateTimer.restart();
#endif
setStartNewKillRingSequence(true);
m_imageElementsToLoadBeforeRevealingSelection.clear();
+ if (!m_hasHandledAnyEditing && !m_document.hasHadUserInteraction() && !m_document.isTopDocument())
+ return;
+
if (m_editorUIUpdateTimer.isActive())
return;
Modified: trunk/Source/WebCore/editing/Editor.h (282614 => 282615)
--- trunk/Source/WebCore/editing/Editor.h 2021-09-16 23:04:30 UTC (rev 282614)
+++ trunk/Source/WebCore/editing/Editor.h 2021-09-16 23:05:14 UTC (rev 282615)
@@ -266,7 +266,7 @@
void applyParagraphStyleToSelection(StyleProperties*, EditAction);
// Returns whether or not we should proceed with editing.
- bool willApplyEditing(CompositeEditCommand&, Vector<RefPtr<StaticRange>>&&) const;
+ bool willApplyEditing(CompositeEditCommand&, Vector<RefPtr<StaticRange>>&&);
bool willUnapplyEditing(const EditCommandComposition&) const;
bool willReapplyEditing(const EditCommandComposition&) const;
@@ -672,7 +672,7 @@
#if ENABLE(TELEPHONE_NUMBER_DETECTION) && PLATFORM(MAC)
bool shouldDetectTelephoneNumbers() const;
- Timer m_telephoneNumberDetectionUpdateTimer;
+ DeferrableOneShotTimer m_telephoneNumberDetectionUpdateTimer;
Vector<SimpleRange> m_detectedTelephoneNumberRanges;
#endif
@@ -679,6 +679,7 @@
mutable std::unique_ptr<ScrollView::ProhibitScrollingWhenChangingContentSizeForScope> m_prohibitScrollingDueToContentSizeChangesWhileTyping;
bool m_isGettingDictionaryPopupInfo { false };
+ bool m_hasHandledAnyEditing { false };
HashSet<RefPtr<HTMLImageElement>> m_imageElementsToLoadBeforeRevealingSelection;
};
