Changeset 281668 in webkit

Timestamp:

Aug 26, 2021, 4:02:17 PM (5 years ago)

Author:

Wenson Hsieh

Message:

Add type-safe transaction identifiers for EditorState and FocusedElementInformation
​https://bugs.webkit.org/show_bug.cgi?id=229571

Reviewed by Tim Horton.

Make transactionID a monotonic object identifier instead of a generic TransactionID, and replace
FocusedElementIdentifier (which is currently type-defined to uint64_t) with its own monotonic object
identifier type.

In a future patch, I plan to implement a mechanism to synchronize EditorState and FocusedElementInformation
updates, which would require one or both of these objects to hold both types of transactional identifiers; in
order to do this, we need to ensure that these two identifiers are distinct types, so that they can't be easily
mixed up.

• Shared/EditorState.cpp:

(WebKit::EditorState::encode const):
(WebKit::EditorState::decode):

• Shared/EditorState.h:
• Shared/FocusedElementInformation.cpp:

(WebKit::FocusedElementInformation::encode const):
(WebKit::FocusedElementInformation::decode):

• Shared/FocusedElementInformation.h:
• Shared/IdentifierTypes.h:
• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::updateEditorState):

• UIProcess/ios/WKContentViewInteraction.mm:

(-[WKContentView updateCurrentFocusedElementInformation:]):

• WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::editorState const):
(WebKit::WebPage::elementDidFocus):

• WebProcess/WebPage/WebPage.h:

Additionally rename m_currentFocusedElementIdentifier to m_lastFocusedElementInformationIdentifier, for
consistency with m_lastEditorStateIdentifier.

• WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::focusedElementInformation):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Shared/EditorState.cpp (modified) (2 diffs)
• Shared/EditorState.h (modified) (2 diffs)
• Shared/FocusedElementInformation.cpp (modified) (2 diffs)
• Shared/FocusedElementInformation.h (modified) (3 diffs)
• Shared/IdentifierTypes.h (modified) (2 diffs)
• UIProcess/WebPageProxy.cpp (modified) (1 diff)
• UIProcess/ios/WKContentViewInteraction.mm (modified) (1 diff)
• WebProcess/WebPage/WebPage.cpp (modified) (2 diffs)
• WebProcess/WebPage/WebPage.h (modified) (2 diffs)
• WebProcess/WebPage/ios/WebPageIOS.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-08-26  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Add type-safe transaction identifiers for EditorState and FocusedElementInformation
+        https://bugs.webkit.org/show_bug.cgi?id=229571
+
+        Reviewed by Tim Horton.
+
+        Make `transactionID` a monotonic object identifier instead of a generic `TransactionID`, and replace
+        `FocusedElementIdentifier` (which is currently type-defined to `uint64_t`) with its own monotonic object
+        identifier type.
+
+        In a future patch, I plan to implement a mechanism to synchronize `EditorState` and `FocusedElementInformation`
+        updates, which would require one or both of these objects to hold both types of transactional identifiers; in
+        order to do this, we need to ensure that these two identifiers are distinct types, so that they can't be easily
+        mixed up.
+
+        * Shared/EditorState.cpp:
+        (WebKit::EditorState::encode const):
+        (WebKit::EditorState::decode):
+        * Shared/EditorState.h:
+        * Shared/FocusedElementInformation.cpp:
+        (WebKit::FocusedElementInformation::encode const):
+        (WebKit::FocusedElementInformation::decode):
+        * Shared/FocusedElementInformation.h:
+        * Shared/IdentifierTypes.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::updateEditorState):
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView updateCurrentFocusedElementInformation:]):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::editorState const):
+        (WebKit::WebPage::elementDidFocus):
+        * WebProcess/WebPage/WebPage.h:
+
+        Additionally rename `m_currentFocusedElementIdentifier` to `m_lastFocusedElementInformationIdentifier`, for
+        consistency with `m_lastEditorStateIdentifier`.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::focusedElementInformation):
+
 2021-08-26  Cameron McCormack  <heycam@apple.com>
 

trunk/Source/WebKit/Shared/EditorState.cpp

@@ -35 +35 @@
 void EditorState::encode(IPC::Encoder& encoder) const
 {
-    encoder << transactionID;
+    encoder << identifier;
     encoder << originIdentifierForPasteboard;
     encoder << shouldIgnoreSelectionChanges;
@@ -54 +54 @@
 bool EditorState::decode(IPC::Decoder& decoder, EditorState& result)
 {
-    if (!decoder.decode(result.transactionID))
+    if (!decoder.decode(result.identifier))
         return false;
 

trunk/Source/WebKit/Shared/EditorState.h

@@ -27 +27 @@
 
 #include "ArgumentCoders.h"
-#include "TransactionID.h"
+#include "IdentifierTypes.h"
 #include <WebCore/Color.h>
 #include <WebCore/FontAttributes.h>
@@ -71 +71 @@
 
 struct EditorState {
-    TransactionID transactionID;
+    EditorStateIdentifier identifier;
     String originIdentifierForPasteboard;
     bool shouldIgnoreSelectionChanges { false };

trunk/Source/WebKit/Shared/FocusedElementInformation.cpp

@@ -99 +99 @@
     encoder << label;
     encoder << ariaLabel;
-    encoder << focusedElementIdentifier;
+    encoder << identifier;
     encoder << containerScrollingNodeID;
 #if ENABLE(DATALIST_ELEMENT)
@@ -224 +224 @@
         return false;
 
-    if (!decoder.decode(result.focusedElementIdentifier))
+    if (!decoder.decode(result.identifier))
         return false;
 

trunk/Source/WebKit/Shared/FocusedElementInformation.h

@@ -27 +27 @@
 
 #include "ArgumentCoders.h"
+#include "IdentifierTypes.h"
 #include <WebCore/AutocapitalizeTypes.h>
 #include <WebCore/Autofill.h>
@@ -88 +89 @@
 };
 
-using FocusedElementIdentifier = uint64_t;
-
 struct FocusedElementInformation {
     WebCore::IntRect interactionRect;
@@ -141 +140 @@
     bool isFocusingWithValidationMessage { false };
 
-    FocusedElementIdentifier focusedElementIdentifier { 0 };
+    FocusedElementInformationIdentifier identifier;
     WebCore::ScrollingNodeID containerScrollingNodeID { 0 };
 

trunk/Source/WebKit/Shared/IdentifierTypes.h

@@ -26 +26 @@
 #pragma once
 
+#include "MonotonicObjectIdentifier.h"
 #include <wtf/ObjectIdentifier.h>
 
@@ -42 +43 @@
 using TextCheckerRequestID = ObjectIdentifier<TextCheckerRequestType>;
 
+enum EditorStateIdentifierType { };
+using EditorStateIdentifier = MonotonicObjectIdentifier<EditorStateIdentifierType>;
+
+enum FocusedElementInformationIdentifierType { };
+using FocusedElementInformationIdentifier = MonotonicObjectIdentifier<FocusedElementInformationIdentifierType>;
+
 } // namespace WebKit

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -7460 +7460 @@
 bool WebPageProxy::updateEditorState(const EditorState& newEditorState)
 {
-    if (newEditorState.transactionID < m_editorState.transactionID)
+    if (newEditorState.identifier < m_editorState.identifier)
         return false;
 

trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

@@ -6685 +6685 @@
 {
     WeakObjCPtr<WKContentView> weakSelf { self };
-    auto identifierBeforeUpdate = _focusedElementInformation.focusedElementIdentifier;
+    auto identifierBeforeUpdate = _focusedElementInformation.identifier;
     _page->requestFocusedElementInformation([callback = WTFMove(callback), identifierBeforeUpdate, weakSelf] (auto& info) {
-        if (!weakSelf || !info || info->focusedElementIdentifier != identifierBeforeUpdate) {
+        if (!weakSelf || !info || info->identifier != identifierBeforeUpdate) {
             // If the focused element may have changed in the meantime, don't overwrite focused element information.
             callback(false);

trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

@@ -1226 +1226 @@
     auto& editor = frame->editor();
 
-    result.transactionID = m_lastEditorStateTransactionID.increment();
+    result.identifier = m_lastEditorStateIdentifier.increment();
     result.selectionIsNone = selection.isNone();
     result.selectionIsRange = selection.isRange();
@@ -6032 +6032 @@
 #endif
 
-        ++m_currentFocusedElementIdentifier;
         auto information = focusedElementInformation();
         if (!information)

trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

@@ -2244 +2244 @@
     OptionSet<WebKit::WebEvent::Modifier> m_pendingSyntheticClickModifiers;
     WebCore::PointerID m_pendingSyntheticClickPointerId { 0 };
-    FocusedElementIdentifier m_currentFocusedElementIdentifier { 0 };
+    FocusedElementInformationIdentifier m_lastFocusedElementInformationIdentifier;
     std::optional<DynamicViewportSizeUpdateID> m_pendingDynamicViewportSizeUpdateID;
     double m_lastTransactionPageScaleFactor { 0 };
@@ -2283 +2283 @@
     enum class EditorStateIsContentEditable { No, Yes, Unset };
     mutable EditorStateIsContentEditable m_lastEditorStateWasContentEditable { EditorStateIsContentEditable::Unset };
-    mutable TransactionID m_lastEditorStateTransactionID;
+    mutable EditorStateIdentifier m_lastEditorStateIdentifier;
 
 #if PLATFORM(GTK) || PLATFORM(WPE)

trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

@@ -3227 +3227 @@
         information.hasPreviousNode = true;
     }
-    information.focusedElementIdentifier = m_currentFocusedElementIdentifier;
+    information.identifier = m_lastFocusedElementInformationIdentifier.increment();
 
     if (is<LabelableElement>(*focusedElement)) {