Bug 31468

Summary: Unify TextBoundaries implementations by only relying on WTF Unicode abstractions
Product: WebKit Reporter: Dominik Röttsches (drott) <d-r>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 15914, 31469    
Attachments:
Description Flags
Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp
none
Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v2)
darin: review+
Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3) eric: review+, eric: commit-queue+

Dominik Röttsches (drott)
Reported 2009-11-13 05:43:44 PST
Currently, there are multiple TextBoundaries implementations, one based on ICU (platform/text/TextBoundariesICU.cpp), one for QT (text/qt/TextBoundaries.cpp) - and for closing bug 15914, there would be another one needed for GTK/GLib. In order to reduce code duplication, I suggest to unify these into one, only using WTF abstractions.
Attachments
Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (11.67 KB, patch)
2009-11-13 08:28 PST, Dominik Röttsches (drott) 		no flags
DetailsFormatted DiffDiff
Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v2) (11.70 KB, patch)
2009-11-13 08:43 PST, Dominik Röttsches (drott) 		darin: review+
DetailsFormatted DiffDiff
Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3) (11.73 KB, patch)
2009-11-13 09:13 PST, Dominik Röttsches (drott) 		eric: review+
eric: commit-queue+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Dominik Röttsches (drott)
Comment 1 2009-11-13 08:28:51 PST
Created attachment 43157 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp TextBoundaries.cpp intends to be equivalent to the TextBoundariesICU.cpp but is relying on internal abstractions. Removing unneeded ICU includes in SimpleFontDataGtk.cpp on the way. I did not try to merge the Qt version of TextBoundaries.cpp - could someone from the Qt WebKit team look at whether that's desired? Who would be best CC'ed on this bug regarding this question?
Dominik Röttsches (drott)
Comment 2 2009-11-13 08:43:34 PST
Created attachment 43160 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v2) Minor copyright fix.
Darin Adler
Comment 3 2009-11-13 08:48:28 PST
Comment on attachment 43160 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v2) We normally use "using namespace" to bring in the WTF Unicode functions. See this idiom in files such as Document.cpp, Font.cpp, and String.cpp. This patch otherwise looks good to me. r=me
Dominik Röttsches (drott)
Comment 4 2009-11-13 09:13:39 PST
Created attachment 43164 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3) Changed according to Darin's suggestion to use a "using namespace" declaration.
Eric Seidel (no email)
Comment 5 2009-11-13 12:30:52 PST
Comment on attachment 43164 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3) Changes to be committed also need an r+. Looks sane to me too.
WebKit Commit Bot
Comment 6 2009-11-13 13:48:04 PST
Comment on attachment 43164 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3) Rejecting patch 43164 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1 Running build-dumprendertree Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 11621 test cases. http/tests/xmlhttprequest/access-control-basic-allow-access-control-origin-header-data-url.html -> crashed Exiting early after 1 failures. 9149 tests run. 466.37s total testing time 9148 test cases (99%) succeeded 1 test case (<1%) crashed 5 test cases (<1%) had stderr output
Eric Seidel (no email)
Comment 7 2009-11-13 13:52:40 PST
Comment on attachment 43164 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3) We have a bad JSC regression atm. bug 31461. Let's try again.
WebKit Commit Bot
Comment 8 2009-11-13 14:11:33 PST
Comment on attachment 43164 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3) Rejecting patch 43164 from commit-queue. Failed to run "WebKitTools/Scripts/build-webkit" exit_code: 1 Last 500 characters of output: ld/WebCore.build/Release/WebCore.build/Objects-normal/i386/JSCompositionEvent.o /Users/eseidel/Projects/CommitQueue/WebKitBuild/Release/DerivedSources/WebCore/JSCompositionEvent.cpp normal i386 c++ com.apple.compilers.gcc.4_2 Distributed-CompileC /Users/eseidel/Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/i386/JSEventCustom.o /Users/eseidel/Projects/CommitQueue/WebCore/bindings/js/JSEventCustom.cpp normal i386 c++ com.apple.compilers.gcc.4_2 (2 failures)
Eric Seidel (no email)
Comment 9 2009-11-13 14:13:52 PST
Comment on attachment 43164 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3) I'm in the process of moving the commit-queue to a new machine, and it's had hiccups. Sorry.
WebKit Commit Bot
Comment 10 2009-11-13 14:45:56 PST
Comment on attachment 43164 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3) Rejecting patch 43164 from commit-queue. Failed to run "WebKitTools/Scripts/build-webkit" exit_code: 1 Last 500 characters of output: ld/WebCore.build/Release/WebCore.build/Objects-normal/i386/JSCompositionEvent.o /Users/eseidel/Projects/CommitQueue/WebKitBuild/Release/DerivedSources/WebCore/JSCompositionEvent.cpp normal i386 c++ com.apple.compilers.gcc.4_2 Distributed-CompileC /Users/eseidel/Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/i386/JSEventCustom.o /Users/eseidel/Projects/CommitQueue/WebCore/bindings/js/JSEventCustom.cpp normal i386 c++ com.apple.compilers.gcc.4_2 (2 failures)
Eric Seidel (no email)
Comment 11 2009-11-13 14:48:11 PST
Comment on attachment 43164 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3) My continued apologies. I've reverted back to the old commit-queue machine until I can figure out the build issue. Hopefully the next run will not hit the JSC crasher. :(
WebKit Commit Bot
Comment 12 2009-11-13 15:47:57 PST
Comment on attachment 43164 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3) Rejecting patch 43164 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1 Running build-dumprendertree Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 11622 test cases. http/tests/xmlhttprequest/workers/shared-worker-close.html -> crashed Exiting early after 1 failures. 9308 tests run. 522.97s total testing time 9307 test cases (99%) succeeded 1 test case (<1%) crashed 5 test cases (<1%) had stderr output
Eric Seidel (no email)
Comment 13 2009-11-13 15:49:38 PST
Comment on attachment 43164 [details] Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3) Bug 31461 again. :(
Eric Seidel (no email)
Comment 14 2009-11-13 16:10:12 PST
Landed as r50977.
Eric Seidel (no email)
Comment 15 2009-11-13 16:39:49 PST
Committed r50980: <http://trac.webkit.org/changeset/50980>
Eric Seidel (no email)
Comment 16 2009-11-13 16:40:19 PST
Windows and Chromium builds broke after the first commit. The second commit was to fix them.
Dominik Röttsches (drott)
Comment 17 2009-11-14 03:57:12 PST
Thanks for additional review and build fixes, Eric!
Note You need to log in before you can comment on or make changes to this bug.