Attachment #256554 for bug #109624

View Details Raw Unified Return to Bug 109624

| Differences between



2015-07-09  Myles C. Maxfield  <mmaxfield@apple.com>

        Bidi-Isolate inlines break layout with collapsed whitespace
        https://bugs.webkit.org/show_bug.cgi?id=109624

        Reviewed by NOBODY (OOPS!).

        This patch changes the logic in constructBidiRunsForSegment() when it encounters an
        isolate. It already has logic to create a BidiResolver for the isolated text;
        however, that logic doesn't handle setting up the MidpointState at all.
        Specifically, we can set the MidpointState's cursor to point to the first midpoint
        whose ancestor is the isolated root. If no such element exists, simply pretend that
        we have no midpoints at all (which is correct).

        Here is some explanatory text regarding how we collapse spaces:

        Collapsing whitespace happens in a series of phases. The first phase occurs when
        we perform line breaking. Here, we keep track of sequences of whitespace which
        should be collapsed, in the form of a vector of pairs of InlineIterators. We put
        this knowledge into a MidpointState object.

        Then, once we have a line, we run the bidi algorithm on the line (including the
        whitespace). As output, the bidi algorithm calls the BidiResolver::appendRun()
        callback with two InlineIterators each time it wants to create a run. Because
        each renderer that we create has to be owned by exactly one DOM node,
        BidiResolver::appendRun() iterates between its two InlineIterator arguments,
        calling RenderBlockFlow::appendRunsForObject() on each interstitial DOM node.

        This is the function where whitespace collapsing happens. The MidpointState object
        keeps a cursor into its remembered whitespace sequences. Here, we simply make a
        bidi run for each region in between adjacent whitespace pairs in the MidpointState
        object. These bidi runs eventually get turned into leaf InlineBoxes.

        The problem is that the BidiResolver::appendRun() callbacks don't occur in
        string-order, but the Midpoint InlineIterator pairs are in string-order. In
        particular, within a particular isolate, appendRun() gets called in string
        order, but callbacks that occur for inner isolates are deferred. This means that
        RenderBlockFlow::appendRunsForObject() gets confused when it looks for relevant
        whitespace to skip.

        No new tests (OOPS!).

        * platform/text/BidiResolver.h:
        (WebCore::MidpointState::numMidpoints): Returning a const unsigned& is silly.
        (WebCore::MidpointState::currentMidpoint): Ditto.
        (WebCore::MidpointState::setCurrentMidpoint): The isolated MidpointState object
        needs to be able to set its current midpoint to point to the first one inside
        the isolate.
        (WebCore::MidpointState::decrementNumMidpoints): Renamed from "decrease"
        (WebCore::MidpointState::betweenMidpoints): This function is true iff
        currentMidpoint() % 2. Instead of keeping a member variable, we can just compute
        that.
        (WebCore::MidpointState::reset): Deleted.
        (WebCore::MidpointState::decreaseNumMidpoints): Deleted.
        (WebCore::MidpointState::setBetweenMidpoints): Deleted.
        * rendering/InlineIterator.h:
        (WebCore::IsolateTracker::addFakeRunIfNecessary): Call
        RenderBlockFlow::appendRunsForObject() to keep our MidpointState object in sync
        when we pop out of the isolated object. However, we pass in a null run list,
        because we don't want to append just yet (that happens when we process the
        isolate).
        (WebCore::InlineBidiResolver::appendRun): Update for new signature of
        appendRunsForObject().
        * rendering/RenderBlock.h:
        (WebCore::RenderBlock::shouldSkipCreatingRunsForObject): Take a reference instead
        of a pointer.
        * rendering/RenderBlockFlow.h:
        * rendering/RenderBlockLineLayout.cpp:
        (WebCore::createRun): Ditto.
        (WebCore::RenderBlockFlow::appendRunsForObject): Allow someone passing us a null
        BidiRunList. In this case, we will keep the resolver's midpointState() up to date,
        but won't actually emit any runs.
        (WebCore::notifyResolverToResumeInIsolate): Renamed from setUp.
        (WebCore::isolatedResolversMidpointState): Calculate the midpoint state for the
        isolated resolver.
        (WebCore::setUpResolverToResumeInIsolate): Call isolatedResolversMidpointState().
        (WebCore::constructBidiRunsForSegment): Pass in the topResolver, which is
        necessary for isolatedResolversMidpointState().
        * rendering/line/BreakingContext.h:
        (WebCore::checkMidpoints):

2015-07-08  Matthew Daiter  <mdaiter@apple.com>

        Make sure MediaStream engine loaded for interpreting MediaStreams



    {
        m_numMidpoints = 0;
        m_currentMidpoint = 0;
        m_betweenMidpoints = false;
    }
    
    void startIgnoringSpaces(const Iterator& midpoint)

    }

    Vector<Iterator>& midpoints() { return m_midpoints; }
    unsigned numMidpoints() const { return m_numMidpoints; }
    unsigned currentMidpoint() const { return m_currentMidpoint; }
    void setCurrentMidpoint(unsigned currentMidpoint) { m_currentMidpoint = currentMidpoint; }
    void incrementCurrentMidpoint() { ++m_currentMidpoint; }
    void decrementNumMidpoints() { --m_numMidpoints; }
    bool betweenMidpoints() const { return m_currentMidpoint % 2; }
    void setBetweenMidpoints(bool betweenMidpoint) { m_betweenMidpoints = betweenMidpoint; }
private:
    // The goal is to reuse the line state across multiple
    // lines so we just keep an array around for midpoints and never clear it across multiple

    Vector<Iterator> m_midpoints;
    unsigned m_numMidpoints;
    unsigned m_currentMidpoint;
    bool m_betweenMidpoints;

    void addMidpoint(const Iterator& midpoint)
    {



    void embed(UCharDirection, BidiEmbeddingSource) { }
    void commitExplicitEmbedding() { }

    void addFakeRunIfNecessary(RenderObject& obj, unsigned pos, unsigned end, InlineBidiResolver& resolver)
    {
        // We only need to add a fake run for a given isolated span once during each call to createBidiRunsForLine.
        // We'll be called for every span inside the isolated span so we just ignore subsequent calls.
        // We also avoid creating a fake run until we hit a child that warrants one, e.g. we skip floats.
        if (RenderBlock::shouldSkipCreatingRunsForObject(obj))
            return;
        if (!m_haveAddedFakeRunForRootIsolate) {
            // obj and pos together denote a single position in the inline, from which the parsing of the isolate will start.
            // We don't need to mark the end of the run because this is implicit: it is either endOfLine or the end of the
            // isolate, when we call createBidiRunsForLine it will stop at whichever comes first.
            addPlaceholderRunForIsolatedInline(resolver, obj, pos);
        // FIXME: Inline isolates don't work properly with collapsing whitespace, see webkit.org/b/109624
        // For now, if we enter an isolate between midpoints, we increment our current midpoint or else
        // we'll leave the isolate and ignore the content that follows.
        MidpointState<InlineIterator>& midpointState = resolver.midpointState();
        if (midpointState.betweenMidpoints() && midpointState.midpoints()[midpointState.currentMidpoint()].renderer() == &obj) {
            midpointState.setBetweenMidpoints(false);
            midpointState.incrementCurrentMidpoint();
        }
        m_haveAddedFakeRunForRootIsolate = true;
        RenderBlockFlow::appendRunsForObject(obj, pos, end, resolver, nullptr);
    }

private:

        RenderObject* obj = m_sor.renderer();
        while (obj && obj != m_eor.renderer() && obj != endOfLine.renderer()) {
            if (isolateTracker.inIsolate())
                isolateTracker.addFakeRunIfNecessary(*obj, start, obj->length(), *this);
            else
                RenderBlockFlow::appendRunsForObject(*obj, start, obj->length(), *this, &m_runs);
            // FIXME: start/obj should be an InlineIterator instead of two separate variables.
            start = 0;
            obj = bidiNextSkippingEmptyInlines(*m_sor.root(), obj, &isolateTracker);

            // It's OK to add runs for zero-length RenderObjects, just don't make the run larger than it should be
            int end = obj->length() ? pos + 1 : 0;
            if (isolateTracker.inIsolate())
                isolateTracker.addFakeRunIfNecessary(*obj, start, obj->length(), *this);
            else
                RenderBlockFlow::appendRunsForObject(*obj, start, end, *this, &m_runs);
        }

        m_eor.increment();




    virtual RenderBox* createAnonymousBoxWithSameTypeAs(const RenderObject* parent) const override;

    static bool shouldSkipCreatingRunsForObject(RenderObject& obj)
    {
        return obj.isFloating() || (obj.isOutOfFlowPositioned() && !obj.style().isOriginalDisplayInlineType() && !obj.container()->isRenderInline());
    }

    static TextRun constructTextRun(RenderObject* context, const FontCascade&, StringView, const RenderStyle&,

Lines 537-543 private: a/Source/WebCore/rendering/RenderBlockFlow.h_sec1
- a/Source/WebCore/rendering/RenderBlockFlow.h -1 / +1 lines
537	// line layout code is separated from RenderBlock and RenderBlockFlow.	537	// line layout code is separated from RenderBlock and RenderBlockFlow.
538	// START METHODS DEFINED IN RenderBlockLineLayout	538	// START METHODS DEFINED IN RenderBlockLineLayout
539	public:	539	public:
540	static void appendRunsForObject(BidiRunList<BidiRun>&, int start, int end, RenderObject*, InlineBidiResolver&);	540	static void appendRunsForObject(RenderObject&, int start, int end, InlineBidiResolver&, BidiRunList<BidiRun>*);
541	RootInlineBox* createAndAppendRootInlineBox();	541	RootInlineBox* createAndAppendRootInlineBox();
542		542
543	LayoutUnit startAlignedOffsetForLine(LayoutUnit position, bool shouldIndentText);	543	LayoutUnit startAlignedOffsetForLine(LayoutUnit position, bool shouldIndentText);



    }
}

inline BidiRun* createRun(int start, int end, RenderObject& obj, InlineBidiResolver& resolver)
{
    return new BidiRun(start, end, obj, resolver.context(), resolver.dir());
    return new BidiRun(start, end, *obj, resolver.context(), resolver.dir());
}

void RenderBlockFlow::appendRunsForObject(RenderObject& obj, int start, int end, InlineBidiResolver& resolver, BidiRunList<BidiRun>* runs)
{
    if (start > end || shouldSkipCreatingRunsForObject(obj))
        return;

    if (haveNextMidpoint)
        nextMidpoint = lineMidpointState.midpoints()[lineMidpointState.currentMidpoint()];
    if (lineMidpointState.betweenMidpoints()) {
        if (!haveNextMidpoint || (&obj != nextMidpoint.renderer()))
            return;
        // This is a new start point. Stop ignoring objects and
        // adjust our start.
        lineMidpointState.setBetweenMidpoints(false);
        start = nextMidpoint.offset();
        lineMidpointState.incrementCurrentMidpoint();
        if (start < end) {
            appendRunsForObject(obj, start, end, resolver, runs);
            return;
        }
    } else {
        if (!haveNextMidpoint || (&obj != nextMidpoint.renderer())) {
            if (runs)
                runs->addRun(createRun(start, end, obj, resolver));
            return;
        }

        // An end midpoint has been encountered within our object. We need to append a run with our endpoint.
        if (static_cast<int>(nextMidpoint.offset() + 1) <= end) {
            lineMidpointState.setBetweenMidpoints(true);
            lineMidpointState.incrementCurrentMidpoint();
            // The end of the line is before the object we're inspecting. Skip everything and return
            if (nextMidpoint.refersToEndOfPreviousNode())
                return;
            if (static_cast<int>(nextMidpoint.offset() + 1) > start && runs)
                runs->addRun(createRun(start, nextMidpoint.offset() + 1, obj, resolver));
            appendRunsForObject(obj, nextMidpoint.offset() + 1, end, resolver, runs);
        } else if (runs)
            runs->addRun(createRun(start, end, obj, resolver));
    }
}


    lastRootBox()->appendFloat(floatingObject->renderer());
}

static inline void notifyResolverToResumeInIsolate(InlineBidiResolver& resolver, RenderObject* root, RenderObject* startObject)
{
    if (root != startObject) {
        RenderObject* parent = startObject->parent();
        notifyResolverToResumeInIsolate(resolver, root, parent);
        notifyObserverEnteredObject(&resolver, startObject);
    }
}

static inline Optional<LineMidpointState> isolatedResolversMidpointState(InlineBidiResolver& topResolver, RenderObject* root)
{
    LineMidpointState midpoints = topResolver.midpointState();
    for (unsigned i = 0; i < midpoints.numMidpoints(); ++i) {
        InlineIterator iter = midpoints.midpoints()[i];
        for (RenderObject* renderer = iter.renderer(); renderer && renderer != topResolver.position().root(); renderer = renderer->parent()) {
            if (renderer == root) {
                midpoints.setCurrentMidpoint(i);
                return midpoints;
            }
        }
    }
    return Optional<LineMidpointState>(Nullopt);
}

static inline void setUpResolverToResumeInIsolate(InlineBidiResolver& resolver, InlineBidiResolver& topResolver, RenderObject* root, RenderObject* startObject)
{
    // Set up m_midpointState
    if (Optional<LineMidpointState> midpoints = isolatedResolversMidpointState(topResolver, root))
        resolver.midpointState() = WTF::move(midpoints.value());

    // Set up m_nestedIsolateCount
    notifyResolverToResumeInIsolate(resolver, root, startObject);
}

// FIXME: BidiResolver should have this logic.
static inline void constructBidiRunsForSegment(InlineBidiResolver& topResolver, BidiRunList<BidiRun>& bidiRuns, const InlineIterator& endOfRuns, VisualDirectionOverride override, bool previousLineBrokeCleanly)
{

        }
        isolatedResolver.setStatus(BidiStatus(direction, isOverride(unicodeBidi)));

        setUpResolverToResumeInIsolate(isolatedResolver, topResolver, isolatedInline, &startObject);

        // The starting position is the beginning of the first run within the isolate that was identified
        // during the earlier call to createBidiRunsForLine. This can be but is not necessarily the

Lines 1147-1153 inline TrailingObjects::CollapseFirstSpaceOrNot checkMidpoints(LineMidpointState a/Source/WebCore/rendering/line/BreakingContext.h_sec1
- a/Source/WebCore/rendering/line/BreakingContext.h -1 / +1 lines
1147	currpoint.increment();	1147	currpoint.increment();
1148	if (currpoint == lBreak) {	1148	if (currpoint == lBreak) {
1149	// We hit the line break before the start point. Shave off the start point.	1149	// We hit the line break before the start point. Shave off the start point.
1150	lineMidpointState.decreaseNumMidpoints();	1150	lineMidpointState.decrementNumMidpoints();
1151	if (endpoint.renderer()->style().collapseWhiteSpace() && endpoint.renderer()->isText()) {	1151	if (endpoint.renderer()->style().collapseWhiteSpace() && endpoint.renderer()->isText()) {
1152	endpoint.fastDecrement();	1152	endpoint.fastDecrement();
1153	return TrailingObjects::DoNotCollapseFirstSpace;	1153	return TrailingObjects::DoNotCollapseFirstSpace;

Return to Bug 109624

Lines 44-50 public: a/Source/WebCore/platform/text/BidiResolver.h_sec1
- a/Source/WebCore/platform/text/BidiResolver.h -7 / +5 lines
44	{	44	{
45	m_numMidpoints = 0;	45	m_numMidpoints = 0;
46	m_currentMidpoint = 0;	46	m_currentMidpoint = 0;
47	m_betweenMidpoints = false;
48	}	47	}
49		48
50	void startIgnoringSpaces(const Iterator& midpoint)	49	void startIgnoringSpaces(const Iterator& midpoint)
Lines 69-80 public: a/Source/WebCore/platform/text/BidiResolver.h_sec2
69	}	68	}
70		69
71	Vector<Iterator>& midpoints() { return m_midpoints; }	70	Vector<Iterator>& midpoints() { return m_midpoints; }
72	const unsigned& numMidpoints() const { return m_numMidpoints; }	71	unsigned numMidpoints() const { return m_numMidpoints; }
73	const unsigned& currentMidpoint() const { return m_currentMidpoint; }	72	unsigned currentMidpoint() const { return m_currentMidpoint; }
		73	void setCurrentMidpoint(unsigned currentMidpoint) { m_currentMidpoint = currentMidpoint; }
74	void incrementCurrentMidpoint() { ++m_currentMidpoint; }	74	void incrementCurrentMidpoint() { ++m_currentMidpoint; }
75	void decreaseNumMidpoints() { --m_numMidpoints; }	75	void decrementNumMidpoints() { --m_numMidpoints; }
76	const bool& betweenMidpoints() const { return m_betweenMidpoints; }	76	bool betweenMidpoints() const { return m_currentMidpoint % 2; }
77	void setBetweenMidpoints(bool betweenMidpoint) { m_betweenMidpoints = betweenMidpoint; }
78	private:	77	private:
79	// The goal is to reuse the line state across multiple	78	// The goal is to reuse the line state across multiple
80	// lines so we just keep an array around for midpoints and never clear it across multiple	79	// lines so we just keep an array around for midpoints and never clear it across multiple
Lines 82-88 private: a/Source/WebCore/platform/text/BidiResolver.h_sec3
82	Vector<Iterator> m_midpoints;	81	Vector<Iterator> m_midpoints;
83	unsigned m_numMidpoints;	82	unsigned m_numMidpoints;
84	unsigned m_currentMidpoint;	83	unsigned m_currentMidpoint;
85	bool m_betweenMidpoints;
86		84
87	void addMidpoint(const Iterator& midpoint)	85	void addMidpoint(const Iterator& midpoint)
88	{	86	{

Lines 506-531 public: a/Source/WebCore/rendering/InlineIterator.h_sec1
- a/Source/WebCore/rendering/InlineIterator.h -18 / +13 lines
506	void embed(UCharDirection, BidiEmbeddingSource) { }	506	void embed(UCharDirection, BidiEmbeddingSource) { }
507	void commitExplicitEmbedding() { }	507	void commitExplicitEmbedding() { }
508		508
509	void addFakeRunIfNecessary(RenderObject& obj, unsigned pos, InlineBidiResolver& resolver)	509	void addFakeRunIfNecessary(RenderObject& obj, unsigned pos, unsigned end, InlineBidiResolver& resolver)
510	{	510	{
511	// We only need to add a fake run for a given isolated span once during each call to createBidiRunsForLine.	511	// We only need to add a fake run for a given isolated span once during each call to createBidiRunsForLine.
512	// We'll be called for every span inside the isolated span so we just ignore subsequent calls.	512	// We'll be called for every span inside the isolated span so we just ignore subsequent calls.
513	// We also avoid creating a fake run until we hit a child that warrants one, e.g. we skip floats.	513	// We also avoid creating a fake run until we hit a child that warrants one, e.g. we skip floats.
514	if (m_haveAddedFakeRunForRootIsolate \|\| RenderBlock::shouldSkipCreatingRunsForObject(&obj))	514	if (RenderBlock::shouldSkipCreatingRunsForObject(obj))
515	return;	515	return;
516	m_haveAddedFakeRunForRootIsolate = true;	516	if (!m_haveAddedFakeRunForRootIsolate) {
517	// obj and pos together denote a single position in the inline, from which the parsing of the isolate will start.	517	// obj and pos together denote a single position in the inline, from which the parsing of the isolate will start.
518	// We don't need to mark the end of the run because this is implicit: it is either endOfLine or the end of the	518	// We don't need to mark the end of the run because this is implicit: it is either endOfLine or the end of the
519	// isolate, when we call createBidiRunsForLine it will stop at whichever comes first.	519	// isolate, when we call createBidiRunsForLine it will stop at whichever comes first.
520	addPlaceholderRunForIsolatedInline(resolver, obj, pos);	520	addPlaceholderRunForIsolatedInline(resolver, obj, pos);
521	// FIXME: Inline isolates don't work properly with collapsing whitespace, see webkit.org/b/109624
522	// For now, if we enter an isolate between midpoints, we increment our current midpoint or else
523	// we'll leave the isolate and ignore the content that follows.
524	MidpointState<InlineIterator>& midpointState = resolver.midpointState();
525	if (midpointState.betweenMidpoints() && midpointState.midpoints()[midpointState.currentMidpoint()].renderer() == &obj) {
526	midpointState.setBetweenMidpoints(false);
527	midpointState.incrementCurrentMidpoint();
528	}	521	}
		522	m_haveAddedFakeRunForRootIsolate = true;
		523	RenderBlockFlow::appendRunsForObject(obj, pos, end, resolver, nullptr);
529	}	524	}
530		525
531	private:	526	private:
Lines 545-553 inline void InlineBidiResolver::appendRun() a/Source/WebCore/rendering/InlineIterator.h_sec2
545	RenderObject* obj = m_sor.renderer();	540	RenderObject* obj = m_sor.renderer();
546	while (obj && obj != m_eor.renderer() && obj != endOfLine.renderer()) {	541	while (obj && obj != m_eor.renderer() && obj != endOfLine.renderer()) {
547	if (isolateTracker.inIsolate())	542	if (isolateTracker.inIsolate())
548	isolateTracker.addFakeRunIfNecessary(obj, start, this);	543	isolateTracker.addFakeRunIfNecessary(obj, start, obj->length(), this);
549	else	544	else
550	RenderBlockFlow::appendRunsForObject(m_runs, start, obj->length(), obj, *this);	545	RenderBlockFlow::appendRunsForObject(obj, start, obj->length(), this, &m_runs);
551	// FIXME: start/obj should be an InlineIterator instead of two separate variables.	546	// FIXME: start/obj should be an InlineIterator instead of two separate variables.
552	start = 0;	547	start = 0;
553	obj = bidiNextSkippingEmptyInlines(*m_sor.root(), obj, &isolateTracker);	548	obj = bidiNextSkippingEmptyInlines(*m_sor.root(), obj, &isolateTracker);
Lines 561-569 inline void InlineBidiResolver::appendRun() a/Source/WebCore/rendering/InlineIterator.h_sec3
561	// It's OK to add runs for zero-length RenderObjects, just don't make the run larger than it should be	556	// It's OK to add runs for zero-length RenderObjects, just don't make the run larger than it should be
562	int end = obj->length() ? pos + 1 : 0;	557	int end = obj->length() ? pos + 1 : 0;
563	if (isolateTracker.inIsolate())	558	if (isolateTracker.inIsolate())
564	isolateTracker.addFakeRunIfNecessary(obj, start, this);	559	isolateTracker.addFakeRunIfNecessary(obj, start, obj->length(), this);
565	else	560	else
566	RenderBlockFlow::appendRunsForObject(m_runs, start, end, obj, *this);	561	RenderBlockFlow::appendRunsForObject(obj, start, end, this, &m_runs);
567	}	562	}
568		563
569	m_eor.increment();	564	m_eor.increment();

Lines 199-207 public: a/Source/WebCore/rendering/RenderBlock.h_sec1
- a/Source/WebCore/rendering/RenderBlock.h -2 / +2 lines
199		199
200	virtual RenderBox* createAnonymousBoxWithSameTypeAs(const RenderObject* parent) const override;	200	virtual RenderBox* createAnonymousBoxWithSameTypeAs(const RenderObject* parent) const override;
201		201
202	static bool shouldSkipCreatingRunsForObject(RenderObject* obj)	202	static bool shouldSkipCreatingRunsForObject(RenderObject& obj)
203	{	203	{
204	return obj->isFloating() \|\| (obj->isOutOfFlowPositioned() && !obj->style().isOriginalDisplayInlineType() && !obj->container()->isRenderInline());	204	return obj.isFloating() \|\| (obj.isOutOfFlowPositioned() && !obj.style().isOriginalDisplayInlineType() && !obj.container()->isRenderInline());
205	}	205	}
206		206
207	static TextRun constructTextRun(RenderObject* context, const FontCascade&, StringView, const RenderStyle&,	207	static TextRun constructTextRun(RenderObject* context, const FontCascade&, StringView, const RenderStyle&,

Lines 71-83 static void determineDirectionality(TextDirection& dir, InlineIterator iter) a/Source/WebCore/rendering/RenderBlockLineLayout.cpp_sec1
- a/Source/WebCore/rendering/RenderBlockLineLayout.cpp -19 / +44 lines
71	}	71	}
72	}	72	}
73		73
74	inline BidiRun* createRun(int start, int end, RenderObject* obj, InlineBidiResolver& resolver)	74	inline BidiRun* createRun(int start, int end, RenderObject& obj, InlineBidiResolver& resolver)
75	{	75	{
76	ASSERT(obj);	76	return new BidiRun(start, end, obj, resolver.context(), resolver.dir());
77	return new BidiRun(start, end, *obj, resolver.context(), resolver.dir());
78	}	77	}
79		78
80	void RenderBlockFlow::appendRunsForObject(BidiRunList<BidiRun>& runs, int start, int end, RenderObject* obj, InlineBidiResolver& resolver)	79	void RenderBlockFlow::appendRunsForObject(RenderObject& obj, int start, int end, InlineBidiResolver& resolver, BidiRunList<BidiRun>* runs)
81	{	80	{
82	if (start > end \|\| shouldSkipCreatingRunsForObject(obj))	81	if (start > end \|\| shouldSkipCreatingRunsForObject(obj))
83	return;	82	return;
Lines 88-120 void RenderBlockFlow::appendRunsForObject(BidiRunList<BidiRun>& runs, int start, a/Source/WebCore/rendering/RenderBlockLineLayout.cpp_sec2
88	if (haveNextMidpoint)	87	if (haveNextMidpoint)
89	nextMidpoint = lineMidpointState.midpoints()[lineMidpointState.currentMidpoint()];	88	nextMidpoint = lineMidpointState.midpoints()[lineMidpointState.currentMidpoint()];
90	if (lineMidpointState.betweenMidpoints()) {	89	if (lineMidpointState.betweenMidpoints()) {
91	if (!(haveNextMidpoint && nextMidpoint.renderer() == obj))	90	if (!haveNextMidpoint \|\| (&obj != nextMidpoint.renderer()))
92	return;	91	return;
93	// This is a new start point. Stop ignoring objects and	92	// This is a new start point. Stop ignoring objects and
94	// adjust our start.	93	// adjust our start.
95	lineMidpointState.setBetweenMidpoints(false);
96	start = nextMidpoint.offset();	94	start = nextMidpoint.offset();
97	lineMidpointState.incrementCurrentMidpoint();	95	lineMidpointState.incrementCurrentMidpoint();
98	if (start < end)	96	if (start < end) {
99	return appendRunsForObject(runs, start, end, obj, resolver);	97	appendRunsForObject(obj, start, end, resolver, runs);
		98	return;
		99	}
100	} else {	100	} else {
101	if (!haveNextMidpoint \|\| (obj != nextMidpoint.renderer())) {	101	if (!haveNextMidpoint \|\| (&obj != nextMidpoint.renderer())) {
102	runs.addRun(createRun(start, end, obj, resolver));	102	if (runs)
		103	runs->addRun(createRun(start, end, obj, resolver));
103	return;	104	return;
104	}	105	}
105		106
106	// An end midpoint has been encountered within our object. We need to append a run with our endpoint.	107	// An end midpoint has been encountered within our object. We need to append a run with our endpoint.
107	if (static_cast<int>(nextMidpoint.offset() + 1) <= end) {	108	if (static_cast<int>(nextMidpoint.offset() + 1) <= end) {
108	lineMidpointState.setBetweenMidpoints(true);
109	lineMidpointState.incrementCurrentMidpoint();	109	lineMidpointState.incrementCurrentMidpoint();
110	// The end of the line is before the object we're inspecting. Skip everything and return	110	// The end of the line is before the object we're inspecting. Skip everything and return
111	if (nextMidpoint.refersToEndOfPreviousNode())	111	if (nextMidpoint.refersToEndOfPreviousNode())
112	return;	112	return;
113	if (static_cast<int>(nextMidpoint.offset() + 1) > start)	113	if (static_cast<int>(nextMidpoint.offset() + 1) > start && runs)
114	runs.addRun(createRun(start, nextMidpoint.offset() + 1, obj, resolver));	114	runs->addRun(createRun(start, nextMidpoint.offset() + 1, obj, resolver));
115	appendRunsForObject(runs, nextMidpoint.offset() + 1, end, obj, resolver);	115	appendRunsForObject(obj, nextMidpoint.offset() + 1, end, resolver, runs);
116	} else	116	} else if (runs)
117	runs.addRun(createRun(start, end, obj, resolver));	117	runs->addRun(createRun(start, end, obj, resolver));
118	}	118	}
119	}	119	}
120		120
Lines 1011-1025 void RenderBlockFlow::appendFloatingObjectToLastLine(FloatingObject* floatingObj a/Source/WebCore/rendering/RenderBlockLineLayout.cpp_sec3
1011	lastRootBox()->appendFloat(floatingObject->renderer());	1011	lastRootBox()->appendFloat(floatingObject->renderer());
1012	}	1012	}
1013		1013
1014	static inline void setUpResolverToResumeInIsolate(InlineBidiResolver& resolver, RenderObject* root, RenderObject* startObject)	1014	static inline void notifyResolverToResumeInIsolate(InlineBidiResolver& resolver, RenderObject* root, RenderObject* startObject)
1015	{	1015	{
1016	if (root != startObject) {	1016	if (root != startObject) {
1017	RenderObject* parent = startObject->parent();	1017	RenderObject* parent = startObject->parent();
1018	setUpResolverToResumeInIsolate(resolver, root, parent);	1018	notifyResolverToResumeInIsolate(resolver, root, parent);
1019	notifyObserverEnteredObject(&resolver, startObject);	1019	notifyObserverEnteredObject(&resolver, startObject);
1020	}	1020	}
1021	}	1021	}
1022		1022
		1023	static inline Optional<LineMidpointState> isolatedResolversMidpointState(InlineBidiResolver& topResolver, RenderObject* root)
		1024	{
		1025	LineMidpointState midpoints = topResolver.midpointState();
		1026	for (unsigned i = 0; i < midpoints.numMidpoints(); ++i) {
		1027	InlineIterator iter = midpoints.midpoints()[i];
		1028	for (RenderObject* renderer = iter.renderer(); renderer && renderer != topResolver.position().root(); renderer = renderer->parent()) {
		1029	if (renderer == root) {
		1030	midpoints.setCurrentMidpoint(i);
		1031	return midpoints;
		1032	}
		1033	}
		1034	}
		1035	return Optional<LineMidpointState>(Nullopt);
		1036	}
		1037
		1038	static inline void setUpResolverToResumeInIsolate(InlineBidiResolver& resolver, InlineBidiResolver& topResolver, RenderObject* root, RenderObject* startObject)
		1039	{
		1040	// Set up m_midpointState
		1041	if (Optional<LineMidpointState> midpoints = isolatedResolversMidpointState(topResolver, root))
		1042	resolver.midpointState() = WTF::move(midpoints.value());
		1043
		1044	// Set up m_nestedIsolateCount
		1045	notifyResolverToResumeInIsolate(resolver, root, startObject);
		1046	}
		1047
1023	// FIXME: BidiResolver should have this logic.	1048	// FIXME: BidiResolver should have this logic.
1024	static inline void constructBidiRunsForSegment(InlineBidiResolver& topResolver, BidiRunList<BidiRun>& bidiRuns, const InlineIterator& endOfRuns, VisualDirectionOverride override, bool previousLineBrokeCleanly)	1049	static inline void constructBidiRunsForSegment(InlineBidiResolver& topResolver, BidiRunList<BidiRun>& bidiRuns, const InlineIterator& endOfRuns, VisualDirectionOverride override, bool previousLineBrokeCleanly)
1025	{	1050	{
Lines 1056-1062 static inline void constructBidiRunsForSegment(InlineBidiResolver& topResolver, a/Source/WebCore/rendering/RenderBlockLineLayout.cpp_sec4
1056	}	1081	}
1057	isolatedResolver.setStatus(BidiStatus(direction, isOverride(unicodeBidi)));	1082	isolatedResolver.setStatus(BidiStatus(direction, isOverride(unicodeBidi)));
1058		1083
1059	setUpResolverToResumeInIsolate(isolatedResolver, isolatedInline, &startObject);	1084	setUpResolverToResumeInIsolate(isolatedResolver, topResolver, isolatedInline, &startObject);
1060		1085
1061	// The starting position is the beginning of the first run within the isolate that was identified	1086	// The starting position is the beginning of the first run within the isolate that was identified
1062	// during the earlier call to createBidiRunsForLine. This can be but is not necessarily the	1087	// during the earlier call to createBidiRunsForLine. This can be but is not necessarily the

Lines 1-3 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +81 lines
		1	2015-07-09 Myles C. Maxfield <mmaxfield@apple.com>
		2
		3	Bidi-Isolate inlines break layout with collapsed whitespace
		4	https://bugs.webkit.org/show_bug.cgi?id=109624
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	This patch changes the logic in constructBidiRunsForSegment() when it encounters an
		9	isolate. It already has logic to create a BidiResolver for the isolated text;
		10	however, that logic doesn't handle setting up the MidpointState at all.
		11	Specifically, we can set the MidpointState's cursor to point to the first midpoint
		12	whose ancestor is the isolated root. If no such element exists, simply pretend that
		13	we have no midpoints at all (which is correct).
		14
		15	Here is some explanatory text regarding how we collapse spaces:
		16
		17	Collapsing whitespace happens in a series of phases. The first phase occurs when
		18	we perform line breaking. Here, we keep track of sequences of whitespace which
		19	should be collapsed, in the form of a vector of pairs of InlineIterators. We put
		20	this knowledge into a MidpointState object.
		21
		22	Then, once we have a line, we run the bidi algorithm on the line (including the
		23	whitespace). As output, the bidi algorithm calls the BidiResolver::appendRun()
		24	callback with two InlineIterators each time it wants to create a run. Because
		25	each renderer that we create has to be owned by exactly one DOM node,
		26	BidiResolver::appendRun() iterates between its two InlineIterator arguments,
		27	calling RenderBlockFlow::appendRunsForObject() on each interstitial DOM node.
		28
		29	This is the function where whitespace collapsing happens. The MidpointState object
		30	keeps a cursor into its remembered whitespace sequences. Here, we simply make a
		31	bidi run for each region in between adjacent whitespace pairs in the MidpointState
		32	object. These bidi runs eventually get turned into leaf InlineBoxes.
		33
		34	The problem is that the BidiResolver::appendRun() callbacks don't occur in
		35	string-order, but the Midpoint InlineIterator pairs are in string-order. In
		36	particular, within a particular isolate, appendRun() gets called in string
		37	order, but callbacks that occur for inner isolates are deferred. This means that
		38	RenderBlockFlow::appendRunsForObject() gets confused when it looks for relevant
		39	whitespace to skip.
		40
		41	No new tests (OOPS!).
		42
		43	* platform/text/BidiResolver.h:
		44	(WebCore::MidpointState::numMidpoints): Returning a const unsigned& is silly.
		45	(WebCore::MidpointState::currentMidpoint): Ditto.
		46	(WebCore::MidpointState::setCurrentMidpoint): The isolated MidpointState object
		47	needs to be able to set its current midpoint to point to the first one inside
		48	the isolate.
		49	(WebCore::MidpointState::decrementNumMidpoints): Renamed from "decrease"
		50	(WebCore::MidpointState::betweenMidpoints): This function is true iff
		51	currentMidpoint() % 2. Instead of keeping a member variable, we can just compute
		52	that.
		53	(WebCore::MidpointState::reset): Deleted.
		54	(WebCore::MidpointState::decreaseNumMidpoints): Deleted.
		55	(WebCore::MidpointState::setBetweenMidpoints): Deleted.
		56	* rendering/InlineIterator.h:
		57	(WebCore::IsolateTracker::addFakeRunIfNecessary): Call
		58	RenderBlockFlow::appendRunsForObject() to keep our MidpointState object in sync
		59	when we pop out of the isolated object. However, we pass in a null run list,
		60	because we don't want to append just yet (that happens when we process the
		61	isolate).
		62	(WebCore::InlineBidiResolver::appendRun): Update for new signature of
		63	appendRunsForObject().
		64	* rendering/RenderBlock.h:
65	(WebCore::RenderBlock::shouldSkipCreatingRunsForObject): Take a reference instead
66	of a pointer.
67	* rendering/RenderBlockFlow.h:
68	* rendering/RenderBlockLineLayout.cpp:
69	(WebCore::createRun): Ditto.
70	(WebCore::RenderBlockFlow::appendRunsForObject): Allow someone passing us a null
71	BidiRunList. In this case, we will keep the resolver's midpointState() up to date,
72	but won't actually emit any runs.
73	(WebCore::notifyResolverToResumeInIsolate): Renamed from setUp.
74	(WebCore::isolatedResolversMidpointState): Calculate the midpoint state for the
75	isolated resolver.
76	(WebCore::setUpResolverToResumeInIsolate): Call isolatedResolversMidpointState().
77	(WebCore::constructBidiRunsForSegment): Pass in the topResolver, which is
78	necessary for isolatedResolversMidpointState().
79	* rendering/line/BreakingContext.h:
80	(WebCore::checkMidpoints):
81
1	2015-07-08 Matthew Daiter <mdaiter@apple.com>	82	2015-07-08 Matthew Daiter <mdaiter@apple.com>
2		83
3	Make sure MediaStream engine loaded for interpreting MediaStreams	84	Make sure MediaStream engine loaded for interpreting MediaStreams