Bug 57764 - Split run storage out from BidiResolver into a new BidiRunList class
Summary: Split run storage out from BidiResolver into a new BidiRunList class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks: 50912 57727
  Show dependency tree / graph
 
Reported: 2011-04-04 10:29 PDT by Eric Seidel (no email)
Modified: 2011-04-07 19:12 PDT (History)
6 users (show)

See Also:

Attachments
Patch (35.00 KB, patch)
2011-04-04 10:52 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
Patch (35.32 KB, patch)
2011-04-04 10:58 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
Patch (40.56 KB, patch)
2011-04-04 12:06 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
Patch for landing (39.44 KB, patch)
2011-04-07 11:05 PDT, Eric Seidel (no email) 		commit-queue: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2011-04-04 10:29:46 PDT 
Split run storage out from BidiResolver into a new BidiRunList class
Comment 1 Eric Seidel (no email) 2011-04-04 10:52:27 PDT 
Created attachment 88079 [details]
Patch
Comment 2 Eric Seidel (no email) 2011-04-04 10:58:23 PDT 
Created attachment 88081 [details]
Patch
Comment 3 Eric Seidel (no email) 2011-04-04 11:06:35 PDT 
This patch was based on top of the patch for bug 57727, but is not technically dependent on it.
Comment 4 Eric Seidel (no email) 2011-04-04 11:15:03 PDT 
In my first draft of this patch, I went all the way and tried to fix the tangled relationship between BidiResolver and InlineIterator.  But I backed off in hopes of making this patch easier to review.

The portion of this patch dealing with the creation of BidiRunList is *entirely* just moving code.  I highlighted the one line change in removing the m_emptyRun = true from deleteRuns() in the ChangeLog.

The other portion of this patch is fixing up the callers to deal with these methods being removed from BidiResolver, and is again no functional change.  I did clean up the argument passing a little now that the storage isn't strictly part of BidiResolver.

Hopefully you will all find this a no-brainer to review.
Comment 5 Ryosuke Niwa 2011-04-04 11:53:13 PDT 
Comment on attachment 88081 [details]
Patch

It seems like you've forgot to include BidiRunList.h here.
Comment 6 Eric Seidel (no email) 2011-04-04 12:06:59 PDT 
Created attachment 88092 [details]
Patch
Comment 7 Ryosuke Niwa 2011-04-05 05:00:15 PDT 
Comment on attachment 88092 [details]
Patch

looks sane to me.
Comment 8 Eric Seidel (no email) 2011-04-07 11:05:46 PDT 
Created attachment 88664 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2011-04-07 18:26:57 PDT 
Comment on attachment 88664 [details]
Patch for landing

Rejecting attachment 88664 [details] from commit-queue.

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

Last 500 characters of output:
e/WebCore/WebCore.xcodeproj/project.pbxproj
patching file Source/WebCore/platform/graphics/GraphicsContext.cpp
patching file Source/WebCore/platform/text/BidiResolver.h
patching file Source/WebCore/platform/text/BidiRunList.h
patching file Source/WebCore/rendering/InlineIterator.h
patching file Source/WebCore/rendering/RenderBlock.h
patching file Source/WebCore/rendering/RenderBlockLineLayout.cpp

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/8344794
Comment 10 Eric Seidel (no email) 2011-04-07 19:12:55 PDT 
Committed r83240: <http://trac.webkit.org/changeset/83240>