Bug 31468 – Unify TextBoundaries implementations by only relying on WTF Unicode abstractions

• Component: Platform
• Status: RESOLVED FIXED

[Dominik Röttsches (drott)]

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.

[Dominik Röttsches (drott)]

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)]

Created attachment 43160 [details]
Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v2)

Minor copyright fix.

[Darin Adler]

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)]

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 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 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 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 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 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 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 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 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 on attachment 43164 [details]
Refactoring TextBoundariesICU.cpp into generic TextBoundaries.cpp (v3)

Bug 31461 again. :(

[Eric Seidel (no email)]

Landed as r50977.

[Eric Seidel (no email)]

Committed r50980: <http://trac.webkit.org/changeset/50980>

[Eric Seidel (no email)]

Windows and Chromium builds broke after the first commit.  The second commit was to fix them.

[Dominik Röttsches (drott)]

Thanks for additional review and build fixes, Eric!