Bug 1393348 - part3: Converting SelectionType to index of Selection array in nsFrameSelection should use array instead of switch statement r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 24 Aug 2017 20:01:20 +0900
changeset 653546 b2cef3edcc430bd5d16cd7c60b3f96b3f870fc4d
parent 653545 8b6a73ca698da09867071bdf9ee7a7ef8fab783b
child 653550 5497be48fbcd39d1dd0b8ad1d3cb17133bd7a292
push id76339
push usermasayuki@d-toybox.com
push dateSat, 26 Aug 2017 02:22:48 +0000
reviewerssmaug
bugs1393348, 8848015
milestone57.0a1
Bug 1393348 - part3: Converting SelectionType to index of Selection array in nsFrameSelection should use array instead of switch statement r?smaug GetIndexFromSelectionType() in nsFrameSelection.cpp or nsFrameSelection::GetSelection() appears in profile of attachment 8848015. So, it should not use switch statement due to really hot path. With the previous patch, we can make it use array to retrieve the index from SelectionType with static_cast<int8_t>. MozReview-Commit-ID: 8jvIF5buTyT
layout/generic/nsFrameSelection.cpp
--- a/layout/generic/nsFrameSelection.cpp
+++ b/layout/generic/nsFrameSelection.cpp
@@ -139,44 +139,41 @@ nsPeekOffsetStruct::nsPeekOffsetStruct(n
   , mExtend(aExtend)
   , mResultContent()
   , mResultFrame(nullptr)
   , mContentOffset(0)
   , mAttach(CARET_ASSOCIATE_BEFORE)
 {
 }
 
-static int8_t
+// Array which contains index of each SelecionType in Selection::mDOMSelections.
+// For avoiding using if nor switch to retrieve the index, this needs to have
+// -1 for SelectionTypes which won't be created its Selection instance.
+static const int8_t kIndexOfSelections[] = {
+  -1,    // SelectionType::eInvalid
+  -1,    // SelectionType::eNone
+  0,     // SelectionType::eNormal
+  1,     // SelectionType::eSpellCheck
+  2,     // SelectionType::eIMERawClause
+  3,     // SelectionType::eIMESelectedRawClause
+  4,     // SelectionType::eIMEConvertedClause
+  5,     // SelectionType::eIMESelectedClause
+  6,     // SelectionType::eAccessibility
+  7,     // SelectionType::eFind
+  8,     // SelectionType::eURLSecondary
+  9,     // SelectionType::eURLStrikeout
+};
+
+inline int8_t
 GetIndexFromSelectionType(SelectionType aSelectionType)
 {
-  switch (aSelectionType) {
-    case SelectionType::eNormal:
-      return 0;
-    case SelectionType::eSpellCheck:
-      return 1;
-    case SelectionType::eIMERawClause:
-      return 2;
-    case SelectionType::eIMESelectedRawClause:
-      return 3;
-    case SelectionType::eIMEConvertedClause:
-      return 4;
-    case SelectionType::eIMESelectedClause:
-      return 5;
-    case SelectionType::eAccessibility:
-      return 6;
-    case SelectionType::eFind:
-      return 7;
-    case SelectionType::eURLSecondary:
-      return 8;
-    case SelectionType::eURLStrikeout:
-      return 9;
-    default:
-      return -1;
-  }
-  /* NOTREACHED */
+  // The enum value of eInvalid is -1 and the others are sequential value
+  // starting from 0.  Therefore, |SelectionType + 1| is the index of
+  // kIndexOfSelections.
+  return kIndexOfSelections[static_cast<int8_t>(aSelectionType) + 1];
 }
 
 /*
 The limiter is used specifically for the text areas and textfields
 In that case it is the DIV tag that is anonymously created for the text
 areas/fields.  Text nodes and BR nodes fall beneath it.  In the case of a
 BR node the limiter will be the parent and the offset will point before or
 after the BR node.  In the case of the text node the parent content is