Bug 1100071 patch 3: don't pass around bidi levels when we only need the direction, r=dholbert
authorSimon Montagu <smontagu@smontagu.org>
Thu, 20 Nov 2014 12:45:23 +0200
changeset 216630 1bee60dc14ec7a2563de9a8c9e412143948e5cdf
parent 216629 53fb431ea9433f4c31517760502114dcea6fbc23
child 216631 68dd715f6f79a4fc540b7fffeec047cfe4dc3b6a
push id27858
push userkwierso@gmail.com
push dateFri, 21 Nov 2014 01:35:46 +0000
treeherdermozilla-central@6309710dd71d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1100071
milestone36.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 1100071 patch 3: don't pass around bidi levels when we only need the direction, r=dholbert
layout/base/nsBidiPresUtils.h
layout/generic/nsFrame.cpp
layout/generic/nsFrameList.cpp
layout/generic/nsILineIterator.h
layout/generic/nsSelection.cpp
--- a/layout/base/nsBidiPresUtils.h
+++ b/layout/base/nsBidiPresUtils.h
@@ -281,16 +281,40 @@ public:
    */
   static uint8_t GetParagraphDepth(nsIFrame* aFrame);
 
   /**
    * Get the bidi base level of the given (inline) frame.
    */
   static nsBidiLevel GetFrameBaseLevel(nsIFrame* aFrame);
 
+  /**
+   * Get an nsBidiDirection representing the direction implied by the
+   * bidi base level of the frame.
+   * @return NSBIDI_LTR (left-to-right) or NSBIDI_RTL (right-to-left)
+   *  NSBIDI_MIXED will never be returned.
+   */
+  static nsBidiDirection ParagraphDirection(nsIFrame* aFrame) {
+    return DIRECTION_FROM_LEVEL(GetFrameBaseLevel(aFrame));
+  }
+
+  /**
+   * Get an nsBidiDirection representing the direction implied by the
+   * bidi embedding level of the frame.
+   * @return NSBIDI_LTR (left-to-right) or NSBIDI_RTL (right-to-left)
+   *  NSBIDI_MIXED will never be returned.
+   */
+  static nsBidiDirection FrameDirection(nsIFrame* aFrame) {
+    return DIRECTION_FROM_LEVEL(GetFrameEmbeddingLevel(aFrame));
+  }
+
+  static bool IsFrameInParagraphDirection(nsIFrame* aFrame) {
+    return ParagraphDirection(aFrame) == FrameDirection(aFrame);
+  }
+
   enum Mode { MODE_DRAW, MODE_MEASURE };
 
   /**
    * Reorder plain text using the Unicode Bidi algorithm and send it to
    * a processor for rendering or measuring
    *
    * @param[in] aText  the string to be processed (in logical order)
    * @param aLength the number of characters in the string
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -6648,21 +6648,24 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct*
 
       if (aPos->mVisual && PresContext()->BidiEnabled()) {
         bool lineIsRTL = it->GetDirection();
         bool isReordered;
         nsIFrame *lastFrame;
         result = it->CheckLineOrder(thisLine, &isReordered, &firstFrame, &lastFrame);
         baseFrame = endOfLine ? lastFrame : firstFrame;
         if (baseFrame) {
-          nsBidiLevel embeddingLevel = nsBidiPresUtils::GetFrameEmbeddingLevel(baseFrame);
-          // If the direction of the frame on the edge is opposite to that of the line,
-          // we'll need to drill down to its opposite end, so reverse endOfLine.
-          if (IS_LEVEL_RTL(embeddingLevel) == !lineIsRTL)
+          bool frameIsRTL =
+            (nsBidiPresUtils::FrameDirection(baseFrame) == NSBIDI_RTL);
+          // If the direction of the frame on the edge is opposite to
+          // that of the line, we'll need to drill down to its opposite
+          // end, so reverse endOfLine.
+          if (frameIsRTL != lineIsRTL) {
             endOfLine = !endOfLine;
+          }
         }
       } else {
         it->GetLine(thisLine, &firstFrame, &lineFrameCount, usedRect, &lineFlags);
 
         nsIFrame* frame = firstFrame;
         for (int32_t count = lineFrameCount; count;
              --count, frame = frame->GetNextSibling()) {
           if (!frame->IsGeneratedContentFrame()) {
@@ -6873,18 +6876,18 @@ nsIFrame::GetFrameFromDirection(nsDirect
     nsIFrame *firstFrame;
     nsIFrame *lastFrame;
     if (aVisual && presContext->BidiEnabled()) {
       bool lineIsRTL = it->GetDirection();
       bool isReordered;
       result = it->CheckLineOrder(thisLine, &isReordered, &firstFrame, &lastFrame);
       nsIFrame** framePtr = aDirection == eDirPrevious ? &firstFrame : &lastFrame;
       if (*framePtr) {
-        nsBidiLevel embeddingLevel = nsBidiPresUtils::GetFrameEmbeddingLevel(*framePtr);
-        bool frameIsRTL = IS_LEVEL_RTL(embeddingLevel);
+        bool frameIsRTL =
+          (nsBidiPresUtils::FrameDirection(*framePtr) == NSBIDI_RTL);
         if ((frameIsRTL == lineIsRTL) == (aDirection == eDirPrevious)) {
           nsFrame::GetFirstLeaf(presContext, framePtr);
         } else {
           nsFrame::GetLastLeaf(presContext, framePtr);
         }
         atLineEdge = *framePtr == traversedFrame;
       } else {
         atLineEdge = true;
--- a/layout/generic/nsFrameList.cpp
+++ b/layout/generic/nsFrameList.cpp
@@ -342,37 +342,36 @@ nsFrameList::List(FILE* out) const
 }
 #endif
 
 nsIFrame*
 nsFrameList::GetPrevVisualFor(nsIFrame* aFrame) const
 {
   if (!mFirstChild)
     return nullptr;
-  
+
   nsIFrame* parent = mFirstChild->GetParent();
   if (!parent)
     return aFrame ? aFrame->GetPrevSibling() : LastChild();
 
-  nsBidiLevel baseLevel = nsBidiPresUtils::GetFrameBaseLevel(mFirstChild);  
+  nsBidiDirection paraDir = nsBidiPresUtils::ParagraphDirection(mFirstChild);
 
   nsAutoLineIterator iter = parent->GetLineIterator();
-  if (!iter) { 
+  if (!iter) {
     // Parent is not a block Frame
     if (parent->GetType() == nsGkAtoms::lineFrame) {
       // Line frames are not bidi-splittable, so need to consider bidi reordering
-      if (baseLevel == NSBIDI_LTR) {
+      if (paraDir == NSBIDI_LTR) {
         return nsBidiPresUtils::GetFrameToLeftOf(aFrame, mFirstChild, -1);
       } else { // RTL
         return nsBidiPresUtils::GetFrameToRightOf(aFrame, mFirstChild, -1);
       }
     } else {
       // Just get the next or prev sibling, depending on block and frame direction.
-      nsBidiLevel frameEmbeddingLevel = nsBidiPresUtils::GetFrameEmbeddingLevel(mFirstChild);
-      if (IS_SAME_DIRECTION(frameEmbeddingLevel, baseLevel)) {
+      if (nsBidiPresUtils::IsFrameInParagraphDirection(mFirstChild)) {
         return aFrame ? aFrame->GetPrevSibling() : LastChild();
       } else {
         return aFrame ? aFrame->GetNextSibling() : mFirstChild;
       }
     }
   }
 
   // Parent is a block frame, so use the LineIterator to find the previous visual 
@@ -391,28 +390,28 @@ nsFrameList::GetPrevVisualFor(nsIFrame* 
   nsIFrame* firstFrameOnLine;
   int32_t numFramesOnLine;
   nsRect lineBounds;
   uint32_t lineFlags;
 
   if (aFrame) {
     iter->GetLine(thisLine, &firstFrameOnLine, &numFramesOnLine, lineBounds, &lineFlags);
 
-    if (baseLevel == NSBIDI_LTR) {
+    if (paraDir == NSBIDI_LTR) {
       frame = nsBidiPresUtils::GetFrameToLeftOf(aFrame, firstFrameOnLine, numFramesOnLine);
     } else { // RTL
       frame = nsBidiPresUtils::GetFrameToRightOf(aFrame, firstFrameOnLine, numFramesOnLine);
     }
   }
 
   if (!frame && thisLine > 0) {
     // Get the last frame of the previous line
     iter->GetLine(thisLine - 1, &firstFrameOnLine, &numFramesOnLine, lineBounds, &lineFlags);
 
-    if (baseLevel == NSBIDI_LTR) {
+    if (paraDir == NSBIDI_LTR) {
       frame = nsBidiPresUtils::GetFrameToLeftOf(nullptr, firstFrameOnLine, numFramesOnLine);
     } else { // RTL
       frame = nsBidiPresUtils::GetFrameToRightOf(nullptr, firstFrameOnLine, numFramesOnLine);
     }
   }
   return frame;
 }
 
@@ -421,32 +420,31 @@ nsFrameList::GetNextVisualFor(nsIFrame* 
 {
   if (!mFirstChild)
     return nullptr;
   
   nsIFrame* parent = mFirstChild->GetParent();
   if (!parent)
     return aFrame ? aFrame->GetPrevSibling() : mFirstChild;
 
-  nsBidiLevel baseLevel = nsBidiPresUtils::GetFrameBaseLevel(mFirstChild);
-  
+  nsBidiDirection paraDir = nsBidiPresUtils::ParagraphDirection(mFirstChild);
+
   nsAutoLineIterator iter = parent->GetLineIterator();
   if (!iter) { 
     // Parent is not a block Frame
     if (parent->GetType() == nsGkAtoms::lineFrame) {
       // Line frames are not bidi-splittable, so need to consider bidi reordering
-      if (baseLevel == NSBIDI_LTR) {
+      if (paraDir == NSBIDI_LTR) {
         return nsBidiPresUtils::GetFrameToRightOf(aFrame, mFirstChild, -1);
       } else { // RTL
         return nsBidiPresUtils::GetFrameToLeftOf(aFrame, mFirstChild, -1);
       }
     } else {
       // Just get the next or prev sibling, depending on block and frame direction.
-      nsBidiLevel frameEmbeddingLevel = nsBidiPresUtils::GetFrameEmbeddingLevel(mFirstChild);
-      if (IS_SAME_DIRECTION(frameEmbeddingLevel, baseLevel)) {
+      if (nsBidiPresUtils::IsFrameInParagraphDirection(mFirstChild)) {
         return aFrame ? aFrame->GetNextSibling() : mFirstChild;
       } else {
         return aFrame ? aFrame->GetPrevSibling() : LastChild();
       }
     }
   }
 
   // Parent is a block frame, so use the LineIterator to find the next visual 
@@ -464,30 +462,30 @@ nsFrameList::GetNextVisualFor(nsIFrame* 
   nsIFrame* frame = nullptr;
   nsIFrame* firstFrameOnLine;
   int32_t numFramesOnLine;
   nsRect lineBounds;
   uint32_t lineFlags;
 
   if (aFrame) {
     iter->GetLine(thisLine, &firstFrameOnLine, &numFramesOnLine, lineBounds, &lineFlags);
-    
-    if (baseLevel == NSBIDI_LTR) {
+
+    if (paraDir == NSBIDI_LTR) {
       frame = nsBidiPresUtils::GetFrameToRightOf(aFrame, firstFrameOnLine, numFramesOnLine);
     } else { // RTL
       frame = nsBidiPresUtils::GetFrameToLeftOf(aFrame, firstFrameOnLine, numFramesOnLine);
     }
   }
-  
+
   int32_t numLines = iter->GetNumLines();
   if (!frame && thisLine < numLines - 1) {
     // Get the first frame of the next line
     iter->GetLine(thisLine + 1, &firstFrameOnLine, &numFramesOnLine, lineBounds, &lineFlags);
-    
-    if (baseLevel == NSBIDI_LTR) {
+
+    if (paraDir == NSBIDI_LTR) {
       frame = nsBidiPresUtils::GetFrameToRightOf(nullptr, firstFrameOnLine, numFramesOnLine);
     } else { // RTL
       frame = nsBidiPresUtils::GetFrameToLeftOf(nullptr, firstFrameOnLine, numFramesOnLine);
     }
   }
   return frame;
 }
 
--- a/layout/generic/nsILineIterator.h
+++ b/layout/generic/nsILineIterator.h
@@ -44,16 +44,18 @@ public:
    */
   virtual int32_t GetNumLines() = 0;
 
   /**
    * The prevailing direction of lines.
    *
    * @return true if the CSS direction property for the block is
    *         "rtl", otherwise false
+   *
+   *XXX after bug 924851 change this to return a UBiDiDirection
    */
   virtual bool GetDirection() = 0;
 
   // Return structural information about a line. aFirstFrameOnLine is
   // the first frame on the line and aNumFramesOnLine is the number of
   // frames that are on the line. If the line-number is invalid then
   // aFirstFrameOnLine will be nullptr and aNumFramesOnLine will be
   // zero.
--- a/layout/generic/nsSelection.cpp
+++ b/layout/generic/nsSelection.cpp
@@ -865,27 +865,27 @@ nsFrameSelection::MoveCaret(uint32_t    
   if (NS_FAILED(result) || !frame)
     return NS_FAILED(result) ? result : NS_ERROR_FAILURE;
 
   //set data using mLimiter to stop on scroll views.  If we have a limiter then we stop peeking
   //when we hit scrollable views.  If no limiter then just let it go ahead
   nsPeekOffsetStruct pos(aAmount, eDirPrevious, offsetused, desiredX,
                          true, mLimiter != nullptr, true, aVisualMovement);
 
-  nsBidiLevel baseLevel = nsBidiPresUtils::GetFrameBaseLevel(frame);
-  
+  nsBidiDirection paraDir = nsBidiPresUtils::ParagraphDirection(frame);
+
   CaretAssociateHint tHint(mHint); //temporary variable so we dont set mHint until it is necessary
   switch (aKeycode){
-    case nsIDOMKeyEvent::DOM_VK_RIGHT : 
+    case nsIDOMKeyEvent::DOM_VK_RIGHT :
         InvalidateDesiredX();
-        pos.mDirection = IS_LEVEL_RTL(baseLevel) ? eDirPrevious : eDirNext;
+        pos.mDirection = (paraDir == NSBIDI_RTL) ? eDirPrevious : eDirNext;
       break;
     case nsIDOMKeyEvent::DOM_VK_LEFT :
         InvalidateDesiredX();
-        pos.mDirection = IS_LEVEL_RTL(baseLevel) ? eDirNext : eDirPrevious;
+        pos.mDirection = (paraDir == NSBIDI_RTL) ? eDirNext : eDirPrevious;
       break;
     case nsIDOMKeyEvent::DOM_VK_DELETE :
         InvalidateDesiredX();
         pos.mDirection = eDirNext;
       break;
     case nsIDOMKeyEvent::DOM_VK_BACK_SPACE : 
         InvalidateDesiredX();
         pos.mDirection = eDirPrevious;
@@ -5820,25 +5820,25 @@ Selection::Modify(const nsAString& aAlte
     if (!focusNode) {
       aRv.Throw(NS_ERROR_UNEXPECTED);
       return;
     }
     uint32_t focusOffset = FocusOffset();
     Collapse(focusNode, focusOffset);
   }
 
-  // If the base level of the focused frame is odd, we may have to swap the
-  // direction of the keycode.
+  // If the paragraph direction of the focused frame is right-to-left,
+  // we may have to swap the direction of the keycode.
   nsIFrame *frame;
   int32_t offset;
   rv = GetPrimaryFrameForFocusNode(&frame, &offset, visual);
   if (NS_SUCCEEDED(rv) && frame) {
-    nsBidiLevel baseLevel = nsBidiPresUtils::GetFrameBaseLevel(frame);
-
-    if (IS_LEVEL_RTL(baseLevel)) {
+    nsBidiDirection paraDir = nsBidiPresUtils::ParagraphDirection(frame);
+
+    if (paraDir == NSBIDI_RTL) {
       if (!visual && keycode == nsIDOMKeyEvent::DOM_VK_RIGHT) {
         keycode = nsIDOMKeyEvent::DOM_VK_LEFT;
       }
       else if (!visual && keycode == nsIDOMKeyEvent::DOM_VK_LEFT) {
         keycode = nsIDOMKeyEvent::DOM_VK_RIGHT;
       }
       else if (visual && keycode == nsIDOMKeyEvent::DOM_VK_HOME) {
         keycode = nsIDOMKeyEvent::DOM_VK_END;