Bug 1339543 part 6 PBrowser::RequestNativeKeyBindings() should retrieves edit commands only for specified type r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 19 May 2017 18:46:02 +0900
changeset 581132 801a9eed6aaa697742d23d4621cd0dacf077bd0f
parent 581131 714cde97e14680ea2bb4b657b805b4ef7c58da96
child 629492 b84015b9f101e6ccd88cafb678db5f9f38f6755d
push id59777
push usermasayuki@d-toybox.com
push dateFri, 19 May 2017 09:49:08 +0000
reviewerssmaug
bugs1339543
milestone55.0a1
Bug 1339543 part 6 PBrowser::RequestNativeKeyBindings() should retrieves edit commands only for specified type r?smaug PBrowser::RequestNativeKeyBindings() is used only when somebody tries to execute native key bindings for synthesized keyboard events. Therefore, it doesn't need to retrieve edit commands for all editor types. Instead, it should take the editor type and just return the edit commands for it. MozReview-Commit-ID: HF4Gz99SBQP
dom/ipc/PBrowser.ipdl
dom/ipc/TabChild.cpp
dom/ipc/TabChild.h
dom/ipc/TabParent.cpp
dom/ipc/TabParent.h
widget/PuppetWidget.cpp
widget/TextEvents.h
widget/WidgetEventImpl.cpp
widget/nsIWidget.h
--- a/dom/ipc/PBrowser.ipdl
+++ b/dom/ipc/PBrowser.ipdl
@@ -82,29 +82,16 @@ using nsSizeMode from "nsIWidgetListener
 using mozilla::widget::CandidateWindowPosition from "ipc/nsGUIEventIPC.h";
 using class mozilla::NativeEventData from "ipc/nsGUIEventIPC.h";
 using mozilla::FontRange from "ipc/nsGUIEventIPC.h";
 using mozilla::a11y::IAccessibleHolder from "mozilla/a11y/IPCTypes.h";
 
 namespace mozilla {
 namespace dom {
 
-struct NativeKeyBinding
-{
-  CommandInt[] singleLineCommands;
-  CommandInt[] multiLineCommands;
-  CommandInt[] richTextCommands;
-};
-
-union MaybeNativeKeyBinding
-{
-  NativeKeyBinding;
-  void_t;
-};
-
 struct ShowInfo
 {
   nsString name;
   bool fullscreenAllowed;
   bool isPrivate;
   bool fakeShowInfo;
   bool isTransparent;
   float dpi;
@@ -506,18 +493,27 @@ parent:
      */
     async LookUpDictionary(nsString aText, FontRange[] aFontRangeArray,
                            bool aIsVertical, LayoutDeviceIntPoint aPoint);
 
     async __delete__();
 
     async ReplyKeyEvent(WidgetKeyboardEvent event);
 
-    sync RequestNativeKeyBindings(WidgetKeyboardEvent event)
-        returns (MaybeNativeKeyBinding bindings);
+    /**
+     * Retrieves edit commands for the key combination represented by aEvent.
+     *
+     * @param aType       One of nsIWidget::NativeKeyBindingsType.
+     * @param aEvent      KeyboardEvent which represents a key combination.
+     *                    Note that this must be a trusted event.
+     * @return            Array of edit commands which should be executed in
+     *                    editor of native applications.
+     */
+    sync RequestNativeKeyBindings(uint32_t aType, WidgetKeyboardEvent aEvent)
+        returns (CommandInt[] commands);
 
     async SynthesizeNativeKeyEvent(int32_t aNativeKeyboardLayout,
                                    int32_t aNativeKeyCode,
                                    uint32_t aModifierFlags,
                                    nsString aCharacters,
                                    nsString aUnmodifiedCharacters,
                                    uint64_t aObserverId);
     async SynthesizeNativeMouseEvent(LayoutDeviceIntPoint aPoint,
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -1873,48 +1873,37 @@ TabChild::RecvPluginEvent(const WidgetPl
   if (status != nsEventStatus_eConsumeNoDefault) {
     // If not consumed, we should call default action
     SendDefaultProcOfPluginEvent(aEvent);
   }
   return IPC_OK();
 }
 
 void
-TabChild::RequestNativeKeyBindings(nsIWidget::NativeKeyBindingsType aType,
-                                   const WidgetKeyboardEvent& aEvent,
-                                   nsTArray<CommandInt>& aCommands)
+TabChild::RequestEditCommands(nsIWidget::NativeKeyBindingsType aType,
+                              const WidgetKeyboardEvent& aEvent,
+                              nsTArray<CommandInt>& aCommands)
 {
   MOZ_ASSERT(aCommands.IsEmpty());
 
   if (NS_WARN_IF(aEvent.IsEditCommandsInitialized(aType))) {
     aCommands = aEvent.EditCommandsConstRef(aType);
     return;
   }
 
-  // TODO: Should retrieve edit commands only for specific type.
-  MaybeNativeKeyBinding maybeBindings;
-  if (!SendRequestNativeKeyBindings(aEvent, &maybeBindings) ||
-      maybeBindings.type() != MaybeNativeKeyBinding::TNativeKeyBinding) {
-    return;
-  }
-
-  const NativeKeyBinding& bindings = maybeBindings;
   switch (aType) {
     case nsIWidget::NativeKeyBindingsForSingleLineEditor:
-      aCommands = bindings.singleLineCommands();
-      break;
     case nsIWidget::NativeKeyBindingsForMultiLineEditor:
-      aCommands = bindings.multiLineCommands();
-      break;
     case nsIWidget::NativeKeyBindingsForRichTextEditor:
-      aCommands = bindings.richTextCommands();
       break;
     default:
       MOZ_ASSERT_UNREACHABLE("Invalid native key bindings type");
   }
+
+  SendRequestNativeKeyBindings(aType, aEvent, &aCommands);
 }
 
 mozilla::ipc::IPCResult
 TabChild::RecvNativeSynthesisResponse(const uint64_t& aObserverId,
                                       const nsCString& aResponse)
 {
   mozilla::widget::AutoObserverNotifier::NotifySavedObserver(aObserverId,
                                                              aResponse.get());
--- a/dom/ipc/TabChild.h
+++ b/dom/ipc/TabChild.h
@@ -506,19 +506,19 @@ public:
   void GetMaxTouchPoints(uint32_t* aTouchPoints);
 
   ScreenOrientationInternal GetOrientation() const { return mOrientation; }
 
   void SetBackgroundColor(const nscolor& aColor);
 
   void NotifyPainted();
 
-  void RequestNativeKeyBindings(nsIWidget::NativeKeyBindingsType aType,
-                                const WidgetKeyboardEvent& aEvent,
-                                nsTArray<CommandInt>& aCommands);
+  void RequestEditCommands(nsIWidget::NativeKeyBindingsType aType,
+                           const WidgetKeyboardEvent& aEvent,
+                           nsTArray<CommandInt>& aCommands);
 
   /**
    * Signal to this TabChild that it should be made visible:
    * activated widget, retained layer tree, etc.  (Respectively,
    * made not visible.)
    */
   void MakeVisible();
   void MakeHidden();
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -1210,47 +1210,48 @@ TabParent::RecvDispatchKeyboardEvent(con
   localEvent.mWidget = widget;
   localEvent.mRefPoint -= GetChildProcessOffset();
 
   widget->DispatchInputEvent(&localEvent);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
-TabParent::RecvRequestNativeKeyBindings(const WidgetKeyboardEvent& aEvent,
-                                        MaybeNativeKeyBinding* aBindings)
+TabParent::RecvRequestNativeKeyBindings(const uint32_t& aType,
+                                        const WidgetKeyboardEvent& aEvent,
+                                        nsTArray<CommandInt>* aCommands)
 {
-  *aBindings = mozilla::void_t();
+  MOZ_ASSERT(aCommands);
+  MOZ_ASSERT(aCommands->IsEmpty());
+
+  nsIWidget::NativeKeyBindingsType keyBindingsType =
+    static_cast<nsIWidget::NativeKeyBindingsType>(aType);
+  switch (keyBindingsType) {
+    case nsIWidget::NativeKeyBindingsForSingleLineEditor:
+    case nsIWidget::NativeKeyBindingsForMultiLineEditor:
+    case nsIWidget::NativeKeyBindingsForRichTextEditor:
+      break;
+    default:
+      return IPC_FAIL(this, "Invalid aType value");
+  }
 
   nsCOMPtr<nsIWidget> widget = GetWidget();
   if (!widget) {
     return IPC_OK();
   }
 
   WidgetKeyboardEvent localEvent(aEvent);
   localEvent.mWidget = widget;
 
   if (NS_FAILED(widget->AttachNativeKeyEvent(localEvent))) {
     return IPC_OK();
   }
 
-  localEvent.InitAllEditCommands();
-
-  const nsTArray<CommandInt>& multiLine =
-    localEvent.EditCommandsConstRef(
-                 nsIWidget::NativeKeyBindingsForSingleLineEditor);
-  const nsTArray<CommandInt>& singleLine =
-    localEvent.EditCommandsConstRef(
-                 nsIWidget::NativeKeyBindingsForMultiLineEditor);
-  const nsTArray<CommandInt>& richText =
-    localEvent.EditCommandsConstRef(
-                 nsIWidget::NativeKeyBindingsForRichTextEditor);
-  if (!singleLine.IsEmpty() || !multiLine.IsEmpty() || !richText.IsEmpty()) {
-    *aBindings = NativeKeyBinding(singleLine, multiLine, richText);
-  }
+  localEvent.InitEditCommandsFor(keyBindingsType);
+  *aCommands = localEvent.EditCommandsConstRef(keyBindingsType);
 
   return IPC_OK();
 }
 
 class SynthesizedEventObserver : public nsIObserver
 {
   NS_DECL_ISUPPORTS
 
--- a/dom/ipc/TabParent.h
+++ b/dom/ipc/TabParent.h
@@ -388,18 +388,20 @@ public:
   bool MapEventCoordinatesForChildProcess(mozilla::WidgetEvent* aEvent);
 
   void MapEventCoordinatesForChildProcess(const LayoutDeviceIntPoint& aOffset,
                                           mozilla::WidgetEvent* aEvent);
 
   LayoutDeviceToCSSScale GetLayoutDeviceToCSSScale();
 
   virtual mozilla::ipc::IPCResult
-  RecvRequestNativeKeyBindings(const mozilla::WidgetKeyboardEvent& aEvent,
-                               MaybeNativeKeyBinding* aBindings) override;
+  RecvRequestNativeKeyBindings(
+    const uint32_t& aType,
+    const mozilla::WidgetKeyboardEvent& aEvent,
+    nsTArray<mozilla::CommandInt>* aCommands) override;
 
   virtual mozilla::ipc::IPCResult
   RecvSynthesizeNativeKeyEvent(const int32_t& aNativeKeyboardLayout,
                                const int32_t& aNativeKeyCode,
                                const uint32_t& aModifierFlags,
                                const nsString& aCharacters,
                                const nsString& aUnmodifiedCharacters,
                                const uint64_t& aObserverId) override;
--- a/widget/PuppetWidget.cpp
+++ b/widget/PuppetWidget.cpp
@@ -547,17 +547,17 @@ PuppetWidget::AsyncPanZoomEnabled() cons
 void
 PuppetWidget::GetEditCommands(NativeKeyBindingsType aType,
                               const WidgetKeyboardEvent& aEvent,
                               nsTArray<CommandInt>& aCommands)
 {
   // Validate the arguments.
   nsIWidget::GetEditCommands(aType, aEvent, aCommands);
 
-  mTabChild->RequestNativeKeyBindings(aType, aEvent, aCommands);
+  mTabChild->RequestEditCommands(aType, aEvent, aCommands);
 }
 
 LayerManager*
 PuppetWidget::GetLayerManager(PLayerTransactionChild* aShadowManager,
                               LayersBackend aBackendHint,
                               LayerManagerPersistence aPersistence)
 {
   if (!mLayerManager) {
--- a/widget/TextEvents.h
+++ b/widget/TextEvents.h
@@ -291,16 +291,22 @@ public:
   /**
    * Retrieves all edit commands from mWidget.  This shouldn't be called when
    * the instance is an untrusted event, doesn't have widget or in non-chrome
    * process.
    */
   void InitAllEditCommands();
 
   /**
+   * 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.
+   */
+  void InitEditCommandsFor(nsIWidget::NativeKeyBindingsType aType);
+
+  /**
    * PreventNativeKeyBindings() makes the instance to not cause any edit
    * actions even if it matches with a native key binding.
    */
   void PreventNativeKeyBindings()
   {
     mEditCommandsForSingleLineEditor.Clear();
     mEditCommandsForMultiLineEditor.Clear();
     mEditCommandsForRichTextEditor.Clear();
@@ -544,18 +550,16 @@ private:
         return mEditCommandsForMultiLineEditorInitialized;
       case nsIWidget::NativeKeyBindingsForRichTextEditor:
         return mEditCommandsForRichTextEditorInitialized;
       default:
         MOZ_MAKE_COMPILER_ASSUME_IS_UNREACHABLE(
           "Invalid native key binding type");
     }
   }
-
-  void InitEditCommandsFor(nsIWidget::NativeKeyBindingsType aType);
 };
 
 /******************************************************************************
  * mozilla::WidgetCompositionEvent
  ******************************************************************************/
 
 class WidgetCompositionEvent : public WidgetGUIEvent
 {
--- a/widget/WidgetEventImpl.cpp
+++ b/widget/WidgetEventImpl.cpp
@@ -626,18 +626,19 @@ WidgetKeyboardEvent::InitAllEditCommands
   InitEditCommandsFor(nsIWidget::NativeKeyBindingsForSingleLineEditor);
   InitEditCommandsFor(nsIWidget::NativeKeyBindingsForMultiLineEditor);
   InitEditCommandsFor(nsIWidget::NativeKeyBindingsForRichTextEditor);
 }
 
 void
 WidgetKeyboardEvent::InitEditCommandsFor(nsIWidget::NativeKeyBindingsType aType)
 {
-  MOZ_ASSERT(mWidget);
-  MOZ_RELEASE_ASSERT(IsTrusted());
+  if (NS_WARN_IF(!mWidget) || NS_WARN_IF(!IsTrusted())) {
+    return;
+  }
 
   bool& initialized = IsEditCommandsInitializedRef(aType);
   if (initialized) {
     return;
   }
   nsTArray<CommandInt>& commands = EditCommandsRef(aType);
   mWidget->GetEditCommands(aType, *this, commands);
   initialized = true;
--- a/widget/nsIWidget.h
+++ b/widget/nsIWidget.h
@@ -1816,17 +1816,17 @@ public:
      */
     virtual MOZ_MUST_USE nsresult
     AttachNativeKeyEvent(mozilla::WidgetKeyboardEvent& aEvent) = 0;
 
     /**
      * Retrieve edit commands when the key combination of aEvent is used
      * in platform native applications.
      */
-    enum NativeKeyBindingsType
+    enum NativeKeyBindingsType : uint8_t
     {
       NativeKeyBindingsForSingleLineEditor,
       NativeKeyBindingsForMultiLineEditor,
       NativeKeyBindingsForRichTextEditor
     };
     virtual void GetEditCommands(NativeKeyBindingsType aType,
                                  const mozilla::WidgetKeyboardEvent& aEvent,
                                  nsTArray<mozilla::CommandInt>& aCommands);