Bug 1863611 Make EventStateManager send content command events to focused remote process r=smaug

Currently, `EventStateManager` handles some content commands only in the parent
process.  Therefore, if `eSetSelection` event or something is being handled in
the focused remote process, the command may be disabled yet.

Avoiding the race conditions, we should handle the content command events in the
focused process.

Differential Revision: https://phabricator.services.mozilla.com/D222789

dom/events/EventStateManager.cpp

@@ -6747,24 +6747,31 @@ nsresult EventStateManager::DoContentCommandEvent(
   nsCOMPtr<nsPIWindowRoot> root = window->GetTopWindowRoot();
   NS_ENSURE_TRUE(root, NS_ERROR_FAILURE);
   const char* cmd;
+  bool maybeNeedToHandleInRemote = false;
   switch (aEvent->mMessage) {
     case eContentCommandCut:
       cmd = "cmd_cut";
+      maybeNeedToHandleInRemote = true;
       break;
     case eContentCommandCopy:
       cmd = "cmd_copy";
+      maybeNeedToHandleInRemote = true;
       break;
     case eContentCommandPaste:
       cmd = "cmd_paste";
+      maybeNeedToHandleInRemote = true;
       break;
     case eContentCommandDelete:
       cmd = "cmd_delete";
+      maybeNeedToHandleInRemote = true;
       break;
     case eContentCommandUndo:
       cmd = "cmd_undo";
+      maybeNeedToHandleInRemote = true;
       break;
     case eContentCommandRedo:
       cmd = "cmd_redo";
+      maybeNeedToHandleInRemote = true;
       break;
     case eContentCommandPasteTransferable:
       cmd = "cmd_pasteTransferable";
@@ -6775,6 +6782,20 @@ nsresult EventStateManager::DoContentCommandEvent(
     default:
       return NS_ERROR_NOT_IMPLEMENTED;
   }
+  if (XRE_IsParentProcess() && maybeNeedToHandleInRemote) {
+    if (BrowserParent* remote = BrowserParent::GetFocused()) {
+      if (!aEvent->mOnlyEnabledCheck) {
+        remote->SendSimpleContentCommandEvent(*aEvent);
+      }
+      // XXX The command may be disabled in the parent process.  Perhaps, we
+      // should set actual enabled state in the parent process here and there
+      // should be another bool flag which indicates whether the content is sent
+      // to a remote process.
+      aEvent->mIsEnabled = true;
+      aEvent->mSucceeded = true;
+      return NS_OK;
+    }
+  }
   // If user tries to do something, user must try to do it in visible window.
   // So, let's retrieve controller of visible window.
   nsCOMPtr<nsIController> controller;
@@ -6864,8 +6885,12 @@ nsresult EventStateManager::DoContentCommandInsertTextEvent(
   if (XRE_IsParentProcess()) {
     // Handle it in focused content process if there is.
     if (BrowserParent* remote = BrowserParent::GetFocused()) {
-      remote->SendInsertText(aEvent->mString.ref());
+      if (!aEvent->mOnlyEnabledCheck) {
-      aEvent->mIsEnabled = true;  // XXX it can be a lie...
+        remote->SendInsertText(*aEvent);
+      }
+      // XXX The remote process may be not editable right now.  Therefore, this
+      // may be different from actual state in the remote process.
+      aEvent->mIsEnabled = true;
       aEvent->mSucceeded = true;
       return NS_OK;
     }
@@ -6901,10 +6926,12 @@ nsresult EventStateManager::DoContentCommandReplaceTextEvent(
   if (XRE_IsParentProcess()) {
     // Handle it in focused content process if there is.
     if (BrowserParent* remote = BrowserParent::GetFocused()) {
-      Unused << remote->SendReplaceText(
+      if (!aEvent->mOnlyEnabledCheck) {
-          aEvent->mSelection.mReplaceSrcString, aEvent->mString.ref(),
+        Unused << remote->SendReplaceText(*aEvent);
-          aEvent->mSelection.mOffset, aEvent->mSelection.mPreventSetSelection);
+      }
-      aEvent->mIsEnabled = true;  // XXX it can be a lie...
+      // XXX The remote process may be not editable right now.  Therefore, this
+      // may be different from actual state in the remote process.
+      aEvent->mIsEnabled = true;
       aEvent->mSucceeded = true;
       return NS_OK;
     }

dom/ipc/BrowserChild.cpp

@@ -2312,6 +2312,20 @@ mozilla::ipc::IPCResult BrowserChild::RecvNormalPrioritySelectionEvent(
   return RecvSelectionEvent(aEvent);
 }
 
+mozilla::ipc::IPCResult BrowserChild::RecvSimpleContentCommandEvent(
+    const EventMessage& aMessage) {
+  WidgetContentCommandEvent localEvent(true, aMessage, mPuppetWidget);
+  DispatchWidgetEventViaAPZ(localEvent);
+  Unused << SendOnEventNeedingAckHandled(aMessage, 0u);
+  return IPC_OK();
+}
+
+mozilla::ipc::IPCResult
+BrowserChild::RecvNormalPrioritySimpleContentCommandEvent(
+    const EventMessage& aMessage) {
+  return RecvSimpleContentCommandEvent(aMessage);
+}
+
 mozilla::ipc::IPCResult BrowserChild::RecvInsertText(
     const nsAString& aStringToInsert) {
   // Use normal event path to reach focused document.

dom/ipc/BrowserChild.h

@@ -400,6 +400,12 @@ class BrowserChild final : public nsMessageManagerScriptExecutor,
   mozilla::ipc::IPCResult RecvNormalPrioritySelectionEvent(
       const mozilla::WidgetSelectionEvent& aEvent);
 
+  mozilla::ipc::IPCResult RecvSimpleContentCommandEvent(
+      const mozilla::EventMessage& aMessage);
+
+  mozilla::ipc::IPCResult RecvNormalPrioritySimpleContentCommandEvent(
+      const mozilla::EventMessage& aMessage);
+
   mozilla::ipc::IPCResult RecvInsertText(const nsAString& aStringToInsert);
 
   mozilla::ipc::IPCResult RecvUpdateRemoteStyle(

dom/ipc/BrowserParent.cpp

@@ -3294,13 +3294,49 @@ bool BrowserParent::SendSelectionEvent(WidgetSelectionEvent& aEvent) {
   return true;
 }
 
-bool BrowserParent::SendInsertText(const nsString& aStringToInsert) {
+bool BrowserParent::SendSimpleContentCommandEvent(
+    const mozilla::WidgetContentCommandEvent& aEvent) {
+  MOZ_ASSERT(aEvent.mMessage != eContentCommandInsertText);
+  MOZ_ASSERT(aEvent.mMessage != eContentCommandReplaceText);
+  MOZ_ASSERT(aEvent.mMessage != eContentCommandPasteTransferable);
+  MOZ_ASSERT(aEvent.mMessage != eContentCommandLookUpDictionary);
+  MOZ_ASSERT(aEvent.mMessage != eContentCommandScroll);
+
   if (mIsDestroyed) {
     return false;
   }
+  mContentCache.OnContentCommandEvent(aEvent);
   return Manager()->IsInputPriorityEventEnabled()
-             ? PBrowserParent::SendInsertText(aStringToInsert)
+             ? PBrowserParent::SendSimpleContentCommandEvent(aEvent.mMessage)
-             : PBrowserParent::SendNormalPriorityInsertText(aStringToInsert);
+             : PBrowserParent::SendNormalPrioritySimpleContentCommandEvent(
+                   aEvent.mMessage);
+}
+
+bool BrowserParent::SendInsertText(const WidgetContentCommandEvent& aEvent) {
+  if (mIsDestroyed) {
+    return false;
+  }
+  mContentCache.OnContentCommandEvent(aEvent);
+  return Manager()->IsInputPriorityEventEnabled()
+             ? PBrowserParent::SendInsertText(aEvent.mString.ref())
+             : PBrowserParent::SendNormalPriorityInsertText(
+                   aEvent.mString.ref());
+}
+
+bool BrowserParent::SendReplaceText(const WidgetContentCommandEvent& aEvent) {
+  if (mIsDestroyed) {
+    return false;
+  }
+  mContentCache.OnContentCommandEvent(aEvent);
+  return Manager()->IsInputPriorityEventEnabled()
+             ? PBrowserParent::SendReplaceText(
+                   aEvent.mSelection.mReplaceSrcString, aEvent.mString.ref(),
+                   aEvent.mSelection.mOffset,
+                   aEvent.mSelection.mPreventSetSelection)
+             : PBrowserParent::SendNormalPriorityReplaceText(
+                   aEvent.mSelection.mReplaceSrcString, aEvent.mString.ref(),
+                   aEvent.mSelection.mOffset,
+                   aEvent.mSelection.mPreventSetSelection);
 }
 
 bool BrowserParent::SendPasteTransferable(IPCTransferable&& aTransferable) {

dom/ipc/BrowserParent.h

@@ -613,7 +613,10 @@ class BrowserParent final : public PBrowserParent,
 
   bool HandleQueryContentEvent(mozilla::WidgetQueryContentEvent& aEvent);
 
-  bool SendInsertText(const nsString& aStringToInsert);
+  bool SendSimpleContentCommandEvent(
+      const mozilla::WidgetContentCommandEvent& aEvent);
+  bool SendInsertText(const mozilla::WidgetContentCommandEvent& aEvent);
+  bool SendReplaceText(const mozilla::WidgetContentCommandEvent& aEvent);
 
   bool SendPasteTransferable(IPCTransferable&& aTransferable);
 

dom/ipc/PBrowser.ipdl

@@ -849,6 +849,10 @@ child:
     [Priority=input] async SelectionEvent(WidgetSelectionEvent event);
     async NormalPrioritySelectionEvent(WidgetSelectionEvent event);
 
+
+    [Priority=input] async SimpleContentCommandEvent(EventMessage message);
+    async NormalPrioritySimpleContentCommandEvent(EventMessage message);
+
     /**
      * Dispatch eContentCommandInsertText event in the remote process.
      */

widget/ContentCache.cpp

@@ -16,6 +16,7 @@
 #include "mozilla/IMEStateManager.h"
 #include "mozilla/IntegerPrintfMacros.h"
 #include "mozilla/Logging.h"
+#include "mozilla/MiscEvents.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/TextComposition.h"
 #include "mozilla/dom/BrowserParent.h"
@@ -1419,6 +1420,28 @@ void ContentCacheInParent::OnSelectionEvent(
   mPendingSetSelectionEventNeedingAck++;
 }
 
+void ContentCacheInParent::OnContentCommandEvent(
+    const WidgetContentCommandEvent& aContentCommandEvent) {
+  MOZ_LOG(sContentCacheLog, LogLevel::Info,
+          ("0x%p OnContentCommandEvent(aEvent={ "
+           "mMessage=%s, mString=\"%s\", mSelection={ mReplaceSrcString=\"%s\" "
+           "mOffset=%u, mPreventSetSelection=%s }, mOnlyEnabledCheck=%s })",
+           this, ToChar(aContentCommandEvent.mMessage),
+           ToString(aContentCommandEvent.mString).c_str(),
+           ToString(aContentCommandEvent.mSelection.mReplaceSrcString).c_str(),
+           aContentCommandEvent.mSelection.mOffset,
+           GetBoolName(aContentCommandEvent.mSelection.mPreventSetSelection),
+           GetBoolName(aContentCommandEvent.mOnlyEnabledCheck)));
+
+  MOZ_ASSERT(!aContentCommandEvent.mOnlyEnabledCheck);
+
+#if MOZ_DIAGNOSTIC_ASSERT_ENABLED && !defined(FUZZING_SNAPSHOT)
+  mDispatchedEventMessages.AppendElement(aContentCommandEvent.mMessage);
+#endif  // MOZ_DIAGNOSTIC_ASSERT_ENABLED
+
+  mPendingContentCommandEventNeedingAck++;
+}
+
 void ContentCacheInParent::OnEventNeedingAckHandled(nsIWidget* aWidget,
                                                     EventMessage aMessage,
                                                     uint32_t aCompositionId) {
@@ -1588,6 +1611,21 @@ void ContentCacheInParent::OnEventNeedingAckHandled(nsIWidget* aWidget,
     } else {
       mPendingSetSelectionEventNeedingAck--;
     }
+  } else if (aMessage >= eContentCommandEventFirst &&
+             aMessage <= eContentCommandEventLast) {
+    if (NS_WARN_IF(!mPendingContentCommandEventNeedingAck)) {
+#if MOZ_DIAGNOSTIC_ASSERT_ENABLED && !defined(FUZZING_SNAPSHOT)
+      nsAutoCString info(
+          "\nThere is no pending content command events but received from the "
+          "remote child\n\n");
+      AppendEventMessageLog(info);
+      CrashReporter::AppendAppNotesToCrashReport(info);
+      MOZ_DIAGNOSTIC_ASSERT(
+          false, "No pending event message but received unexpected event");
+#endif  // #if MOZ_DIAGNOSTIC_ASSERT_ENABLED
+    } else {
+      mPendingContentCommandEventNeedingAck--;
+    }
   }
 
   if (!PendingEventsNeedingAck()) {

widget/ContentCache.h

@@ -423,6 +423,13 @@ class ContentCacheInParent final : public ContentCache {
    */
   void OnSelectionEvent(const WidgetSelectionEvent& aSelectionEvent);
 
+  /**
+   * OnContentCommandEvent() should called before sending a content command
+   * event.
+   */
+  void OnContentCommandEvent(
+      const WidgetContentCommandEvent& aContentCommandEvent);
+
   /**
    * OnEventNeedingAckHandled() should be called after the child process
    * handles a sent event which needs acknowledging.
@@ -489,7 +496,8 @@ class ContentCacheInParent final : public ContentCache {
   // sent to the remote process, but we've not verified that the remote process
   // finished handling it.
   [[nodiscard]] uint32_t PendingEventsNeedingAck() const {
-    uint32_t ret = mPendingSetSelectionEventNeedingAck;
+    uint32_t ret = mPendingSetSelectionEventNeedingAck +
+                   mPendingContentCommandEventNeedingAck;
     for (const HandlingCompositionData& data : mHandlingCompositions) {
       ret += data.mPendingEventsNeedingAck;
     }
@@ -599,6 +607,9 @@ class ContentCacheInParent final : public ContentCache {
   // process finished handling the events.  Note that eSetSelection may be
   // dispatched without composition.  Therefore, we need to count it with this.
   uint32_t mPendingSetSelectionEventNeedingAck = 0u;
+  // Increased when sending WidgetContentCommandEvent events and decreased when
+  // the remote process finished handling them.
+  uint32_t mPendingContentCommandEventNeedingAck = 0u;
   // mPendingCommitLength is commit string length of the first pending
   // composition.  This is used by relative offset query events when querying
   // new composition start offset.

widget/EventMessageList.h

@@ -358,6 +358,8 @@ NS_EVENT_MESSAGE(eContentCommandLookUpDictionary)
 // scrollable ancestor element can only be scrolled vertically, and horizontal
 // scrolling is requested using this event, no scrolling will occur.
 NS_EVENT_MESSAGE(eContentCommandScroll)
+NS_EVENT_MESSAGE_FIRST_LAST(eContentCommandEvent, eContentCommandCut,
+                            eContentCommandScroll)
 
 // Event to gesture notification
 NS_EVENT_MESSAGE(eGestureNotify)

widget/MiscEvents.h

@@ -78,9 +78,20 @@ class WidgetContentCommandEvent : public WidgetGUIEvent {
     bool mPreventSetSelection = false;  // [in]
   } mSelection;
 
+  // If set to true, the event checks whether the command is enabled in the
+  // process or not without executing the command.  I.e., if it's in the parent
+  // process when a remote process has focus, mIsEnabled may be different from
+  // the latest state of the command in the remote process.
   bool mOnlyEnabledCheck;  // [in]
 
   bool mSucceeded;  // [out]
+
+  // If the command is enabled, set to true.  Otherwise, false. This is set
+  // synchronously in the process.  If it's in the parent process when a remote
+  // process has focus, this returns the command state in the parent process
+  // which may be different from the remote process.
+  // XXX When mOnlyEnabledCheck is set to true, this may be always set to true
+  // even when the command is disabled in the parent process.
   bool mIsEnabled;  // [out]
 
   void AssignContentCommandEventData(const WidgetContentCommandEvent& aEvent,

widget/tests/browser/browser.toml

@@ -19,6 +19,8 @@ skip-if = [
 ["browser_test_consumed_alt_char_on_windows.js"]
 skip-if = ["os != 'win'"]
 
+["browser_test_contentCommand_immediately_after_setSelection.js"]
+
 ["browser_test_fullscreen_size.js"]
 
 ["browser_test_ime_state_after_body_removed_and_reconnected_in_designMode.js"]

widget/tests/browser/browser_test_contentCommand_immediately_after_setSelection.js

@@ -0,0 +1,55 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+add_task(async function () {
+  await BrowserTestUtils.withNewTab(
+    "data:text/html,<html><body><textarea></textarea></body></html>",
+    async function (browser) {
+      await SimpleTest.promiseFocus(browser);
+
+      for (const test of [
+        { command: "cut" },
+        { command: "delete" },
+        { command: "insertText", data: "B" },
+      ]) {
+        // ab[]c
+        await SpecialPowers.spawn(browser, [], () => {
+          const textarea = content.document.querySelector("textarea");
+          textarea.value = "abc";
+          textarea.focus();
+          textarea.selectionStart = "ab".length;
+          textarea.selectionEnd = textarea.selectionStart;
+          content.wrappedJSObject.promiseInput = new Promise(resolve =>
+            textarea.addEventListener("input", () => resolve(textarea.value), {
+              once: true,
+            })
+          );
+        });
+
+        // a[b]c
+        window.windowUtils.sendSelectionSetEvent("a".length, "b".length);
+        // a[]c or a[B]c
+        window.windowUtils.sendContentCommandEvent(
+          test.command,
+          null,
+          test.data
+        );
+
+        info("Waiting for input event on <textarea>...");
+        const textareaValue = await SpecialPowers.spawn(browser, [], () => {
+          return content.wrappedJSObject.promiseInput;
+        });
+        Assert.equal(
+          textareaValue,
+          `a${test.data || ""}c`,
+          `${test.command} immediately after setting selection should affect to the focused <textarea> in the remote process`
+        );
+
+        // wait for a while to flush content cache.
+        for (let i = 0; i < 10; i++) {
+          await new Promise(resolve => requestAnimationFrame(resolve));
+        }
+      }
+    }
+  );
+});