Bug 57341 – Split more logic out from createBidiRunsForLine for readability

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

Split more logic out from createBidiRunsForLine for readability

[Eric Seidel (no email)]

Created attachment 87327 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 87327 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=87327&action=review

> Source/WebCore/platform/text/BidiResolver.h:572
> +    for (Run* r = firstRun(); r; r = r->next()) {

I'd rename "r" to "run".

> Source/WebCore/platform/text/BidiResolver.h:590
> +        Run* currRun = firstRun();

I'd rename currRun to currentRun.

> Source/WebCore/platform/text/BidiResolver.h:602
> +            while (i < count && currRun && currRun->m_level < levelHigh) {
> +                i++;
> +                currRun = currRun->next();
> +            }
> +            unsigned start = i;
> +            while (i <= count && currRun && currRun->m_level >= levelHigh) {
> +                i++;
> +                currRun = currRun->next();
> +            }
> +            unsigned end = i - 1;
> +            reverseRuns(start, end);

I'd rewrite these as:

unsigned start;
unsigned end;
for (start = i; start < count && currRun && currRun->m_level < levelHigh; start++)
    currRun = currRun->next();

for (end = start; end < count && currRun && currRun->m_level < levelHigh; end++)
    currRun = currRun->next();

reverseRuns(start, end);

[Eric Seidel (no email)]

Comment on attachment 87327 [details]
Patch

Great suggestions.  I think you meant r+ though. :)

[Eric Seidel (no email)]

(In reply to comment #2)
> I'd rewrite these as:
> 
> unsigned start;
> unsigned end;
> for (start = i; start < count && currRun && currRun->m_level < levelHigh; start++)
>     currRun = currRun->next();
> 
> for (end = start; end < count && currRun && currRun->m_level < levelHigh; end++)
>     currRun = currRun->next();
> 
> reverseRuns(start, end);

I dont' think that quiet works.  We need to keep i in sync with "currRun".  I rewrote it a little.

[Eric Seidel (no email)]

Created attachment 87349 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 87349 [details]
Patch for landing

Rejecting attachment 87349 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'apply-..." exit_code: 2

Last 500 characters of output:
Hunk #3 FAILED at 1584.
Hunk #4 FAILED at 1683.
Hunk #5 FAILED at 1754.
Hunk #6 FAILED at 1850.
Hunk #7 FAILED at 1889.
Hunk #8 FAILED at 1899.
Hunk #9 FAILED at 1908.
Hunk #10 FAILED at 1945.
Hunk #11 FAILED at 2008.
Hunk #12 FAILED at 2038.
Hunk #13 FAILED at 2055.
Hunk #14 FAILED at 2064.
14 out of 14 hunks FAILED -- saving rejects to file Source/WebCore/rendering/RenderBlockLineLayout.cpp.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/8273969

[Eric Seidel (no email)]

Created attachment 87389 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 87389 [details]
Patch for landing

Rejecting attachment 87389 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'apply-..." exit_code: 2

Last 500 characters of output:
bug 57341.
Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Parsed 2 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/platform/text/BidiResolver.h
Hunk #3 FAILED at 981.
1 out of 3 hunks FAILED -- saving rejects to file Source/WebCore/platform/text/BidiResolver.h.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/8280512

[Eric Seidel (no email)]

Created attachment 87460 [details]
Patch for landing

[Eric Seidel (no email)]

Created attachment 87461 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 87461 [details]
Patch for landing

Clearing flags on attachment: 87461

Committed r82389: <http://trac.webkit.org/changeset/82389>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/82389 might have broken SnowLeopard Intel Release (WebKit2 Tests)