Fake Bidi run does not need to be always created

Currently Blink creates a fake run for a given isolated span during each call
to createBidiRunsForLine. However in some cases the added fake run is not necessary.

This patch uses the same logic in RenderBlockFlow::appendRunsObject to create
fake runs just when they are really needed.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=160495

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161410

[eseidel, 2013-10-24 18:10:05]

https://codereview.chromium.org/39783004/diff/30001/Source/core/rendering/Inl...
File Source/core/rendering/InlineIterator.h (right):

https://codereview.chromium.org/39783004/diff/30001/Source/core/rendering/Inl...
Source/core/rendering/InlineIterator.h:625: if (isolateTracker.inIsolate()) {
Why not just move this check inside adjustMidpointsAndAppendRunsForObject (or
some wrapper).  It looks like we're just repeating the same if twice?

[igoroliveira, 2013-10-24 19:39:30]

On 2013/10/24 18:10:05, eseidel wrote:
>
https://codereview.chromium.org/39783004/diff/30001/Source/core/rendering/Inl...
> File Source/core/rendering/InlineIterator.h (right):
> 
>
https://codereview.chromium.org/39783004/diff/30001/Source/core/rendering/Inl...
> Source/core/rendering/InlineIterator.h:625: if (isolateTracker.inIsolate()) {
> Why not just move this check inside adjustMidpointsAndAppendRunsForObject (or
> some wrapper).  It looks like we're just repeating the same if twice?

Reimplemented the last patch from scratch :). Now the code looks sane.

[eseidel, 2013-10-24 19:41:13]

lgtm

Thank you. :)

[commit-bot: I haz the power, 2013-10-24 19:41:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/igor.o@sisa.samsung.com/39783004/170001

[leviw_travelin_and_unemployed, 2013-10-24 20:18:00]

That looks much better!! Lgtm.

[commit-bot: I haz the power, 2013-10-24 21:03:50]

Change committed as 160495

[eseidel, 2013-10-26 21:15:03]

We have a bad crasher in Canary:
http://pastebin.com/GruVKNGm

This is the most likely culprit.  Unfortunately I don't have a repro for you,
but karen may be able to provide you one if this turns out to be the cause.

[eseidel, 2013-10-26 21:16:45]

According to Karen, the regression range for that crasher is:
http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog_blink.html?u...

This change seems most likely of those...

[igoroliveira, 2013-11-01 20:36:16]

On 2013/10/26 21:16:45, eseidel wrote:
> According to Karen, the regression range for that crasher is:
>
http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog_blink.html?u...
> 
> This change seems most likely of those...

I will give a try again for this change. Since the regressed sites are not going
through fake run in path.

[eseidel, 2013-11-01 21:58:10]

I agree we should attempt to reland this, since none of our testing (Debug,
ASAN, etc) has been able to repro this crash.

Karen: If you see any sign of this crash in Canary we're prepared to roll it out
again immediately.

[igoroliveira, 2013-11-05 22:19:08]

On 2013/11/01 21:58:10, eseidel wrote:
> I agree we should attempt to reland this, since none of our testing (Debug,
> ASAN, etc) has been able to repro this crash.
> 
> Karen: If you see any sign of this crash in Canary we're prepared to roll it
out
> again immediately.

I will release the kraken soon :)(reland the patch).

[commit-bot: I haz the power, 2013-11-06 00:47:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/igor.o@sisa.samsung.com/39783004/360001

[commit-bot: I haz the power, 2013-11-06 02:13:22]

Change committed as 161410