Bug 1168042, restructure HandleAccessKey so that accesskey candidates are only determined once, and clean up return value to use a bool, r=masayuki
authorNeil Deakin <neil@mozilla.com>
Fri, 18 Sep 2015 08:18:07 -0400
changeset 295832 07853d7b87c1ab9c8e210b096aa24790252a92bb
parent 295831 e89d8182d588cdba43a1340b776eafccb964ed89
child 295833 b3a9162ec47badf22c806d758c8c3fc075663dda
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki
bugs1168042
milestone43.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 1168042, restructure HandleAccessKey so that accesskey candidates are only determined once, and clean up return value to use a bool, r=masayuki
dom/events/EventStateManager.cpp
dom/events/EventStateManager.h
--- a/dom/events/EventStateManager.cpp
+++ b/dom/events/EventStateManager.cpp
@@ -679,18 +679,23 @@ EventStateManager::PreHandleEvent(nsPres
         modifierMask |= NS_MODIFIER_META;
       if (keyEvent->IsOS())
         modifierMask |= NS_MODIFIER_OS;
 
       // Prevent keyboard scrolling while an accesskey modifier is in use.
       if (modifierMask &&
           (modifierMask == Prefs::ChromeAccessModifierMask() ||
            modifierMask == Prefs::ContentAccessModifierMask())) {
-        HandleAccessKey(aPresContext, keyEvent, aStatus, nullptr,
-                        eAccessKeyProcessingNormal, modifierMask);
+        nsAutoTArray<uint32_t, 10> accessCharCodes;
+        nsContentUtils::GetAccessKeyCandidates(keyEvent, accessCharCodes);
+
+        if (HandleAccessKey(aPresContext, accessCharCodes,
+                            keyEvent->mFlags.mIsTrusted, modifierMask)) {
+          *aStatus = nsEventStatus_eConsumeNoDefault;
+        }
       }
     }
     // then fall through...
   case eBeforeKeyDown:
   case eKeyDown:
   case eAfterKeyDown:
   case eBeforeKeyUp:
   case eKeyUp:
@@ -957,108 +962,98 @@ EventStateManager::GetAccessKeyLabelPref
     aPrefix.Append(modifierText + separator);
   }
   if (modifierMask & NS_MODIFIER_SHIFT) {
     nsContentUtils::GetShiftText(modifierText);
     aPrefix.Append(modifierText + separator);
   }
 }
 
-void
+bool
 EventStateManager::HandleAccessKey(nsPresContext* aPresContext,
-                                   WidgetKeyboardEvent* aEvent,
-                                   nsEventStatus* aStatus,
+                                   nsTArray<uint32_t>& aAccessCharCodes,
+                                   bool aIsTrusted,
                                    nsIDocShellTreeItem* aBubbledFrom,
                                    ProcessingAccessKeyState aAccessKeyState,
                                    int32_t aModifierMask)
 {
   nsCOMPtr<nsIDocShell> docShell = aPresContext->GetDocShell();
 
-  // Alt or other accesskey modifier is down, we may need to do an accesskey
+  if (!docShell) {
+    NS_WARNING("no docShellTreeNode for presContext");
+    return false;
+  }
+
+  // Alt or other accesskey modifier is down, we may need to do an accesskey.
   if (mAccessKeys.Count() > 0 &&
       aModifierMask == GetAccessModifierMaskFor(docShell)) {
     // Someone registered an accesskey.  Find and activate it.
-    nsAutoTArray<uint32_t, 10> accessCharCodes;
-    nsContentUtils::GetAccessKeyCandidates(aEvent, accessCharCodes);
-    if (ExecuteAccessKey(accessCharCodes, aEvent->mFlags.mIsTrusted)) {
-      *aStatus = nsEventStatus_eConsumeNoDefault;
-      return;
+    if (ExecuteAccessKey(aAccessCharCodes, aIsTrusted)) {
+      return true;
     }
   }
 
-  // after the local accesskey handling
-  if (nsEventStatus_eConsumeNoDefault != *aStatus) {
-    // checking all sub docshells
-
-    if (!docShell) {
-      NS_WARNING("no docShellTreeNode for presContext");
-      return;
+  int32_t childCount;
+  docShell->GetChildCount(&childCount);
+  for (int32_t counter = 0; counter < childCount; counter++) {
+    // Not processing the child which bubbles up the handling
+    nsCOMPtr<nsIDocShellTreeItem> subShellItem;
+    docShell->GetChildAt(counter, getter_AddRefs(subShellItem));
+    if (aAccessKeyState == eAccessKeyProcessingUp &&
+        subShellItem == aBubbledFrom) {
+      continue;
     }
 
-    int32_t childCount;
-    docShell->GetChildCount(&childCount);
-    for (int32_t counter = 0; counter < childCount; counter++) {
-      // Not processing the child which bubbles up the handling
-      nsCOMPtr<nsIDocShellTreeItem> subShellItem;
-      docShell->GetChildAt(counter, getter_AddRefs(subShellItem));
-      if (aAccessKeyState == eAccessKeyProcessingUp &&
-          subShellItem == aBubbledFrom)
+    nsCOMPtr<nsIDocShell> subDS = do_QueryInterface(subShellItem);
+    if (subDS && IsShellVisible(subDS)) {
+      nsCOMPtr<nsIPresShell> subPS = subDS->GetPresShell();
+
+      // Docshells need not have a presshell (eg. display:none
+      // iframes, docshells in transition between documents, etc).
+      if (!subPS) {
+        // Oh, well.  Just move on to the next child
         continue;
-
-      nsCOMPtr<nsIDocShell> subDS = do_QueryInterface(subShellItem);
-      if (subDS && IsShellVisible(subDS)) {
-        nsCOMPtr<nsIPresShell> subPS = subDS->GetPresShell();
-
-        // Docshells need not have a presshell (eg. display:none
-        // iframes, docshells in transition between documents, etc).
-        if (!subPS) {
-          // Oh, well.  Just move on to the next child
-          continue;
-        }
-
-        nsPresContext *subPC = subPS->GetPresContext();
-
-        EventStateManager* esm =
-          static_cast<EventStateManager*>(subPC->EventStateManager());
-
-        if (esm)
-          esm->HandleAccessKey(subPC, aEvent, aStatus, nullptr,
-                               eAccessKeyProcessingDown, aModifierMask);
-
-        if (nsEventStatus_eConsumeNoDefault == *aStatus)
-          break;
+      }
+
+      nsPresContext *subPC = subPS->GetPresContext();
+
+      EventStateManager* esm =
+        static_cast<EventStateManager*>(subPC->EventStateManager());
+
+      if (esm &&
+          esm->HandleAccessKey(subPC, aAccessCharCodes, aIsTrusted, nullptr,
+                               eAccessKeyProcessingDown, aModifierMask)) {
+        return true;
       }
     }
   }// if end . checking all sub docshell ends here.
 
   // bubble up the process to the parent docshell if necessary
-  if (eAccessKeyProcessingDown != aAccessKeyState && nsEventStatus_eConsumeNoDefault != *aStatus) {
-    if (!docShell) {
-      NS_WARNING("no docShellTreeItem for presContext");
-      return;
-    }
-
+  if (eAccessKeyProcessingDown != aAccessKeyState) {
     nsCOMPtr<nsIDocShellTreeItem> parentShellItem;
     docShell->GetParent(getter_AddRefs(parentShellItem));
     nsCOMPtr<nsIDocShell> parentDS = do_QueryInterface(parentShellItem);
     if (parentDS) {
       nsCOMPtr<nsIPresShell> parentPS = parentDS->GetPresShell();
       NS_ASSERTION(parentPS, "Our PresShell exists but the parent's does not?");
 
       nsPresContext *parentPC = parentPS->GetPresContext();
       NS_ASSERTION(parentPC, "PresShell without PresContext");
 
       EventStateManager* esm =
         static_cast<EventStateManager*>(parentPC->EventStateManager());
-
-      if (esm)
-        esm->HandleAccessKey(parentPC, aEvent, aStatus, docShell,
-                             eAccessKeyProcessingUp, aModifierMask);
+      if (esm &&
+          esm->HandleAccessKey(parentPC, aAccessCharCodes, aIsTrusted, docShell,
+                               eAccessKeyProcessingUp, aModifierMask)) {
+        return true;
+      }
     }
   }// if end. bubble up process
+
+  return false;
 }// end of HandleAccessKey
 
 bool
 EventStateManager::DispatchCrossProcessEvent(WidgetEvent* aEvent,
                                              nsFrameLoader* aFrameLoader,
                                              nsEventStatus *aStatus) {
   TabParent* remote = TabParent::GetFrom(aFrameLoader);
   if (!remote) {
--- a/dom/events/EventStateManager.h
+++ b/dom/events/EventStateManager.h
@@ -168,16 +168,25 @@ public:
    *
    * @param  aContent  the given element (must not be null)
    * @return           registered accesskey
    */
   uint32_t GetRegisteredAccessKey(nsIContent* aContent);
 
   static void GetAccessKeyLabelPrefix(dom::Element* aElement, nsAString& aPrefix);
 
+  bool HandleAccessKey(nsPresContext* aPresContext,
+                       nsTArray<uint32_t>& aAccessCharCodes,
+                       bool aIsTrusted,
+                       int32_t aModifierMask)
+  {
+    return HandleAccessKey(aPresContext, aAccessCharCodes, aIsTrusted,
+                           nullptr, eAccessKeyProcessingNormal, aModifierMask);
+  }
+
   nsresult SetCursor(int32_t aCursor, imgIContainer* aContainer,
                      bool aHaveHotspot, float aHotspotX, float aHotspotY,
                      nsIWidget* aWidget, bool aLockCursor); 
 
   static void StartHandlingUserInput()
   {
     ++sUserInputEventDepth;
     if (sUserInputEventDepth == 1) {
@@ -383,33 +392,33 @@ protected:
   /**
    * Access key handling.  If there is registered content for the accesskey
    * given by the key event and modifier mask then call
    * content.PerformAccesskey(), otherwise call HandleAccessKey() recursively,
    * on descendant docshells first, then on the ancestor (with |aBubbledFrom|
    * set to the docshell associated with |this|), until something matches.
    *
    * @param aPresContext the presentation context
-   * @param aEvent the key event
-   * @param aStatus the event status
+   * @param aAccessCharCodes list of charcode candidates
+   * @param aIsTrusted true if triggered by a trusted key event
    * @param aBubbledFrom is used by an ancestor to avoid calling HandleAccessKey()
    *        on the child the call originally came from, i.e. this is the child
    *        that recursively called us in its Up phase. The initial caller
    *        passes |nullptr| here. This is to avoid an infinite loop.
    * @param aAccessKeyState Normal, Down or Up processing phase (see enums
    *        above). The initial event receiver uses 'normal', then 'down' when
    *        processing children and Up when recursively calling its ancestor.
    * @param aModifierMask modifier mask for the key event
    */
-  void HandleAccessKey(nsPresContext* aPresContext,
-                       WidgetKeyboardEvent* aEvent,
-                       nsEventStatus* aStatus,
-                       nsIDocShellTreeItem* aBubbledFrom,
-                       ProcessingAccessKeyState aAccessKeyState,
-                       int32_t aModifierMask);
+  bool HandleAccessKey(nsPresContext* aPresContext,
+                     nsTArray<uint32_t>& aAccessCharCodes,
+                     bool aIsTrusted,
+                     nsIDocShellTreeItem* aBubbledFrom,
+                     ProcessingAccessKeyState aAccessKeyState,
+                     int32_t aModifierMask);
 
   bool ExecuteAccessKey(nsTArray<uint32_t>& aAccessCharCodes,
                           bool aIsTrustedEvent);
 
   //---------------------------------------------
   // DocShell Focus Traversal Methods
   //---------------------------------------------