Bug 1120313 - Fix nested ruby inside ruby annotation. r=dbaron
authorXidorn Quan <quanxunzhen@gmail.com>
Tue, 13 Jan 2015 15:14:46 +1100
changeset 223471 c4553528cfdd8d9f858f92d15562890e5777ee22
parent 223470 0638060e5fbacd11a0ad4988d06211901c204893
child 223472 700b10b9be8ae95f588335255923034040bd3328
push id28095
push usercbook@mozilla.com
push dateTue, 13 Jan 2015 13:24:48 +0000
treeherdermozilla-central@a5700bec72e1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1120313
milestone38.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 1120313 - Fix nested ruby inside ruby annotation. r=dbaron
layout/generic/nsLineLayout.cpp
layout/generic/nsLineLayout.h
layout/reftests/css-ruby/nested-ruby-1.html
layout/reftests/css-ruby/reftest.list
--- a/layout/generic/nsLineLayout.cpp
+++ b/layout/generic/nsLineLayout.cpp
@@ -53,17 +53,17 @@ using namespace mozilla;
 nsLineLayout::nsLineLayout(nsPresContext* aPresContext,
                            nsFloatManager* aFloatManager,
                            const nsHTMLReflowState* aOuterReflowState,
                            const nsLineList::iterator* aLine,
                            nsLineLayout* aBaseLineLayout)
   : mPresContext(aPresContext),
     mFloatManager(aFloatManager),
     mBlockReflowState(aOuterReflowState),
-    mBaseLineLayout(aBaseLineLayout ? aBaseLineLayout->mBaseLineLayout : this),
+    mBaseLineLayout(aBaseLineLayout),
     mLastOptionalBreakFrame(nullptr),
     mForceBreakFrame(nullptr),
     mBlockRS(nullptr),/* XXX temporary */
     mLastOptionalBreakPriority(gfxBreakPriority::eNoBreak),
     mLastOptionalBreakFrameOffset(-1),
     mForceBreakFrameOffset(-1),
     mMinLineBSize(0),
     mTextIndent(0),
@@ -82,16 +82,21 @@ nsLineLayout::nsLineLayout(nsPresContext
     mDirtyNextLine(false),
     mLineAtStart(false),
     mHasRuby(false)
 {
   MOZ_ASSERT(aOuterReflowState, "aOuterReflowState must not be null");
   NS_ASSERTION(aFloatManager || aOuterReflowState->frame->GetType() ==
                                   nsGkAtoms::letterFrame,
                "float manager should be present");
+  MOZ_ASSERT((!!mBaseLineLayout) ==
+             (aOuterReflowState->frame->GetType() ==
+              nsGkAtoms::rubyTextContainerFrame),
+             "Only ruby text container frames have "
+             "a different base line layout");
   MOZ_COUNT_CTOR(nsLineLayout);
 
   // Stash away some style data that we need
   nsBlockFrame* blockFrame = do_QueryFrame(aOuterReflowState->frame);
   if (blockFrame)
     mStyleText = blockFrame->StyleTextForLineLayout();
   else
     mStyleText = aOuterReflowState->frame->StyleText();
@@ -177,17 +182,17 @@ nsLineLayout::BeginLineReflow(nscoord aI
 #ifdef DEBUG
   mSpansAllocated = mSpansFreed = mFramesAllocated = mFramesFreed = 0;
 #endif
 
   mFirstLetterStyleOK = false;
   mIsTopOfPage = aIsTopOfPage;
   mImpactedByFloats = aImpactedByFloats;
   mTotalPlacedFrames = 0;
-  if (mBaseLineLayout == this) {
+  if (!mBaseLineLayout) {
     mLineIsEmpty = true;
     mLineAtStart = true;
   } else {
     mLineIsEmpty = false;
     mLineAtStart = false;
   }
   mLineEndsInBR = false;
   mSpanDepth = 0;
@@ -258,17 +263,17 @@ nsLineLayout::BeginLineReflow(nscoord aI
 void
 nsLineLayout::EndLineReflow()
 {
 #ifdef NOISY_REFLOW
   nsFrame::ListTag(stdout, mBlockReflowState->frame);
   printf(": EndLineReflow: width=%d\n", mRootSpan->mICoord - mRootSpan->mIStart);
 #endif
 
-  NS_ASSERTION(mBaseLineLayout == this ||
+  NS_ASSERTION(!mBaseLineLayout ||
                (!mSpansAllocated && !mSpansFreed && !mSpanFreeList &&
                 !mFramesAllocated && !mFramesFreed && !mFrameFreeList),
                "Allocated frames or spans on non-base line layout?");
 
   UnlinkFrame(mRootSpan->mFrame);
   mCurrentSpan = mRootSpan = nullptr;
 
   NS_ASSERTION(mSpansAllocated == mSpansFreed, "leak");
@@ -381,39 +386,40 @@ nsLineLayout::UpdateBand(WritingMode aWM
   mImpactedByFloats = true;
 
   mLastFloatWasLetterFrame = nsGkAtoms::letterFrame == aFloatFrame->GetType();
 }
 
 nsLineLayout::PerSpanData*
 nsLineLayout::NewPerSpanData()
 {
-  PerSpanData* psd = mBaseLineLayout->mSpanFreeList;
+  nsLineLayout* outerLineLayout = GetOutermostLineLayout();
+  PerSpanData* psd = outerLineLayout->mSpanFreeList;
   if (!psd) {
     void *mem;
     size_t sz = sizeof(PerSpanData);
-    PL_ARENA_ALLOCATE(mem, &mBaseLineLayout->mArena, sz);
+    PL_ARENA_ALLOCATE(mem, &outerLineLayout->mArena, sz);
     if (!mem) {
       NS_ABORT_OOM(sz);
     }
     psd = reinterpret_cast<PerSpanData*>(mem);
   }
   else {
-    mBaseLineLayout->mSpanFreeList = psd->mNextFreeSpan;
+    outerLineLayout->mSpanFreeList = psd->mNextFreeSpan;
   }
   psd->mParent = nullptr;
   psd->mFrame = nullptr;
   psd->mFirstFrame = nullptr;
   psd->mLastFrame = nullptr;
   psd->mContainsFloat = false;
   psd->mZeroEffectiveSpanBox = false;
   psd->mHasNonemptyContent = false;
 
 #ifdef DEBUG
-  mBaseLineLayout->mSpansAllocated++;
+  outerLineLayout->mSpansAllocated++;
 #endif
   return psd;
 }
 
 void
 nsLineLayout::BeginSpan(nsIFrame* aFrame,
                         const nsHTMLReflowState* aSpanReflowState,
                         nscoord aIStart, nscoord aIEnd,
@@ -468,23 +474,31 @@ nsLineLayout::EndSpan(nsIFrame* aFrame)
   mCurrentSpan->mReflowState = nullptr;  // no longer valid so null it out!
   mCurrentSpan = mCurrentSpan->mParent;
   return iSizeResult;
 }
 
 void
 nsLineLayout::AttachFrameToBaseLineLayout(PerFrameData* aFrame)
 {
-  NS_PRECONDITION(this != mBaseLineLayout,
+  NS_PRECONDITION(mBaseLineLayout,
                   "This method must not be called in a base line layout.");
 
   PerFrameData* baseFrame = mBaseLineLayout->LastFrame();
   MOZ_ASSERT(aFrame && baseFrame);
   MOZ_ASSERT(!aFrame->mIsLinkedToBase,
              "The frame must not have been linked with the base");
+#ifdef DEBUG
+  nsIAtom* baseType = baseFrame->mFrame->GetType();
+  nsIAtom* annotationType = aFrame->mFrame->GetType();
+  MOZ_ASSERT((baseType == nsGkAtoms::rubyBaseContainerFrame &&
+              annotationType == nsGkAtoms::rubyTextContainerFrame) ||
+             (baseType == nsGkAtoms::rubyBaseFrame &&
+              annotationType == nsGkAtoms::rubyTextFrame));
+#endif
 
   aFrame->mNextAnnotation = baseFrame->mNextAnnotation;
   baseFrame->mNextAnnotation = aFrame;
   aFrame->mIsLinkedToBase = true;
 }
 
 int32_t
 nsLineLayout::GetCurrentSpanCount() const
@@ -598,34 +612,36 @@ nsLineLayout::UnlinkFrame(PerFrameData* 
 }
 
 void
 nsLineLayout::FreeFrame(PerFrameData* pfd)
 {
   if (nullptr != pfd->mSpan) {
     FreeSpan(pfd->mSpan);
   }
-  pfd->mNext = mBaseLineLayout->mFrameFreeList;
-  mBaseLineLayout->mFrameFreeList = pfd;
+  nsLineLayout* outerLineLayout = GetOutermostLineLayout();
+  pfd->mNext = outerLineLayout->mFrameFreeList;
+  outerLineLayout->mFrameFreeList = pfd;
 #ifdef DEBUG
-  mBaseLineLayout->mFramesFreed++;
+  outerLineLayout->mFramesFreed++;
 #endif
 }
 
 void
 nsLineLayout::FreeSpan(PerSpanData* psd)
 {
   // Unlink its frames
   UnlinkFrame(psd->mFirstFrame);
 
+  nsLineLayout* outerLineLayout = GetOutermostLineLayout();
   // Now put the span on the free list since it's free too
-  psd->mNextFreeSpan = mBaseLineLayout->mSpanFreeList;
-  mBaseLineLayout->mSpanFreeList = psd;
+  psd->mNextFreeSpan = outerLineLayout->mSpanFreeList;
+  outerLineLayout->mSpanFreeList = psd;
 #ifdef DEBUG
-  mBaseLineLayout->mSpansFreed++;
+  outerLineLayout->mSpansFreed++;
 #endif
 }
 
 bool
 nsLineLayout::IsZeroBSize()
 {
   PerSpanData* psd = mCurrentSpan;
   PerFrameData* pfd = psd->mFirstFrame;
@@ -636,28 +652,29 @@ nsLineLayout::IsZeroBSize()
     pfd = pfd->mNext;
   }
   return true;
 }
 
 nsLineLayout::PerFrameData*
 nsLineLayout::NewPerFrameData(nsIFrame* aFrame)
 {
-  PerFrameData* pfd = mBaseLineLayout->mFrameFreeList;
+  nsLineLayout* outerLineLayout = GetOutermostLineLayout();
+  PerFrameData* pfd = outerLineLayout->mFrameFreeList;
   if (!pfd) {
     void *mem;
     size_t sz = sizeof(PerFrameData);
-    PL_ARENA_ALLOCATE(mem, &mBaseLineLayout->mArena, sz);
+    PL_ARENA_ALLOCATE(mem, &outerLineLayout->mArena, sz);
     if (!mem) {
       NS_ABORT_OOM(sz);
     }
     pfd = reinterpret_cast<PerFrameData*>(mem);
   }
   else {
-    mBaseLineLayout->mFrameFreeList = pfd->mNext;
+    outerLineLayout->mFrameFreeList = pfd->mNext;
   }
   pfd->mSpan = nullptr;
   pfd->mNext = nullptr;
   pfd->mPrev = nullptr;
   pfd->mNextAnnotation = nullptr;
   pfd->mFrame = aFrame;
 
   // all flags default to false
@@ -679,17 +696,17 @@ nsLineLayout::NewPerFrameData(nsIFrame* 
   pfd->mBorderPadding = LogicalMargin(lineWM);
   pfd->mOffsets = LogicalMargin(frameWM);
 
   pfd->mJustificationInfo = JustificationInfo();
   pfd->mJustificationAssignment = JustificationAssignment();
 
 #ifdef DEBUG
   pfd->mBlockDirAlign = 0xFF;
-  mBaseLineLayout->mFramesAllocated++;
+  outerLineLayout->mFramesAllocated++;
 #endif
   return pfd;
 }
 
 bool
 nsLineLayout::LineIsBreakable() const
 {
   // XXX mTotalPlacedFrames should go away and we should just use
@@ -965,17 +982,18 @@ nsLineLayout::ReflowFrame(nsIFrame* aFra
           // so we can't know whether the float plus that content will fit
           // on the line. So for now, don't place floats after inline
           // content where there's no break opportunity. This is incorrect
           // but hopefully rare. Fixing it will require significant
           // restructuring of line layout.
           // We might as well allow zero-width floats to be placed, though.
           availableISize = 0;
         }
-        placedFloat = mBaseLineLayout->AddFloat(outOfFlowFrame, availableISize);
+        placedFloat = GetOutermostLineLayout()->
+          AddFloat(outOfFlowFrame, availableISize);
         NS_ASSERTION(!(outOfFlowFrame->GetType() == nsGkAtoms::letterFrame &&
                        GetFirstLetterStyleOK()),
                     "FirstLetterStyle set on line with floating first letter");
       }
     }
     else if (isText) {
       // Note non-empty text-frames for inline frame compatibility hackery
       pfd->mIsTextFrame = true;
--- a/layout/generic/nsLineLayout.h
+++ b/layout/generic/nsLineLayout.h
@@ -380,24 +380,33 @@ public:
   nscoord GetCurrentICoord();
 
 protected:
   // This state is constant for a given block frame doing line layout
   nsFloatManager* mFloatManager;
   const nsStyleText* mStyleText; // for the block
   const nsHTMLReflowState* mBlockReflowState;
 
-  // The line layout for the base text. It is usually same as |this|.
-  // It becomes different when the current line layout is for ruby
-  // annotations. All line layouts share the same base line layout
-  // when they are associated. The base line layout is responsible
-  // for managing the life cycle of per-frame data and per-span data,
-  // and handling floats.
+  // The line layout for the base text.  It is usually nullptr.
+  // It becomes not null when the current line layout is for ruby
+  // annotations. When there is nested ruby inside annotation, it
+  // forms a linked list from the inner annotation to the outermost
+  // line layout. The outermost line layout, which has this member
+  // being nullptr, is responsible for managing the life cycle of
+  // per-frame data and per-span data, and handling floats.
   nsLineLayout* const mBaseLineLayout;
 
+  nsLineLayout* GetOutermostLineLayout() {
+    nsLineLayout* lineLayout = this;
+    while (lineLayout->mBaseLineLayout) {
+      lineLayout = lineLayout->mBaseLineLayout;
+    }
+    return lineLayout;
+  }
+
   nsIFrame* mLastOptionalBreakFrame;
   nsIFrame* mForceBreakFrame;
   
   // XXX remove this when landing bug 154892 (splitting absolute positioned frames)
   friend class nsInlineFrame;
 
   nsBlockReflowState* mBlockRS;/* XXX hack! */
 
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-ruby/nested-ruby-1.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <title>Bug 1120313 - Nested ruby inside ruby annotation</title>
+  <link rel="stylesheet" href="common.css">
+</head>
+<body>
+  <ruby>
+    <rb>base1</rb>
+    <rt>
+      <ruby>
+        <rb>base2</rb>
+        <rt>text</rt>
+      </ruby>
+    </rt>
+  </ruby>
+</body>
+</html>
--- a/layout/reftests/css-ruby/reftest.list
+++ b/layout/reftests/css-ruby/reftest.list
@@ -22,16 +22,17 @@ fuzzy-if(winWidget,35,1) == dynamic-remo
 == inlinize-blocks-5.html inlinize-blocks-5-ref.html
 == intra-level-whitespace-1.html intra-level-whitespace-1-ref.html
 == intra-level-whitespace-2.html intra-level-whitespace-2-ref.html
 == intra-level-whitespace-3.html intra-level-whitespace-3-ref.html
 == justification-1.html justification-1-ref.html
 == justification-2.html justification-2-ref.html
 == line-height-1.html line-height-1-ref.html
 == line-height-2.html line-height-2-ref.html
+load nested-ruby-1.html
 == line-height-3.html line-height-3-ref.html
 == ruby-span-1.html ruby-span-1-ref.html
 == ruby-whitespace-1.html ruby-whitespace-1-ref.html
 == ruby-whitespace-2.html ruby-whitespace-2-ref.html
 == ruby-position-horizontal.html ruby-position-horizontal-ref.html
 pref(layout.css.vertical-text.enabled,true) fails == ruby-position-vertical-lr.html ruby-position-vertical-lr-ref.html # bug 1112474
 pref(layout.css.vertical-text.enabled,true) fails == ruby-position-vertical-rl.html ruby-position-vertical-rl-ref.html # bug 1112474
 != ruby-reflow-1-opaqueruby.html ruby-reflow-1-noruby.html