- Revision
- 282419
- Author
- [email protected]
- Date
- 2021-09-14 15:42:58 -0700 (Tue, 14 Sep 2021)
Log Message
[macOS] Avoid computing post-layout EditorState data unless necessary
https://bugs.webkit.org/show_bug.cgi?id=230204
<rdar://problem/83078713>
Reviewed by Devin Rousso.
Source/WebCore:
See WebKit/ChangeLog for more details.
* page/Page.h:
(WebCore::Page::isEditable const): Mark this getter `const`.
(WebCore::Page::isEditable): Deleted.
Source/WebKit:
Add a new codepath on macOS that allows us to entirely avoid post-layout EditorState computation in the case
where it's not needed for anything; in particular, unless the text touch bar may be presented or the UI delegate
implements the `-webView:didChangeFontAttributes:` method, the post-layout portion of EditorState is not needed
at all.
See below for more details.
* Shared/WebPageCreationParameters.cpp:
(WebKit::WebPageCreationParameters::encode const):
(WebKit::WebPageCreationParameters::decode):
* Shared/WebPageCreationParameters.h:
Plumb a bit indicating whether user interaction is required in order to show the text touch bar. See
`WebPage::shouldAvoidComputingPostLayoutDataForEditorState` below for more details.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::creationParameters):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::editorState const):
(WebKit::WebPage::updateEditorStateAfterLayoutIfEditabilityChanged):
When computing `EditorState.isContentEditable`, use VisibleSelection's `hasEditableStyle` method instead of
`isContentEditable`. Unlike the latter, the former does not trigger style resolution.
(WebKit::WebPage::didStartPageTransition):
Make a slight adjustment to only set the `m_hasEverFocusedElementDueToUserInteractionSincePageTransition` bit if
the user is actually interacting with text form control or editable element. This allows us to still avoid
computing post-layout editor state data in cases where the user has clicked on non-editable elements on the
page, such that text editing controls in the touch bar are still not being shown.
(WebKit::WebPage::didChangeSelectionOrOverflowScrollPosition):
(WebKit::WebPage::sendEditorStateUpdate):
Don't bother scheduling another post-layout EditorState in the case where post-layout data is missing, but we're
avoiding post-layout data computation altogether due to `shouldAvoidComputingPostLayoutDataForEditorState`
returning `true`.
* WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::requiresPostLayoutDataForEditorState const):
(WebKit::WebPage::shouldAvoidComputingPostLayoutDataForEditorState const):
(WebKit::WebPage::platformNeedsLayoutForEditorState const): Deleted.
Rename this to `requiresPostLayoutDataForEditorState`, to avoid some confusion with the new method,
`shouldAvoidComputingPostLayoutDataForEditorState`. The former is a way for platforms to indicate that post-
layout data *must* be included, even if layout is not up to date yet upon computing the EditorState. The latter
is a way for platforms to indicate that we can completely avoid all post-layout editor state computation,
regardless of whether or not layout is up to date.
By default, if both of the above methods return `false`, we compute and include post-layout data only if layout
is up-to-date when computing the EditorState, and schedule a subsequent "full" EditorState update after the
next rendering update in the case where layout was *not* up-to-date already (indicated by the EditorState's
`isMissingPostLayoutData` flag).
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::requiresPostLayoutDataForEditorState const):
(WebKit::WebPage::platformNeedsLayoutForEditorState const): Deleted.
* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::shouldAvoidComputingPostLayoutDataForEditorState const):
Only return `true` here if the user interaction requirements for showing text editing controls on the touch bar
have not been met, and the UI delegate method for observing font attributes is also not implemented by the
client.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (282418 => 282419)
--- trunk/Source/WebCore/ChangeLog 2021-09-14 22:41:23 UTC (rev 282418)
+++ trunk/Source/WebCore/ChangeLog 2021-09-14 22:42:58 UTC (rev 282419)
@@ -1,3 +1,17 @@
+2021-09-14 Wenson Hsieh <[email protected]>
+
+ [macOS] Avoid computing post-layout EditorState data unless necessary
+ https://bugs.webkit.org/show_bug.cgi?id=230204
+ <rdar://problem/83078713>
+
+ Reviewed by Devin Rousso.
+
+ See WebKit/ChangeLog for more details.
+
+ * page/Page.h:
+ (WebCore::Page::isEditable const): Mark this getter `const`.
+ (WebCore::Page::isEditable): Deleted.
+
2021-09-14 Ryan Haddad <[email protected]>
Unreviewed, reverting r282408.
Modified: trunk/Source/WebCore/page/Page.h (282418 => 282419)
--- trunk/Source/WebCore/page/Page.h 2021-09-14 22:41:23 UTC (rev 282418)
+++ trunk/Source/WebCore/page/Page.h 2021-09-14 22:42:58 UTC (rev 282419)
@@ -628,7 +628,7 @@
static const int maxNumberOfFrames = 1000;
void setEditable(bool isEditable) { m_isEditable = isEditable; }
- bool isEditable() { return m_isEditable; }
+ bool isEditable() const { return m_isEditable; }
WEBCORE_EXPORT VisibilityState visibilityState() const;
WEBCORE_EXPORT void resumeAnimatingImages();
Modified: trunk/Source/WebKit/ChangeLog (282418 => 282419)
--- trunk/Source/WebKit/ChangeLog 2021-09-14 22:41:23 UTC (rev 282418)
+++ trunk/Source/WebKit/ChangeLog 2021-09-14 22:42:58 UTC (rev 282419)
@@ -1,3 +1,75 @@
+2021-09-14 Wenson Hsieh <[email protected]>
+
+ [macOS] Avoid computing post-layout EditorState data unless necessary
+ https://bugs.webkit.org/show_bug.cgi?id=230204
+ <rdar://problem/83078713>
+
+ Reviewed by Devin Rousso.
+
+ Add a new codepath on macOS that allows us to entirely avoid post-layout EditorState computation in the case
+ where it's not needed for anything; in particular, unless the text touch bar may be presented or the UI delegate
+ implements the `-webView:didChangeFontAttributes:` method, the post-layout portion of EditorState is not needed
+ at all.
+
+ See below for more details.
+
+ * Shared/WebPageCreationParameters.cpp:
+ (WebKit::WebPageCreationParameters::encode const):
+ (WebKit::WebPageCreationParameters::decode):
+ * Shared/WebPageCreationParameters.h:
+
+ Plumb a bit indicating whether user interaction is required in order to show the text touch bar. See
+ `WebPage::shouldAvoidComputingPostLayoutDataForEditorState` below for more details.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::creationParameters):
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::editorState const):
+ (WebKit::WebPage::updateEditorStateAfterLayoutIfEditabilityChanged):
+
+ When computing `EditorState.isContentEditable`, use VisibleSelection's `hasEditableStyle` method instead of
+ `isContentEditable`. Unlike the latter, the former does not trigger style resolution.
+
+ (WebKit::WebPage::didStartPageTransition):
+
+ Make a slight adjustment to only set the `m_hasEverFocusedElementDueToUserInteractionSincePageTransition` bit if
+ the user is actually interacting with text form control or editable element. This allows us to still avoid
+ computing post-layout editor state data in cases where the user has clicked on non-editable elements on the
+ page, such that text editing controls in the touch bar are still not being shown.
+
+ (WebKit::WebPage::didChangeSelectionOrOverflowScrollPosition):
+ (WebKit::WebPage::sendEditorStateUpdate):
+
+ Don't bother scheduling another post-layout EditorState in the case where post-layout data is missing, but we're
+ avoiding post-layout data computation altogether due to `shouldAvoidComputingPostLayoutDataForEditorState`
+ returning `true`.
+
+ * WebProcess/WebPage/WebPage.h:
+ (WebKit::WebPage::requiresPostLayoutDataForEditorState const):
+ (WebKit::WebPage::shouldAvoidComputingPostLayoutDataForEditorState const):
+ (WebKit::WebPage::platformNeedsLayoutForEditorState const): Deleted.
+
+ Rename this to `requiresPostLayoutDataForEditorState`, to avoid some confusion with the new method,
+ `shouldAvoidComputingPostLayoutDataForEditorState`. The former is a way for platforms to indicate that post-
+ layout data *must* be included, even if layout is not up to date yet upon computing the EditorState. The latter
+ is a way for platforms to indicate that we can completely avoid all post-layout editor state computation,
+ regardless of whether or not layout is up to date.
+
+ By default, if both of the above methods return `false`, we compute and include post-layout data only if layout
+ is up-to-date when computing the EditorState, and schedule a subsequent "full" EditorState update after the
+ next rendering update in the case where layout was *not* up-to-date already (indicated by the EditorState's
+ `isMissingPostLayoutData` flag).
+
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::requiresPostLayoutDataForEditorState const):
+ (WebKit::WebPage::platformNeedsLayoutForEditorState const): Deleted.
+ * WebProcess/WebPage/mac/WebPageMac.mm:
+ (WebKit::WebPage::shouldAvoidComputingPostLayoutDataForEditorState const):
+
+ Only return `true` here if the user interaction requirements for showing text editing controls on the touch bar
+ have not been met, and the UI delegate method for observing font attributes is also not implemented by the
+ client.
+
2021-09-14 Fujii Hironori <[email protected]>
Enable IPCMessages debug logging for non-Cocoa ports
Modified: trunk/Source/WebKit/Shared/WebPageCreationParameters.cpp (282418 => 282419)
--- trunk/Source/WebKit/Shared/WebPageCreationParameters.cpp 2021-09-14 22:41:23 UTC (rev 282418)
+++ trunk/Source/WebKit/Shared/WebPageCreationParameters.cpp 2021-09-14 22:42:58 UTC (rev 282419)
@@ -189,6 +189,10 @@
#if ENABLE(APP_HIGHLIGHTS)
encoder << appHighlightsVisible;
#endif
+
+#if HAVE(TOUCH_BAR)
+ encoder << requiresUserActionForEditingControlsManager;
+#endif
}
std::optional<WebPageCreationParameters> WebPageCreationParameters::decode(IPC::Decoder& decoder)
@@ -608,6 +612,11 @@
return std::nullopt;
#endif
+#if HAVE(TOUCH_BAR)
+ if (!decoder.decode(parameters.requiresUserActionForEditingControlsManager))
+ return std::nullopt;
+#endif
+
return parameters;
}
Modified: trunk/Source/WebKit/Shared/WebPageCreationParameters.h (282418 => 282419)
--- trunk/Source/WebKit/Shared/WebPageCreationParameters.h 2021-09-14 22:41:23 UTC (rev 282418)
+++ trunk/Source/WebKit/Shared/WebPageCreationParameters.h 2021-09-14 22:42:58 UTC (rev 282419)
@@ -264,7 +264,10 @@
#if ENABLE(APP_HIGHLIGHTS)
WebCore::HighlightVisibility appHighlightsVisible { WebCore::HighlightVisibility::Hidden };
#endif
-
+
+#if HAVE(TOUCH_BAR)
+ bool requiresUserActionForEditingControlsManager { false };
+#endif
};
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (282418 => 282419)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2021-09-14 22:41:23 UTC (rev 282418)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2021-09-14 22:42:58 UTC (rev 282419)
@@ -8346,6 +8346,10 @@
parameters.appHighlightsVisible = appHighlightsVisibility() ? HighlightVisibility::Visible : HighlightVisibility::Hidden;
#endif
+#if HAVE(TOUCH_BAR)
+ parameters.requiresUserActionForEditingControlsManager = m_configuration->requiresUserActionForEditingControlsManager();
+#endif
+
return parameters;
}
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (282418 => 282419)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-09-14 22:41:23 UTC (rev 282418)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-09-14 22:42:58 UTC (rev 282419)
@@ -515,6 +515,9 @@
, m_pageScrolledHysteresis([this](PAL::HysteresisState state) { if (state == PAL::HysteresisState::Stopped) pageStoppedScrolling(); }, pageScrollHysteresisDuration)
, m_canRunBeforeUnloadConfirmPanel(parameters.canRunBeforeUnloadConfirmPanel)
, m_canRunModal(parameters.canRunModal)
+#if HAVE(TOUCH_BAR)
+ , m_requiresUserActionForEditingControlsManager(parameters.requiresUserActionForEditingControlsManager)
+#endif
#if ENABLE(META_VIEWPORT)
, m_forceAlwaysUserScalable(parameters.ignoresViewportScaleLimits)
#endif
@@ -1228,7 +1231,7 @@
result.identifier = m_lastEditorStateIdentifier.increment();
result.selectionIsNone = selection.isNone();
result.selectionIsRange = selection.isRange();
- result.isContentEditable = selection.isContentEditable();
+ result.isContentEditable = selection.hasEditableStyle();
result.isContentRichlyEditable = selection.isContentRichlyEditable();
result.isInPasswordField = selection.isInPasswordField();
result.hasComposition = editor.hasComposition();
@@ -1243,7 +1246,14 @@
result.selectionIsRangeInsideImageOverlay = selectionRange && HTMLElement::isInsideImageOverlay(*selectionRange);
}
- if (shouldPerformLayout == ShouldPerformLayout::Yes || platformNeedsLayoutForEditorState(frame))
+ m_lastEditorStateWasContentEditable = result.isContentEditable ? EditorStateIsContentEditable::Yes : EditorStateIsContentEditable::No;
+
+ if (shouldAvoidComputingPostLayoutDataForEditorState()) {
+ getPlatformEditorState(frame, result);
+ return result;
+ }
+
+ if (shouldPerformLayout == ShouldPerformLayout::Yes || requiresPostLayoutDataForEditorState(frame))
document->updateLayout(); // May cause document destruction
if (auto* frameView = document->view(); frameView && !frameView->needsLayout()) {
@@ -1260,8 +1270,6 @@
getPlatformEditorState(frame, result);
- m_lastEditorStateWasContentEditable = result.isContentEditable ? EditorStateIsContentEditable::Yes : EditorStateIsContentEditable::No;
-
return result;
}
@@ -1312,9 +1320,12 @@
if (m_lastEditorStateWasContentEditable == EditorStateIsContentEditable::Unset)
return;
+ if (hasPendingEditorStateUpdate())
+ return;
+
Ref frame = CheckedRef(m_page->focusController())->focusedOrMainFrame();
- EditorStateIsContentEditable editorStateIsContentEditable = frame->selection().selection().isContentEditable() ? EditorStateIsContentEditable::Yes : EditorStateIsContentEditable::No;
- if (m_lastEditorStateWasContentEditable != editorStateIsContentEditable)
+ auto isEditable = frame->selection().selection().hasEditableStyle() ? EditorStateIsContentEditable::Yes : EditorStateIsContentEditable::No;
+ if (m_lastEditorStateWasContentEditable != isEditable)
scheduleFullEditorStateUpdate();
}
@@ -3545,10 +3556,10 @@
{
freezeLayerTree(LayerTreeFreezeReason::PageTransition);
-#if PLATFORM(MAC)
+#if HAVE(TOUCH_BAR)
bool hasPreviouslyFocusedDueToUserInteraction = m_hasEverFocusedElementDueToUserInteractionSincePageTransition;
+ m_hasEverFocusedElementDueToUserInteractionSincePageTransition = false;
#endif
- m_hasEverFocusedElementDueToUserInteractionSincePageTransition = false;
m_lastEditorStateWasContentEditable = EditorStateIsContentEditable::Unset;
#if PLATFORM(MAC)
@@ -5962,12 +5973,12 @@
if (m_isSelectingTextWhileInsertingAsynchronously)
return;
-#if PLATFORM(MAC)
+#if HAVE(TOUCH_BAR)
bool hasPreviouslyFocusedDueToUserInteraction = m_hasEverFocusedElementDueToUserInteractionSincePageTransition;
- m_hasEverFocusedElementDueToUserInteractionSincePageTransition |= m_userIsInteracting;
+ if (m_userIsInteracting && m_focusedElement)
+ m_hasEverFocusedElementDueToUserInteractionSincePageTransition = true;
if (!hasPreviouslyFocusedDueToUserInteraction && m_hasEverFocusedElementDueToUserInteractionSincePageTransition) {
-#if HAVE(TOUCH_BAR)
if (frame->document()->quirks().isTouchBarUpdateSupressedForHiddenContentEditable()) {
m_isTouchBarUpdateSupressedForHiddenContentEditable = true;
send(Messages::WebPageProxy::SetIsTouchBarUpdateSupressedForHiddenContentEditable(m_isTouchBarUpdateSupressedForHiddenContentEditable));
@@ -5977,7 +5988,6 @@
m_isNeverRichlyEditableForTouchBar = true;
send(Messages::WebPageProxy::SetIsNeverRichlyEditableForTouchBar(m_isNeverRichlyEditableForTouchBar));
}
-#endif
send(Messages::WebPageProxy::SetHasHadSelectionChangesFromUserInteraction(m_hasEverFocusedElementDueToUserInteractionSincePageTransition));
}
@@ -5991,7 +6001,7 @@
discardedComposition();
return;
}
-#endif
+#endif // HAVE(TOUCH_BAR)
scheduleFullEditorStateUpdate();
}
@@ -6511,8 +6521,7 @@
// next layer tree commit to compute and send the complete EditorState over.
auto state = editorState();
send(Messages::WebPageProxy::EditorStateChanged(state));
-
- if (state.isMissingPostLayoutData)
+ if (state.isMissingPostLayoutData && !shouldAvoidComputingPostLayoutDataForEditorState())
scheduleFullEditorStateUpdate();
}
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (282418 => 282419)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-09-14 22:41:23 UTC (rev 282418)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-09-14 22:42:58 UTC (rev 282419)
@@ -1496,7 +1496,7 @@
void platformReinitialize();
void platformDetach();
void getPlatformEditorState(WebCore::Frame&, EditorState&) const;
- bool platformNeedsLayoutForEditorState(const WebCore::Frame&) const;
+ bool requiresPostLayoutDataForEditorState(const WebCore::Frame&) const;
void platformWillPerformEditingCommand();
void sendEditorStateUpdate();
void getPlatformEditorStateCommon(const WebCore::Frame&, EditorState&) const;
@@ -1920,6 +1920,7 @@
void consumeNetworkExtensionSandboxExtensions(const Vector<SandboxExtension::Handle>&);
bool hasPendingEditorStateUpdate() const;
+ bool shouldAvoidComputingPostLayoutDataForEditorState() const;
WebCore::PageIdentifier m_identifier;
@@ -2165,9 +2166,10 @@
std::optional<WebCore::IntSize> m_viewportSizeForCSSViewportUnits;
bool m_userIsInteracting { false };
- bool m_hasEverFocusedElementDueToUserInteractionSincePageTransition { false };
#if HAVE(TOUCH_BAR)
+ bool m_hasEverFocusedElementDueToUserInteractionSincePageTransition { false };
+ bool m_requiresUserActionForEditingControlsManager { false };
bool m_isTouchBarUpdateSupressedForHiddenContentEditable { false };
bool m_isNeverRichlyEditableForTouchBar { false };
#endif
@@ -2380,8 +2382,12 @@
#if !PLATFORM(IOS_FAMILY)
inline void WebPage::platformWillPerformEditingCommand() { }
-inline bool WebPage::platformNeedsLayoutForEditorState(const WebCore::Frame&) const { return false; }
+inline bool WebPage::requiresPostLayoutDataForEditorState(const WebCore::Frame&) const { return false; }
inline void WebPage::prepareToRunModalJavaScriptDialog() { }
#endif
+#if !PLATFORM(MAC)
+inline bool WebPage::shouldAvoidComputingPostLayoutDataForEditorState() const { return false; }
+#endif
+
} // namespace WebKit
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (282418 => 282419)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-09-14 22:41:23 UTC (rev 282418)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-09-14 22:42:58 UTC (rev 282419)
@@ -250,7 +250,7 @@
return renderer->hasNonEmptyVisibleRectRespectingParentFrames();
}
-bool WebPage::platformNeedsLayoutForEditorState(const Frame& frame) const
+bool WebPage::requiresPostLayoutDataForEditorState(const Frame& frame) const
{
// If we have a composition or are using a hardware keyboard then we need to send the full
// editor so that the UIProcess can update UI, including the position of the caret.
Modified: trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm (282418 => 282419)
--- trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm 2021-09-14 22:41:23 UTC (rev 282418)
+++ trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm 2021-09-14 22:42:58 UTC (rev 282419)
@@ -1084,6 +1084,21 @@
#endif
}
+bool WebPage::shouldAvoidComputingPostLayoutDataForEditorState() const
+{
+ if (m_needsFontAttributes) {
+ // Font attribute information is propagated to the UI process through post-layout data on EditorState.
+ return false;
+ }
+
+ if (!m_requiresUserActionForEditingControlsManager || m_hasEverFocusedElementDueToUserInteractionSincePageTransition) {
+ // Text editing controls on the touch bar depend on having post-layout editor state data.
+ return false;
+ }
+
+ return true;
+}
+
#if HAVE(APP_ACCENT_COLORS)
void WebPage::setAccentColor(WebCore::Color color)