Bug 900750 - part 3: Remove unnecessary ModifierKeyState argument from some methods of NativeKey and KeyboardLayout r=m_kato

KeyboardLayout::InitNativeKey() takes |const ModifierKeyState&| as its
argument with NativeKey reference and it calls some internal methods with
the given ModifierKeyState without any changes.  Additionally, its caller
is only NativeKey::InitWithKeyChar() and its called with given NativeKey
instance's mModKeyState.  So, removing the redundant arguments from
some methods makes them clearer what they compute with.

So, this patch does not change any behavior.

MozReview-Commit-ID: 3w9Ee7PMU05

widget/windows/KeyboardLayout.cpp

@@ -1524,7 +1524,7 @@ NativeKey::InitWithKeyOrChar()
     }
   }
 
-  keyboardLayout->InitNativeKey(*this, mModKeyState);
+  keyboardLayout->InitNativeKey(*this);
 
   mIsDeadKey =
     (IsFollowedByDeadCharMessage() ||
@@ -1554,14 +1554,13 @@ NativeKey::InitWithKeyOrChar()
 }
 
 void
-NativeKey::InitCommittedCharsAndModifiersWithFollowingCharMessages(
+NativeKey::InitCommittedCharsAndModifiersWithFollowingCharMessages()
-             const ModifierKeyState& aModKeyState)
 {
   mCommittedCharsAndModifiers.Clear();
   // This should cause inputting text in focused editor.  However, it
   // ignores keypress events whose altKey or ctrlKey is true.
   // Therefore, we need to remove these modifier state here.
-  Modifiers modifiers = aModKeyState.GetModifiers();
+  Modifiers modifiers = mModKeyState.GetModifiers();
   if (IsFollowedByPrintableCharMessage()) {
     modifiers &= ~(MODIFIER_ALT | MODIFIER_CONTROL);
   }
@@ -3840,8 +3839,7 @@ KeyboardLayout::IsSysKey(uint8_t aVirtualKey,
 }
 
 void
-KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
+KeyboardLayout::InitNativeKey(NativeKey& aNativeKey)
-                              const ModifierKeyState& aModKeyState)
 {
   if (mIsPendingToRestoreKeyboardLayout) {
     LoadLayout(::GetKeyboardLayout(0));
@@ -3857,7 +3855,7 @@ KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
     if (!NativeKey::IsControlChar(ch)) {
       aNativeKey.mKeyNameIndex = KEY_NAME_INDEX_USE_STRING;
       Modifiers modifiers =
-        aModKeyState.GetModifiers() & ~(MODIFIER_ALT | MODIFIER_CONTROL);
+        aNativeKey.GetModifiers() & ~(MODIFIER_ALT | MODIFIER_CONTROL);
       aNativeKey.mCommittedCharsAndModifiers.Append(ch, modifiers);
       return;
     }
@@ -3876,8 +3874,7 @@ KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
     MOZ_ASSERT(!aNativeKey.IsCharMessage(aNativeKey.mMsg));
     if (aNativeKey.IsFollowedByPrintableCharOrSysCharMessage()) {
       // Initialize mCommittedCharsAndModifiers with following char messages.
-      aNativeKey.
+      aNativeKey.InitCommittedCharsAndModifiersWithFollowingCharMessages();
-        InitCommittedCharsAndModifiersWithFollowingCharMessages(aModKeyState);
       MOZ_ASSERT(!aNativeKey.mCommittedCharsAndModifiers.IsEmpty());
 
       // Currently, we are doing a ugly hack to keypress events to cause
@@ -3888,11 +3885,11 @@ KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
       // we should mark as not removable if Ctrl or Alt key does not cause
       // changing inputting character.
       if (IsPrintableCharKey(aNativeKey.mOriginalVirtualKeyCode) &&
-          (aModKeyState.IsControl() ^ aModKeyState.IsAlt())) {
+          (aNativeKey.IsControl() ^ aNativeKey.IsAlt())) {
-        ModifierKeyState state = aModKeyState;
+        ModifierKeyState state = aNativeKey.ModifierKeyStateRef();
         state.Unset(MODIFIER_ALT | MODIFIER_CONTROL);
         UniCharsAndModifiers charsWithoutModifier =
-          GetUniCharsAndModifiers(aNativeKey.mOriginalVirtualKeyCode, state);
+          GetUniCharsAndModifiers(aNativeKey.GenericVirtualKeyCode(), state);
         aNativeKey.mCanIgnoreModifierStateAtKeyPress =
           !charsWithoutModifier.UniCharsEqual(
                                   aNativeKey.mCommittedCharsAndModifiers);
@@ -3920,7 +3917,7 @@ KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
 
   // If it's a dead key, aNativeKey will be initialized by
   // MaybeInitNativeKeyAsDeadKey().
-  if (MaybeInitNativeKeyAsDeadKey(aNativeKey, aModKeyState)) {
+  if (MaybeInitNativeKeyAsDeadKey(aNativeKey)) {
     return;
   }
 
@@ -3941,12 +3938,11 @@ KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
   // If it's in dead key handling and the pressed key causes a composite
   // character, aNativeKey will be initialized by
   // MaybeInitNativeKeyWithCompositeChar().
-  if (MaybeInitNativeKeyWithCompositeChar(aNativeKey, aModKeyState)) {
+  if (MaybeInitNativeKeyWithCompositeChar(aNativeKey)) {
     return;
   }
 
-  UniCharsAndModifiers baseChars =
+  UniCharsAndModifiers baseChars = GetUniCharsAndModifiers(aNativeKey);
-    GetUniCharsAndModifiers(aNativeKey.mOriginalVirtualKeyCode, aModKeyState);
 
   // If the key press isn't related to any dead keys, initialize aNativeKey
   // with the characters which should be caused by the key.
@@ -3966,13 +3962,10 @@ KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
 }
 
 bool
-KeyboardLayout::MaybeInitNativeKeyAsDeadKey(
+KeyboardLayout::MaybeInitNativeKeyAsDeadKey(NativeKey& aNativeKey)
-                  NativeKey& aNativeKey,
-                  const ModifierKeyState& aModKeyState)
 {
   // Only when it's not in dead key sequence, we can trust IsDeadKey() result.
-  if (!IsInDeadKeySequence() &&
+  if (!IsInDeadKeySequence() && !IsDeadKey(aNativeKey)) {
-      !IsDeadKey(aNativeKey.mOriginalVirtualKeyCode, aModKeyState)) {
     return false;
   }
 
@@ -3987,10 +3980,10 @@ KeyboardLayout::MaybeInitNativeKeyAsDeadKey(
   // different.
   bool isDeadKeyUpEvent =
     !aNativeKey.IsKeyDownMessage() &&
-    mActiveDeadKeys.Contains(aNativeKey.mOriginalVirtualKeyCode);
+    mActiveDeadKeys.Contains(aNativeKey.GenericVirtualKeyCode());
 
   if (isDeadKeyDownEvent || isDeadKeyUpEvent) {
-    ActivateDeadKeyState(aNativeKey, aModKeyState);
+    ActivateDeadKeyState(aNativeKey);
     // Any dead key events don't generate characters.  So, a dead key should
     // cause only keydown event and keyup event whose KeyboardEvent.key
     // values are "Dead".
@@ -4006,14 +3999,14 @@ KeyboardLayout::MaybeInitNativeKeyAsDeadKey(
   // set only a character for current key for keyup event.
   if (!IsInDeadKeySequence()) {
     aNativeKey.mCommittedCharsAndModifiers =
-      GetUniCharsAndModifiers(aNativeKey.mOriginalVirtualKeyCode, aModKeyState);
+      GetUniCharsAndModifiers(aNativeKey);
     return true;
   }
 
   // When non-printable key event comes during a dead key sequence, that must
   // be a modifier key event.  So, such events shouldn't be handled as a part
   // of the dead key sequence.
-  if (!IsDeadKey(aNativeKey.mOriginalVirtualKeyCode, aModKeyState)) {
+  if (!IsDeadKey(aNativeKey)) {
     return false;
   }
 
@@ -4024,15 +4017,14 @@ KeyboardLayout::MaybeInitNativeKeyAsDeadKey(
 
   // Dead key followed by another dead key may cause a composed character
   // (e.g., "Russian - Mnemonic" keyboard layout's 's' -> 'c').
-  if (MaybeInitNativeKeyWithCompositeChar(aNativeKey, aModKeyState)) {
+  if (MaybeInitNativeKeyWithCompositeChar(aNativeKey)) {
     return true;
   }
 
   // Otherwise, dead key followed by another dead key causes inputting both
   // character.
   UniCharsAndModifiers prevDeadChars = GetDeadUniCharsAndModifiers();
-  UniCharsAndModifiers newChars =
+  UniCharsAndModifiers newChars = GetUniCharsAndModifiers(aNativeKey);
-    GetUniCharsAndModifiers(aNativeKey.mOriginalVirtualKeyCode, aModKeyState);
   // But keypress events should be fired for each committed character.
   aNativeKey.mCommittedCharsAndModifiers = prevDeadChars + newChars;
   if (aNativeKey.IsKeyDownMessage()) {
@@ -4042,9 +4034,7 @@ KeyboardLayout::MaybeInitNativeKeyAsDeadKey(
 }
 
 bool
-KeyboardLayout::MaybeInitNativeKeyWithCompositeChar(
+KeyboardLayout::MaybeInitNativeKeyWithCompositeChar(NativeKey& aNativeKey)
-                  NativeKey& aNativeKey,
-                  const ModifierKeyState& aModKeyState)
 {
   if (!IsInDeadKeySequence()) {
     return false;
@@ -4054,8 +4044,7 @@ KeyboardLayout::MaybeInitNativeKeyWithCompositeChar(
     return false;
   }
 
-  UniCharsAndModifiers baseChars =
+  UniCharsAndModifiers baseChars = GetUniCharsAndModifiers(aNativeKey);
-    GetUniCharsAndModifiers(aNativeKey.mOriginalVirtualKeyCode, aModKeyState);
   if (baseChars.IsEmpty() || !baseChars.CharAt(0)) {
     return false;
   }
@@ -4088,20 +4077,6 @@ KeyboardLayout::GetUniCharsAndModifiers(
   return mVirtualKeys[key].GetUniChars(aShiftState);
 }
 
-UniCharsAndModifiers
-KeyboardLayout::GetNativeUniCharsAndModifiers(
-                  uint8_t aVirtualKey,
-                  const ModifierKeyState& aModKeyState) const
-{
-  int32_t key = GetKeyIndex(aVirtualKey);
-  if (key < 0) {
-    return UniCharsAndModifiers();
-  }
-  VirtualKey::ShiftState shiftState =
-    VirtualKey::ModifierKeyStateToShiftState(aModKeyState);
-  return mVirtualKeys[key].GetNativeUniChars(shiftState);
-}
-
 UniCharsAndModifiers
 KeyboardLayout::GetDeadUniCharsAndModifiers() const
 {
@@ -4533,8 +4508,7 @@ KeyboardLayout::EnsureDeadKeyActive(bool aIsActive,
 }
 
 void
-KeyboardLayout::ActivateDeadKeyState(const NativeKey& aNativeKey,
+KeyboardLayout::ActivateDeadKeyState(const NativeKey& aNativeKey)
-                                     const ModifierKeyState& aModKeyState)
 {
   // Dead-key state should be activated at keydown.
   if (!aNativeKey.IsKeyDownMessage()) {
@@ -4542,8 +4516,7 @@ KeyboardLayout::ActivateDeadKeyState(const NativeKey& aNativeKey,
   }
 
   mActiveDeadKeys.AppendElement(aNativeKey.mOriginalVirtualKeyCode);
-  mDeadKeyShiftStates.AppendElement(
+  mDeadKeyShiftStates.AppendElement(aNativeKey.GetShiftState());
-    VirtualKey::ModifierKeyStateToShiftState(aModKeyState));
 }
 
 void

widget/windows/KeyboardLayout.h

@@ -478,6 +478,33 @@ public:
    */
   static bool IsControlChar(char16_t aChar);
 
+  bool IsControl() const { return mModKeyState.IsControl(); }
+  bool IsAlt() const { return mModKeyState.IsAlt(); }
+  Modifiers GetModifiers() const { return mModKeyState.GetModifiers(); }
+  const ModifierKeyState& ModifierKeyStateRef() const
+  {
+    return mModKeyState;
+  }
+  VirtualKey::ShiftState GetShiftState() const
+  {
+    return VirtualKey::ModifierKeyStateToShiftState(mModKeyState);
+  }
+
+  /**
+   * GenericVirtualKeyCode() returns virtual keycode which cannot distinguish
+   * position of modifier keys.  E.g., VK_CONTROL for both ControlLeft and
+   * ControlRight.
+   */
+  uint8_t GenericVirtualKeyCode() const { return mOriginalVirtualKeyCode; }
+
+  /**
+   * SpecificVirtualKeyCode() returns virtual keycode which can distinguish
+   * position of modifier keys.  E.g., returns VK_LCONTROL or VK_RCONTROL
+   * instead of VK_CONTROL.  If the key message is synthesized with not
+   * enough information, this prefers left position's keycode.
+   */
+  uint8_t SpecificVirtualKeyCode() const { return mVirtualKeyCode; }
+
 private:
   NativeKey* mLastInstance;
   // mRemovingMsg is set at removing a char message from
@@ -594,12 +621,11 @@ private:
 
   /**
    * InitCommittedCharsAndModifiersWithFollowingCharMessages() initializes
-   * mCommittedCharsAndModifiers with mFollowingCharMsgs and aModKeyState.
+   * mCommittedCharsAndModifiers with mFollowingCharMsgs and mModKeyState.
    * If mFollowingCharMsgs includes non-printable char messages, they are
    * ignored (skipped).
    */
-  void InitCommittedCharsAndModifiersWithFollowingCharMessages(
+  void InitCommittedCharsAndModifiersWithFollowingCharMessages();
-         const ModifierKeyState& aModKeyState);
 
   UINT GetScanCodeWithExtendedFlag() const;
 
@@ -864,6 +890,11 @@ public:
    */
   bool IsDeadKey(uint8_t aVirtualKey,
                  const ModifierKeyState& aModKeyState) const;
+  bool IsDeadKey(const NativeKey& aNativeKey) const
+  {
+    return IsDeadKey(aNativeKey.GenericVirtualKeyCode(),
+                     aNativeKey.ModifierKeyStateRef());
+  }
 
   /**
    * IsInDeadKeySequence() returns true when it's in a dead key sequence.
@@ -878,6 +909,11 @@ public:
    */
   bool IsSysKey(uint8_t aVirtualKey,
                 const ModifierKeyState& aModKeyState) const;
+  bool IsSysKey(const NativeKey& aNativeKey) const
+  {
+    return IsSysKey(aNativeKey.GenericVirtualKeyCode(),
+                    aNativeKey.ModifierKeyStateRef());
+  }
 
   /**
    * GetUniCharsAndModifiers() returns characters which are inputted by
@@ -885,24 +921,20 @@ public:
    * Note that if the combination causes text input, the result's Ctrl and
    * Alt key state are never active.
    */
-  UniCharsAndModifiers GetUniCharsAndModifiers(
+  UniCharsAndModifiers
-                         uint8_t aVirtualKey,
+  GetUniCharsAndModifiers(uint8_t aVirtualKey,
                           const ModifierKeyState& aModKeyState) const
   {
     VirtualKey::ShiftState shiftState =
       VirtualKey::ModifierKeyStateToShiftState(aModKeyState);
     return GetUniCharsAndModifiers(aVirtualKey, shiftState);
   }
-
+  UniCharsAndModifiers
-  /**
+  GetUniCharsAndModifiers(const NativeKey& aNativeKey) const
-   * GetNativeUniCharsAndModifiers() returns characters which are inputted by
+  {
-   * aVirtualKey with aModKeyState.  The method isn't stateful.
+    return GetUniCharsAndModifiers(aNativeKey.GenericVirtualKeyCode(),
-   * Note that different from GetUniCharsAndModifiers(), this returns
+                                   aNativeKey.GetShiftState());
-   * actual modifier state of Ctrl and Alt.
+  }
-   */
-  UniCharsAndModifiers GetNativeUniCharsAndModifiers(
-                         uint8_t aVirtualKey,
-                         const ModifierKeyState& aModKeyState) const;
 
   /**
    * OnLayoutChange() must be called before the first keydown message is
@@ -1015,8 +1047,7 @@ private:
   /**
    * Activates or deactivates dead key state.
    */
-  void ActivateDeadKeyState(const NativeKey& aNativeKey,
+  void ActivateDeadKeyState(const NativeKey& aNativeKey);
-                            const ModifierKeyState& aModKeyState);
   void DeactivateDeadKeyState();
 
   const DeadKeyTable* AddDeadKeyTable(const DeadKeyEntry* aDeadKeyArray,
@@ -1041,8 +1072,7 @@ private:
    * WM_KEYDOWN.  Additionally, computes current inputted character(s) and set
    * them to the aNativeKey.
    */
-  void InitNativeKey(NativeKey& aNativeKey,
+  void InitNativeKey(NativeKey& aNativeKey);
-                     const ModifierKeyState& aModKeyState);
 
   /**
    * MaybeInitNativeKeyAsDeadKey() initializes aNativeKey only when aNativeKey
@@ -1053,23 +1083,20 @@ private:
    * be caused by aNativeKey.
    * Returns true when this initializes aNativeKey.  Otherwise, false.
    */
-  bool MaybeInitNativeKeyAsDeadKey(NativeKey& aNativeKey,
+  bool MaybeInitNativeKeyAsDeadKey(NativeKey& aNativeKey);
-                                   const ModifierKeyState& aModKeyState);
 
   /**
    * MaybeInitNativeKeyWithCompositeChar() may initialize aNativeKey with
    * proper composite character when dead key produces a composite character.
    * Otherwise, just returns false.
    */
-  bool MaybeInitNativeKeyWithCompositeChar(
+  bool MaybeInitNativeKeyWithCompositeChar(NativeKey& aNativeKey);
-         NativeKey& aNativeKey,
-         const ModifierKeyState& aModKeyState);
 
   /**
    * See the comment of GetUniCharsAndModifiers() below.
    */
-  UniCharsAndModifiers GetUniCharsAndModifiers(
+  UniCharsAndModifiers
-                         uint8_t aVirtualKey,
+  GetUniCharsAndModifiers(uint8_t aVirtualKey,
                           VirtualKey::ShiftState aShiftState) const;
 
   /**