Bug 1594215 - Make `PuppetWidget::GetEditCommands()` check `mBrowserChild` before using it r=smaug

The reason of the crash is, the window may have already been destroyed and
`PuppetWidget::mBrowserChild` was set to `nullptr` when synthesizing key event.

This patch makes `PuppetWidget::GetEditCommands()` check whether it's `nullptr`
and returns whether it's succeeded or not.  Therefore, `TextInputProcessor`
can throw exception in such case.

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

--HG--
extra : moz-landing-system : lando

dom/base/TextInputProcessor.cpp

@@ -999,12 +999,18 @@ nsresult TextInputProcessor::PrepareKeyboardEventToDispatch(
       //      since it checks whether it's called in the main process to
       //      avoid performance issues so that we need to initialize each
       //      command manually here.
-      aKeyboardEvent.InitEditCommandsFor(
-          nsIWidget::NativeKeyBindingsForSingleLineEditor);
-      aKeyboardEvent.InitEditCommandsFor(
-          nsIWidget::NativeKeyBindingsForMultiLineEditor);
-      aKeyboardEvent.InitEditCommandsFor(
-          nsIWidget::NativeKeyBindingsForRichTextEditor);
+      if (NS_WARN_IF(!aKeyboardEvent.InitEditCommandsFor(
+              nsIWidget::NativeKeyBindingsForSingleLineEditor))) {
+        return NS_ERROR_NOT_AVAILABLE;
+      }
+      if (NS_WARN_IF(!aKeyboardEvent.InitEditCommandsFor(
+              nsIWidget::NativeKeyBindingsForMultiLineEditor))) {
+        return NS_ERROR_NOT_AVAILABLE;
+      }
+      if (NS_WARN_IF(!aKeyboardEvent.InitEditCommandsFor(
+              nsIWidget::NativeKeyBindingsForRichTextEditor))) {
+        return NS_ERROR_NOT_AVAILABLE;
+      }
     } else {
       aKeyboardEvent.PreventNativeKeyBindings();
     }

dom/ipc/BrowserParent.cpp

@@ -1610,8 +1610,9 @@ mozilla::ipc::IPCResult BrowserParent::RecvRequestNativeKeyBindings(
     return IPC_OK();
   }
 
-  localEvent.InitEditCommandsFor(keyBindingsType);
-  *aCommands = localEvent.EditCommandsConstRef(keyBindingsType);
+  if (localEvent.InitEditCommandsFor(keyBindingsType)) {
+    *aCommands = localEvent.EditCommandsConstRef(keyBindingsType);
+  }
 
   return IPC_OK();
 }

widget/PuppetWidget.cpp

@@ -528,13 +528,18 @@ bool PuppetWidget::AsyncPanZoomEnabled() const {
   return mBrowserChild && mBrowserChild->AsyncPanZoomEnabled();
 }
 
-void PuppetWidget::GetEditCommands(NativeKeyBindingsType aType,
+bool PuppetWidget::GetEditCommands(NativeKeyBindingsType aType,
                                    const WidgetKeyboardEvent& aEvent,
                                    nsTArray<CommandInt>& aCommands) {
   // Validate the arguments.
-  nsIWidget::GetEditCommands(aType, aEvent, aCommands);
-
+  if (NS_WARN_IF(!nsIWidget::GetEditCommands(aType, aEvent, aCommands))) {
+    return false;
+  }
+  if (NS_WARN_IF(!mBrowserChild)) {
+    return false;
+  }
   mBrowserChild->RequestEditCommands(aType, aEvent, aCommands);
+  return true;
 }
 
 LayerManager* PuppetWidget::GetLayerManager(

widget/PuppetWidget.h

@@ -156,7 +156,7 @@ class PuppetWidget : public nsBaseWidget,
       const mozilla::Maybe<ZoomConstraints>& aConstraints) override;
   bool AsyncPanZoomEnabled() const override;
 
-  virtual void GetEditCommands(
+  virtual bool GetEditCommands(
       NativeKeyBindingsType aType, const mozilla::WidgetKeyboardEvent& aEvent,
       nsTArray<mozilla::CommandInt>& aCommands) override;
 

widget/TextEvents.h

@@ -454,8 +454,12 @@ class WidgetKeyboardEvent : public WidgetInputEvent {
   /**
    * Retrieves edit commands from mWidget only for aType.  This shouldn't be
    * called when the instance is an untrusted event or doesn't have widget.
+   *
+   * @return            false if some resource is not available to get
+   *                    commands unexpectedly.  Otherwise, true even if
+   *                    retrieved command is nothing.
    */
-  void InitEditCommandsFor(nsIWidget::NativeKeyBindingsType aType);
+  bool InitEditCommandsFor(nsIWidget::NativeKeyBindingsType aType);
 
   /**
    * PreventNativeKeyBindings() makes the instance to not cause any edit

widget/WidgetEventImpl.cpp

@@ -750,24 +750,39 @@ void WidgetKeyboardEvent::InitAllEditCommands() {
   MOZ_ASSERT(!AreAllEditCommandsInitialized(),
              "Shouldn't be called two or more times");
 
-  InitEditCommandsFor(nsIWidget::NativeKeyBindingsForSingleLineEditor);
-  InitEditCommandsFor(nsIWidget::NativeKeyBindingsForMultiLineEditor);
-  InitEditCommandsFor(nsIWidget::NativeKeyBindingsForRichTextEditor);
+  DebugOnly<bool> okIgnored =
+      InitEditCommandsFor(nsIWidget::NativeKeyBindingsForSingleLineEditor);
+  NS_WARNING_ASSERTION(
+      okIgnored,
+      "InitEditCommandsFor(nsIWidget::NativeKeyBindingsForSingleLineEditor) "
+      "failed, but ignored");
+  okIgnored =
+      InitEditCommandsFor(nsIWidget::NativeKeyBindingsForMultiLineEditor);
+  NS_WARNING_ASSERTION(
+      okIgnored,
+      "InitEditCommandsFor(nsIWidget::NativeKeyBindingsForMultiLineEditor) "
+      "failed, but ignored");
+  okIgnored =
+      InitEditCommandsFor(nsIWidget::NativeKeyBindingsForRichTextEditor);
+  NS_WARNING_ASSERTION(
+      okIgnored,
+      "InitEditCommandsFor(nsIWidget::NativeKeyBindingsForRichTextEditor) "
+      "failed, but ignored");
 }
 
-void WidgetKeyboardEvent::InitEditCommandsFor(
+bool WidgetKeyboardEvent::InitEditCommandsFor(
     nsIWidget::NativeKeyBindingsType aType) {
   if (NS_WARN_IF(!mWidget) || NS_WARN_IF(!IsTrusted())) {
-    return;
+    return false;
   }
 
   bool& initialized = IsEditCommandsInitializedRef(aType);
   if (initialized) {
-    return;
+    return true;
   }
   nsTArray<CommandInt>& commands = EditCommandsRef(aType);
-  mWidget->GetEditCommands(aType, *this, commands);
-  initialized = true;
+  initialized = mWidget->GetEditCommands(aType, *this, commands);
+  return initialized;
 }
 
 bool WidgetKeyboardEvent::ExecuteEditCommands(
@@ -785,7 +800,9 @@ bool WidgetKeyboardEvent::ExecuteEditCommands(
     return false;
   }
 
-  InitEditCommandsFor(aType);
+  if (NS_WARN_IF(!InitEditCommandsFor(aType))) {
+    return false;
+  }
 
   const nsTArray<CommandInt>& commands = EditCommandsRef(aType);
   if (commands.IsEmpty()) {

widget/cocoa/nsChildView.h

@@ -429,7 +429,7 @@ class nsChildView final : public nsBaseWidget {
   virtual InputContext GetInputContext() override;
   virtual TextEventDispatcherListener* GetNativeTextEventDispatcherListener() override;
   virtual MOZ_MUST_USE nsresult AttachNativeKeyEvent(mozilla::WidgetKeyboardEvent& aEvent) override;
-  virtual void GetEditCommands(NativeKeyBindingsType aType,
+  virtual bool GetEditCommands(NativeKeyBindingsType aType,
                                const mozilla::WidgetKeyboardEvent& aEvent,
                                nsTArray<mozilla::CommandInt>& aCommands) override;
   void GetEditCommandsRemapped(NativeKeyBindingsType aType,

widget/cocoa/nsChildView.mm

@@ -1761,10 +1761,12 @@ void nsChildView::GetEditCommandsRemapped(NativeKeyBindingsType aType,
   keyBindings->GetEditCommands(modifiedEvent, aCommands);
 }
 
-void nsChildView::GetEditCommands(NativeKeyBindingsType aType, const WidgetKeyboardEvent& aEvent,
+bool nsChildView::GetEditCommands(NativeKeyBindingsType aType, const WidgetKeyboardEvent& aEvent,
                                   nsTArray<CommandInt>& aCommands) {
   // Validate the arguments.
-  nsIWidget::GetEditCommands(aType, aEvent, aCommands);
+  if (NS_WARN_IF(!nsIWidget::GetEditCommands(aType, aEvent, aCommands))) {
+    return false;
+  }
 
   // If the key is a cursor-movement arrow, and the current selection has
   // vertical writing-mode, we'll remap so that the movement command
@@ -1812,12 +1814,13 @@ void nsChildView::GetEditCommands(NativeKeyBindingsType aType, const WidgetKeybo
       }
 
       GetEditCommandsRemapped(aType, aEvent, aCommands, geckoKey, cocoaKey);
-      return;
+      return true;
     }
   }
 
   NativeKeyBindings* keyBindings = NativeKeyBindings::GetInstance(aType);
   keyBindings->GetEditCommands(aEvent, aCommands);
+  return true;
 }
 
 NSView<mozView>* nsChildView::GetEditorView() {

widget/cocoa/nsCocoaWindow.h

@@ -328,7 +328,7 @@ class nsCocoaWindow final : public nsBaseWidget, public nsPIWidgetCocoa {
   virtual void SetInputContext(const InputContext& aContext,
                                const InputContextAction& aAction) override;
   virtual InputContext GetInputContext() override { return mInputContext; }
-  virtual void GetEditCommands(NativeKeyBindingsType aType,
+  virtual bool GetEditCommands(NativeKeyBindingsType aType,
                                const mozilla::WidgetKeyboardEvent& aEvent,
                                nsTArray<mozilla::CommandInt>& aCommands) override;
 

widget/cocoa/nsCocoaWindow.mm

@@ -2257,13 +2257,16 @@ void nsCocoaWindow::SetInputContext(const InputContext& aContext,
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
-void nsCocoaWindow::GetEditCommands(NativeKeyBindingsType aType, const WidgetKeyboardEvent& aEvent,
+bool nsCocoaWindow::GetEditCommands(NativeKeyBindingsType aType, const WidgetKeyboardEvent& aEvent,
                                     nsTArray<CommandInt>& aCommands) {
   // Validate the arguments.
-  nsIWidget::GetEditCommands(aType, aEvent, aCommands);
+  if (NS_WARN_IF(!nsIWidget::GetEditCommands(aType, aEvent, aCommands))) {
+    return false;
+  }
 
   NativeKeyBindings* keyBindings = NativeKeyBindings::GetInstance(aType);
   keyBindings->GetEditCommands(aEvent, aCommands);
+  return true;
 }
 
 already_AddRefed<nsIWidget> nsIWidget::CreateTopLevelWindow() {

widget/gtk/nsWindow.cpp

@@ -6394,11 +6394,13 @@ void nsWindow::GetEditCommandsRemapped(NativeKeyBindingsType aType,
   keyBindings->GetEditCommands(modifiedEvent, aCommands);
 }
 
-void nsWindow::GetEditCommands(NativeKeyBindingsType aType,
+bool nsWindow::GetEditCommands(NativeKeyBindingsType aType,
                                const WidgetKeyboardEvent& aEvent,
                                nsTArray<CommandInt>& aCommands) {
   // Validate the arguments.
-  nsIWidget::GetEditCommands(aType, aEvent, aCommands);
+  if (NS_WARN_IF(!nsIWidget::GetEditCommands(aType, aEvent, aCommands))) {
+    return false;
+  }
 
   if (aEvent.mKeyCode >= NS_VK_LEFT && aEvent.mKeyCode <= NS_VK_DOWN) {
     // Check if we're targeting content with vertical writing mode,
@@ -6444,12 +6446,13 @@ void nsWindow::GetEditCommands(NativeKeyBindingsType aType,
       }
 
       GetEditCommandsRemapped(aType, aEvent, aCommands, geckoCode, gdkCode);
-      return;
+      return true;
     }
   }
 
   NativeKeyBindings* keyBindings = NativeKeyBindings::GetInstance(aType);
   keyBindings->GetEditCommands(aEvent, aCommands);
+  return true;
 }
 
 already_AddRefed<DrawTarget> nsWindow::StartRemoteDrawingInRegion(

widget/gtk/nsWindow.h

@@ -300,7 +300,7 @@ class nsWindow final : public nsBaseWidget {
                                const mozilla::WidgetKeyboardEvent& aEvent,
                                nsTArray<mozilla::CommandInt>& aCommands,
                                uint32_t aGeckoKeyCode, uint32_t aNativeKeyCode);
-  virtual void GetEditCommands(
+  virtual bool GetEditCommands(
       NativeKeyBindingsType aType, const mozilla::WidgetKeyboardEvent& aEvent,
       nsTArray<mozilla::CommandInt>& aCommands) override;
 

widget/headless/HeadlessWidget.cpp

@@ -391,14 +391,17 @@ nsresult HeadlessWidget::AttachNativeKeyEvent(WidgetKeyboardEvent& aEvent) {
   return bindings.AttachNativeKeyEvent(aEvent);
 }
 
-void HeadlessWidget::GetEditCommands(NativeKeyBindingsType aType,
+bool HeadlessWidget::GetEditCommands(NativeKeyBindingsType aType,
                                      const WidgetKeyboardEvent& aEvent,
                                      nsTArray<CommandInt>& aCommands) {
   // Validate the arguments.
-  nsIWidget::GetEditCommands(aType, aEvent, aCommands);
+  if (NS_WARN_IF(!nsIWidget::GetEditCommands(aType, aEvent, aCommands))) {
+    return false;
+  }
 
   HeadlessKeyBindings& bindings = HeadlessKeyBindings::GetInstance();
   bindings.GetEditCommands(aType, aEvent, aCommands);
+  return true;
 }
 
 nsresult HeadlessWidget::DispatchEvent(WidgetGUIEvent* aEvent,

widget/headless/HeadlessWidget.h

@@ -124,7 +124,7 @@ class HeadlessWidget : public nsBaseWidget {
 
   virtual MOZ_MUST_USE nsresult
   AttachNativeKeyEvent(WidgetKeyboardEvent& aEvent) override;
-  virtual void GetEditCommands(NativeKeyBindingsType aType,
+  virtual bool GetEditCommands(NativeKeyBindingsType aType,
                                const WidgetKeyboardEvent& aEvent,
                                nsTArray<CommandInt>& aCommands) override;
 

widget/nsBaseWidget.cpp

@@ -2217,11 +2217,12 @@ nsresult nsIWidget::OnWindowedPluginKeyEvent(
 
 void nsIWidget::PostHandleKeyEvent(mozilla::WidgetKeyboardEvent* aEvent) {}
 
-void nsIWidget::GetEditCommands(nsIWidget::NativeKeyBindingsType aType,
+bool nsIWidget::GetEditCommands(nsIWidget::NativeKeyBindingsType aType,
                                 const WidgetKeyboardEvent& aEvent,
                                 nsTArray<CommandInt>& aCommands) {
   MOZ_ASSERT(aEvent.IsTrusted());
   MOZ_ASSERT(aCommands.IsEmpty());
+  return true;
 }
 
 already_AddRefed<nsIBidiKeyboard> nsIWidget::CreateBidiKeyboard() {

widget/nsIWidget.h

@@ -1895,7 +1895,7 @@ class nsIWidget : public nsISupports {
     NativeKeyBindingsForMultiLineEditor,
     NativeKeyBindingsForRichTextEditor
   };
-  virtual void GetEditCommands(NativeKeyBindingsType aType,
+  virtual bool GetEditCommands(NativeKeyBindingsType aType,
                                const mozilla::WidgetKeyboardEvent& aEvent,
                                nsTArray<mozilla::CommandInt>& aCommands);