Bug 1339543 part 6 PBrowser::RequestNativeKeyBindings() should retrieves edit commands only for specified type r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 19 May 2017 18:46:02 +0900
changeset 359320 bf71ece03a290119c947cb3f93d311c34a6d4a78
parent 359319 8d32e150bf7fe9b0db1c37174c70b352ce34812e
child 359321 44874c36cec7ff5a084954b81cfc3a00b66f73bb
push id90494
push userkwierso@gmail.com
push dateFri, 19 May 2017 22:19:13 +0000
treeherdermozilla-inbound@cd9b1ab819e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1339543
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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);