Bug 123265

Summary:

PCRE workaround in Source/WTF/wtf/text/StringStatics.cpp can be removed

Product:

WebKit

Reporter:

Peter Molnar <pmolnar.u-szeged>

Component:

Web Template Framework

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED INVALID

Severity:

Normal

CC:

andersca, benjamin, bfulgham, buildbot, cmarcelo, commit-queue, kling, msaboff, rniwa

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

123284, 123558

Bug Blocks:

Attachments:

Description	Flags
Remove PCRE workaround from Source/WTF/wtf/text/StringStatics.cpp	none
Remove PCRE workaround from Source/WTF/wtf/text/StringStatics.cpp - fixed alignment problem	buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion	none
Remove PCRE workaround - fix failing layout test.	none

Peter Molnar

Reported 2013-10-24 05:44:19 PDT

PCRE workaround in Source/WTF/wtf/text/StringStatics.cpp can be removed, as PCRE was removed 2.5 years ago: http://trac.webkit.org/changeset/78211

Attachments
Remove PCRE workaround from Source/WTF/wtf/text/StringStatics.cpp (1.65 KB, patch) 2013-10-24 05:46 PDT, Peter Molnar	no flags	Details Formatted Diff Diff
Remove PCRE workaround from Source/WTF/wtf/text/StringStatics.cpp - fixed alignment problem (1.66 KB, patch) 2013-10-28 06:20 PDT, Peter Molnar	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (487.68 KB, application/zip) 2013-10-28 07:30 PDT, Build Bot	no flags	Details
Remove PCRE workaround - fix failing layout test. (1.65 KB, patch) 2013-10-29 05:59 PDT, Peter Molnar	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Peter Molnar

Comment 1 2013-10-24 05:46:43 PDT

Created attachment 215057 [details] Remove PCRE workaround from Source/WTF/wtf/text/StringStatics.cpp

WebKit Commit Bot

Comment 2 2013-10-24 09:08:10 PDT

Comment on attachment 215057 [details] Remove PCRE workaround from Source/WTF/wtf/text/StringStatics.cpp Clearing flags on attachment: 215057 Committed r157931: <http://trac.webkit.org/changeset/157931>

WebKit Commit Bot

Comment 3 2013-10-24 09:08:12 PDT

All reviewed patches have been landed. Closing bug.

Alexey Proskuryakov

Comment 4 2013-10-24 12:07:22 PDT

There are tons of string related assertions on bots now, this patch looks like the most likely culprit. Both layout tests are API tests are broken: http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29/builds/10963

WebKit Commit Bot

Comment 5 2013-10-24 12:07:28 PDT

Re-opened since this is blocked by bug 123284

Peter Molnar

Comment 6 2013-10-28 06:20:54 PDT

Created attachment 215299 [details] Remove PCRE workaround from Source/WTF/wtf/text/StringStatics.cpp - fixed alignment problem Fixed the alignment problem of previous patch, that caused failing API tests on Mac platforms.

Build Bot

Comment 7 2013-10-28 07:30:23 PDT

Comment on attachment 215299 [details] Remove PCRE workaround from Source/WTF/wtf/text/StringStatics.cpp - fixed alignment problem Attachment 215299 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/11168175 New failing tests: js/kde/StringObject.html

Build Bot

Comment 8 2013-10-28 07:30:24 PDT

Created attachment 215305 [details] Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Peter Molnar

Comment 9 2013-10-29 05:59:48 PDT

Created attachment 215379 [details] Remove PCRE workaround - fix failing layout test.

Peter Molnar

Comment 10 2013-10-29 08:27:32 PDT

With the latest patch, both API and layout test breakages are fixed. I only have access to a MacOSX 10.7 machine, API tests were passing on it as of r157986, the revision before dropping OSX 10.7 support.

Brent Fulgham

Comment 11 2013-10-30 11:28:53 PDT

Comment on attachment 215379 [details] Remove PCRE workaround - fix failing layout test. Let's try again! r=me.

WebKit Commit Bot

Comment 12 2013-10-30 12:02:02 PDT

Comment on attachment 215379 [details] Remove PCRE workaround - fix failing layout test. Clearing flags on attachment: 215379 Committed r158299: <http://trac.webkit.org/changeset/158299>

WebKit Commit Bot

Comment 13 2013-10-30 12:02:05 PDT

All reviewed patches have been landed. Closing bug.

Ryosuke Niwa

Comment 14 2013-10-30 23:25:05 PDT

This patch caused fast/canvas/canvas-color-serialization.html and fast/forms/input-text-paste-maxlength.html to hit assertions: http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fcanvas%2Fcanvas-color-serialization.html%2Cfast%2Fforms%2Finput-text-paste-maxlength.html e.g. http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK2%20(Tests)/r158341%20(13796)/results.html

Ryosuke Niwa

Comment 15 2013-10-30 23:27:42 PDT

I'm reverting the patch for now since I don't think this change fixes any bug in WebKit. I can reliably reproduce the crash on my Mountain Lion local machine (it also reproduced on Mavericks and GTK+) so please investigate why assertion failures are occurring before relanding the patch again.

WebKit Commit Bot

Comment 16 2013-10-30 23:28:47 PDT

Re-opened since this is blocked by bug 123558

Brent Fulgham

Comment 17 2013-10-30 23:52:38 PDT

This patch seems to cause large test breaks. Is it really worth landing? Alternatively, should these tests be re baselines or changed in some way to reduce flakiness?

Brent Fulgham

Comment 18 2016-03-22 14:09:33 PDT

The implementation of this empty string function is totally different now.

Note You need to log in before you can comment on or make changes to this bug.