- Revision
- 282940
- Author
- repst...@apple.com
- Date
- 2021-09-22 22:14:17 -0700 (Wed, 22 Sep 2021)
Log Message
Cherry-pick r282615. rdar://problem/83429940
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:
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282615 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-612-branch/Source/WebCore/ChangeLog (282939 => 282940)
--- branches/safari-612-branch/Source/WebCore/ChangeLog 2021-09-23 05:14:14 UTC (rev 282939)
+++ branches/safari-612-branch/Source/WebCore/ChangeLog 2021-09-23 05:14:17 UTC (rev 282940)
@@ -1,5 +1,118 @@
2021-09-22 Alan Coon <alanc...@apple.com>
+ Cherry-pick r282615. rdar://problem/83429940
+
+ 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:
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282615 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 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-22 Alan Coon <alanc...@apple.com>
+
Cherry-pick r282369. rdar://problem/83429914
AX: set insertion point to the end of a native text control does not work when passing a collapsed TextMarkerRange with both start and end equals to the end TextMarker for the element.
Modified: branches/safari-612-branch/Source/WebCore/dom/Document.cpp (282939 => 282940)
--- branches/safari-612-branch/Source/WebCore/dom/Document.cpp 2021-09-23 05:14:14 UTC (rev 282939)
+++ branches/safari-612-branch/Source/WebCore/dom/Document.cpp 2021-09-23 05:14:17 UTC (rev 282940)
@@ -873,6 +873,8 @@
m_timelinesController->detachFromDocument();
m_timeline = nullptr;
+ m_associatedFormControls.clear();
+ m_didAssociateFormControlsTimer.stop();
}
Element* Document::elementForAccessKey(const String& key)
@@ -7534,18 +7536,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: branches/safari-612-branch/Source/WebCore/dom/Document.h (282939 => 282940)
--- branches/safari-612-branch/Source/WebCore/dom/Document.h 2021-09-23 05:14:14 UTC (rev 282939)
+++ branches/safari-612-branch/Source/WebCore/dom/Document.h 2021-09-23 05:14:17 UTC (rev 282940)
@@ -2031,7 +2031,7 @@
std::optional<WallTime> m_overrideLastModified;
- HashSet<RefPtr<Element>> m_associatedFormControls;
+ WeakHashSet<Element> m_associatedFormControls;
unsigned m_disabledFieldsetElementsCount { 0 };
unsigned m_dataListElementCount { 0 };
Modified: branches/safari-612-branch/Source/WebCore/editing/Editor.cpp (282939 => 282940)
--- branches/safari-612-branch/Source/WebCore/editing/Editor.cpp 2021-09-23 05:14:14 UTC (rev 282939)
+++ branches/safari-612-branch/Source/WebCore/editing/Editor.cpp 2021-09-23 05:14:17 UTC (rev 282940)
@@ -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: branches/safari-612-branch/Source/WebCore/editing/Editor.h (282939 => 282940)
--- branches/safari-612-branch/Source/WebCore/editing/Editor.h 2021-09-23 05:14:14 UTC (rev 282939)
+++ branches/safari-612-branch/Source/WebCore/editing/Editor.h 2021-09-23 05:14:17 UTC (rev 282940)
@@ -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;
};
