Bug 1308359 - patch 2 - Move trivial ICU-wrapper nsBidi methods to the header file as inlines, and remove unnecessary nsresult return values from methods that cannot fail. r=xidorn
authorJonathan Kew <jkew@mozilla.com>
Fri, 22 Sep 2017 10:37:17 +0100
changeset 669134 593bddd0b6bd812e16c9a882ce627e31bc65d985
parent 669133 5bf8977ee7a0c6b10e5f551018cceccd0039217a
child 669135 513b9e8f28e64f26bf268f76c86105d439201ec8
push id81223
push usergpascutto@mozilla.com
push dateFri, 22 Sep 2017 15:21:17 +0000
reviewersxidorn
bugs1308359
milestone58.0a1
Bug 1308359 - patch 2 - Move trivial ICU-wrapper nsBidi methods to the header file as inlines, and remove unnecessary nsresult return values from methods that cannot fail. r=xidorn
layout/base/moz.build
layout/base/nsBidi.cpp
layout/base/nsBidi.h
layout/base/nsBidiPresUtils.cpp
--- a/layout/base/moz.build
+++ b/layout/base/moz.build
@@ -88,17 +88,16 @@ EXPORTS.mozilla += [
 UNIFIED_SOURCES += [
     'AccessibleCaret.cpp',
     'AccessibleCaretEventHub.cpp',
     'AccessibleCaretManager.cpp',
     'GeckoRestyleManager.cpp',
     'GeometryUtils.cpp',
     'LayoutLogging.cpp',
     'MobileViewportManager.cpp',
-    'nsBidi.cpp',
     'nsBidiPresUtils.cpp',
     'nsCaret.cpp',
     'nsCounterManager.cpp',
     'nsCSSColorUtils.cpp',
     'nsCSSFrameConstructor.cpp',
     'nsDocumentViewer.cpp',
     'nsFrameManager.cpp',
     'nsFrameTraversal.cpp',
deleted file mode 100644
--- a/layout/base/nsBidi.cpp
+++ /dev/null
@@ -1,68 +0,0 @@
-/* -*- Mode: C; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
- *
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include "nsBidi.h"
-#include "ICUUtils.h"
-
-nsBidi::nsBidi()
-{
-  mBiDi = ubidi_open();
-}
-
-nsBidi::~nsBidi()
-{
-  ubidi_close(mBiDi);
-}
-
-nsresult nsBidi::SetPara(const char16_t *aText, int32_t aLength,
-                         nsBidiLevel aParaLevel)
-{
-  UErrorCode error = U_ZERO_ERROR;
-  ubidi_setPara(mBiDi, reinterpret_cast<const UChar*>(aText), aLength,
-                aParaLevel, nullptr, &error);
-  return ICUUtils::UErrorToNsResult(error);
-}
-
-nsresult nsBidi::GetDirection(nsBidiDirection* aDirection)
-{
-  *aDirection = nsBidiDirection(ubidi_getDirection(mBiDi));
-  return NS_OK;
-}
-
-nsresult nsBidi::GetParaLevel(nsBidiLevel* aParaLevel)
-{
-  *aParaLevel = ubidi_getParaLevel(mBiDi);
-  return NS_OK;
-}
-
-nsresult nsBidi::GetLogicalRun(int32_t aLogicalStart, int32_t* aLogicalLimit,
-                               nsBidiLevel* aLevel)
-{
-  ubidi_getLogicalRun(mBiDi, aLogicalStart, aLogicalLimit, aLevel);
-  return NS_OK;
-}
-
-nsresult nsBidi::CountRuns(int32_t* aRunCount)
-{
-  UErrorCode errorCode = U_ZERO_ERROR;
-  *aRunCount = ubidi_countRuns(mBiDi, &errorCode);
-  return ICUUtils::UErrorToNsResult(errorCode);
-}
-
-nsresult nsBidi::GetVisualRun(int32_t aRunIndex, int32_t* aLogicalStart,
-                              int32_t* aLength, nsBidiDirection* aDirection)
-{
-  *aDirection = nsBidiDirection(ubidi_getVisualRun(mBiDi, aRunIndex,
-                                                   aLogicalStart, aLength));
-  return NS_OK;
-}
-
-nsresult nsBidi::ReorderVisual(const nsBidiLevel* aLevels, int32_t aLength,
-                               int32_t* aIndexMap)
-{
-  ubidi_reorderVisual(aLevels, aLength, aIndexMap);
-  return NS_OK;
-}
--- a/layout/base/nsBidi.h
+++ b/layout/base/nsBidi.h
@@ -3,36 +3,42 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef nsBidi_h__
 #define nsBidi_h__
 
 #include "unicode/ubidi.h"
+#include "ICUUtils.h"
 #include "nsIFrame.h" // for nsBidiLevel/nsBidiDirection declarations
 
 // nsBidi implemented as a simple wrapper around the bidi reordering engine
 // from ICU.
 // We could eliminate this and let callers use the ICU functions directly
 // once we no longer care about building without ICU available.
 
 class nsBidi
 {
 public:
   /** @brief Default constructor.
    *
    * The nsBidi object is initially empty. It is assigned
    * the Bidi properties of a paragraph by <code>SetPara()</code>.
    */
-  explicit nsBidi();
+  nsBidi()
+  {
+    mBiDi = ubidi_open();
+  }
 
   /** @brief Destructor. */
-  virtual ~nsBidi();
-
+  ~nsBidi()
+  {
+    ubidi_close(mBiDi);
+  }
 
   /**
    * Perform the Unicode Bidi algorithm.
    *
    * @param aText is a pointer to the single-paragraph text that the
    *      Bidi algorithm will be performed on
    *      (step (P1) of the algorithm is performed externally).
    *      <strong>The text must be (at least) <code>aLength</code> long.</strong>
@@ -46,39 +52,51 @@ public:
    *      then <code>aParaLevel</code> can be set to
    *      either <code>NSBIDI_DEFAULT_LTR</code>
    *      or <code>NSBIDI_DEFAULT_RTL</code>;
    *      if there is no strongly typed character, then
    *      the desired default is used (0 for LTR or 1 for RTL).
    *      Any other value between 0 and <code>NSBIDI_MAX_EXPLICIT_LEVEL</code>
    *      is also valid, with odd levels indicating RTL.
    */
-  nsresult SetPara(const char16_t *aText, int32_t aLength,
-                   nsBidiLevel aParaLevel);
+  nsresult SetPara(const char16_t* aText, int32_t aLength,
+                   nsBidiLevel aParaLevel)
+  {
+    UErrorCode error = U_ZERO_ERROR;
+    ubidi_setPara(mBiDi, reinterpret_cast<const UChar*>(aText), aLength,
+                  aParaLevel, nullptr, &error);
+    return ICUUtils::UErrorToNsResult(error);
+  }
 
   /**
    * Get the directionality of the text.
    *
    * @param aDirection receives a <code>NSBIDI_XXX</code> value that indicates
    *       if the entire text represented by this object is unidirectional,
    *       and which direction, or if it is mixed-directional.
    *
    * @see nsBidiDirection
    */
-  nsresult GetDirection(nsBidiDirection* aDirection);
+  nsBidiDirection GetDirection()
+  {
+    return nsBidiDirection(ubidi_getDirection(mBiDi));
+  }
 
   /**
    * Get the paragraph level of the text.
    *
    * @param aParaLevel receives a <code>NSBIDI_XXX</code> value indicating
    *                   the paragraph level
    *
    * @see nsBidiLevel
    */
-  nsresult GetParaLevel(nsBidiLevel* aParaLevel);
+  nsBidiLevel GetParaLevel()
+  {
+    return ubidi_getParaLevel(mBiDi);
+  }
 
   /**
    * Get a logical run.
    * This function returns information about a run and is used
    * to retrieve runs in logical order.<p>
    * This is especially useful for line-breaking on a paragraph.
    *
    * @param aLogicalStart is the first character of the run.
@@ -89,30 +107,38 @@ public:
    *      <code>aLogicalStart</code>.
    *      This pointer can be <code>nullptr</code> if this
    *      value is not necessary.
    *
    * @param aLevel will receive the level of the run.
    *      This pointer can be <code>nullptr</code> if this
    *      value is not necessary.
    */
-  nsresult GetLogicalRun(int32_t aLogicalStart, int32_t* aLogicalLimit,
-                         nsBidiLevel* aLevel);
+  void GetLogicalRun(int32_t aLogicalStart, int32_t* aLogicalLimit,
+                     nsBidiLevel* aLevel)
+  {
+    ubidi_getLogicalRun(mBiDi, aLogicalStart, aLogicalLimit, aLevel);
+  }
 
   /**
    * Get the number of runs.
    * This function may invoke the actual reordering on the
    * <code>nsBidi</code> object, after <code>SetPara</code>
    * may have resolved only the levels of the text. Therefore,
    * <code>CountRuns</code> may have to allocate memory,
    * and may fail doing so.
    *
    * @param aRunCount will receive the number of runs.
    */
-  nsresult CountRuns(int32_t* aRunCount);
+  nsresult CountRuns(int32_t* aRunCount)
+  {
+    UErrorCode errorCode = U_ZERO_ERROR;
+    *aRunCount = ubidi_countRuns(mBiDi, &errorCode);
+    return ICUUtils::UErrorToNsResult(errorCode);
+  }
 
   /**
    * Get one run's logical start, length, and directionality,
    * which can be 0 for LTR or 1 for RTL.
    * In an RTL run, the character at the logical start is
    * visually on the right of the displayed run.
    * The length is the number of characters in the run.<p>
    * <code>CountRuns</code> should be called
@@ -122,29 +148,29 @@ public:
    *      range <code>[0..CountRuns-1]</code>.
    *
    * @param aLogicalStart is the first logical character index in the text.
    *      The pointer may be <code>nullptr</code> if this index is not needed.
    *
    * @param aLength is the number of characters (at least one) in the run.
    *      The pointer may be <code>nullptr</code> if this is not needed.
    *
-   * @param aDirection will receive the directionality of the run,
+   * @returns the directionality of the run,
    *       <code>NSBIDI_LTR==0</code> or <code>NSBIDI_RTL==1</code>,
    *       never <code>NSBIDI_MIXED</code>.
    *
    * @see CountRuns<p>
    *
    * Example:
    * @code
    *  int32_t i, count, logicalStart, visualIndex=0, length;
    *  nsBidiDirection dir;
    *  pBidi->CountRuns(&count);
    *  for(i=0; i<count; ++i) {
-   *    pBidi->GetVisualRun(i, &logicalStart, &length, &dir);
+   *    dir = pBidi->GetVisualRun(i, &logicalStart, &length);
    *    if(NSBIDI_LTR==dir) {
    *      do { // LTR
    *        show_char(text[logicalStart++], visualIndex++);
    *      } while(--length>0);
    *    } else {
    *      logicalStart+=length;  // logicalLimit
    *      do { // RTL
    *        show_char(text[--logicalStart], visualIndex++);
@@ -152,18 +178,22 @@ public:
    *    }
    *  }
    * @endcode
    *
    * Note that in right-to-left runs, code like this places
    * modifier letters before base characters and second surrogates
    * before first ones.
    */
-  nsresult GetVisualRun(int32_t aRunIndex, int32_t* aLogicalStart,
-                        int32_t* aLength, nsBidiDirection* aDirection);
+  nsBidiDirection GetVisualRun(int32_t aRunIndex,
+                               int32_t* aLogicalStart, int32_t* aLength)
+  {
+    return nsBidiDirection(ubidi_getVisualRun(mBiDi, aRunIndex,
+                                              aLogicalStart, aLength));
+  }
 
   /**
    * This is a convenience function that does not use a nsBidi object.
    * It is intended to be used for when an application has determined the levels
    * of objects (character sequences) and just needs to have them reordered (L2).
    * This is equivalent to using <code>GetVisualMap</code> on a
    * <code>nsBidi</code> object.
    *
@@ -175,20 +205,22 @@ public:
    *      It must be <code>aLength>0</code>.
    *
    * @param aIndexMap is a pointer to an array of <code>aLength</code>
    *      indexes which will reflect the reordering of the characters.
    *      The array does not need to be initialized.<p>
    *      The index map will result in
    *        <code>aIndexMap[aVisualIndex]==aLogicalIndex</code>.
    */
-  static nsresult ReorderVisual(const nsBidiLevel* aLevels, int32_t aLength,
-                                int32_t* aIndexMap);
+  static void ReorderVisual(const nsBidiLevel* aLevels, int32_t aLength,
+                            int32_t* aIndexMap)
+  {
+    ubidi_reorderVisual(aLevels, aLength, aIndexMap);
+  }
 
 private:
   nsBidi(const nsBidi&) = delete;
   void operator=(const nsBidi&) = delete;
 
-protected:
   UBiDi* mBiDi;
 };
 
 #endif // _nsBidi_h_
--- a/layout/base/nsBidiPresUtils.cpp
+++ b/layout/base/nsBidiPresUtils.cpp
@@ -198,43 +198,40 @@ struct MOZ_STACK_CLASS BidiParagraphData
    * mParaLevel can be NSBIDI_DEFAULT_LTR as well as NSBIDI_LTR or NSBIDI_RTL.
    * GetParaLevel() returns the actual (resolved) paragraph level which is
    * always either NSBIDI_LTR or NSBIDI_RTL
    */
   nsBidiLevel GetParaLevel()
   {
     nsBidiLevel paraLevel = mParaLevel;
     if (paraLevel == NSBIDI_DEFAULT_LTR || paraLevel == NSBIDI_DEFAULT_RTL) {
-      mPresContext->GetBidiEngine().GetParaLevel(&paraLevel);
+      paraLevel = mPresContext->GetBidiEngine().GetParaLevel();
     }
     return paraLevel;
   }
 
   nsBidiDirection GetDirection()
   {
-    nsBidiDirection dir;
-    mPresContext->GetBidiEngine().GetDirection(&dir);
-    return dir;
+    return mPresContext->GetBidiEngine().GetDirection();
   }
 
   nsresult CountRuns(int32_t *runCount)
   {
     return mPresContext->GetBidiEngine().CountRuns(runCount);
   }
 
-  nsresult GetLogicalRun(int32_t aLogicalStart,
-                         int32_t* aLogicalLimit,
-                         nsBidiLevel* aLevel)
+  void GetLogicalRun(int32_t aLogicalStart,
+                     int32_t* aLogicalLimit,
+                     nsBidiLevel* aLevel)
   {
-    nsresult rv =
-      mPresContext->GetBidiEngine().GetLogicalRun(aLogicalStart,
-                                                   aLogicalLimit, aLevel);
-    if (mIsVisual || NS_FAILED(rv))
+    mPresContext->GetBidiEngine().GetLogicalRun(aLogicalStart,
+                                                aLogicalLimit, aLevel);
+    if (mIsVisual) {
       *aLevel = GetParaLevel();
-    return rv;
+    }
   }
 
   void ResetData()
   {
     mLogicalFrames.Clear();
     mLinePerFrame.Clear();
     mContentToFrameIndex.Clear();
     mBuffer.SetLength(0);
@@ -888,20 +885,17 @@ nsBidiPresUtils::ResolveParagraph(BidiPa
     } // if (fragmentLength <= 0)
 
     if (runLength <= 0) {
       // Get the next run of text from the Bidi engine
       if (++numRun >= runCount) {
         break;
       }
       int32_t lineOffset = logicalLimit;
-      if (NS_FAILED(aBpd->GetLogicalRun(
-              lineOffset, &logicalLimit, &embeddingLevel) ) ) {
-        break;
-      }
+      aBpd->GetLogicalRun(lineOffset, &logicalLimit, &embeddingLevel);
       runLength = logicalLimit - lineOffset;
     } // if (runLength <= 0)
 
     if (frame == NS_BIDI_CONTROL_FRAME) {
       // In theory, we only need to do this for isolates. However, it is
       // easier to do this for all here because we do not maintain the
       // index to get corresponding character from buffer. Since we do
       // have proper embedding level for all those characters, including
@@ -2120,25 +2114,20 @@ nsresult nsBidiPresUtils::ProcessText(co
   for(int nPosResolve=0; nPosResolve < aPosResolveCount; ++nPosResolve)
   {
     aPosResolve[nPosResolve].visualIndex = kNotFound;
     aPosResolve[nPosResolve].visualLeftTwips = kNotFound;
     aPosResolve[nPosResolve].visualWidth = kNotFound;
   }
 
   for (i = 0; i < runCount; i++) {
-    nsBidiDirection dir;
-    rv = aBidiEngine->GetVisualRun(i, &start, &length, &dir);
-    if (NS_FAILED(rv))
-      return rv;
+    nsBidiDirection dir = aBidiEngine->GetVisualRun(i, &start, &length);
 
     nsBidiLevel level;
-    rv = aBidiEngine->GetLogicalRun(start, &limit, &level);
-    if (NS_FAILED(rv))
-      return rv;
+    aBidiEngine->GetLogicalRun(start, &limit, &level);
 
     dir = DIRECTION_FROM_LEVEL(level);
     int32_t subRunLength = limit - start;
     int32_t lineOffset = start;
     int32_t typeLimit = std::min(limit, aLength);
     int32_t subRunCount = 1;
     int32_t subRunLimit = typeLimit;