Bug 123265 - PCRE workaround in Source/WTF/wtf/text/StringStatics.cpp can be removed
Summary: PCRE workaround in Source/WTF/wtf/text/StringStatics.cpp can be removed
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 123284 123558
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-24 05:44 PDT by Peter Molnar
Modified: 2016-03-22 14:09 PDT (History)
9 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Molnar 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
Comment 1 Peter Molnar 2013-10-24 05:46:43 PDT
Created attachment 215057 [details]
Remove PCRE workaround from Source/WTF/wtf/text/StringStatics.cpp
Comment 2 WebKit Commit Bot 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>
Comment 3 WebKit Commit Bot 2013-10-24 09:08:12 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Alexey Proskuryakov 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
Comment 5 WebKit Commit Bot 2013-10-24 12:07:28 PDT
Re-opened since this is blocked by bug 123284
Comment 6 Peter Molnar 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.
Comment 7 Build Bot 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
Comment 8 Build Bot 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
Comment 9 Peter Molnar 2013-10-29 05:59:48 PDT
Created attachment 215379 [details]
Remove PCRE workaround - fix failing layout test.
Comment 10 Peter Molnar 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.
Comment 11 Brent Fulgham 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.
Comment 12 WebKit Commit Bot 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>
Comment 13 WebKit Commit Bot 2013-10-30 12:02:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Ryosuke Niwa 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.
Comment 16 WebKit Commit Bot 2013-10-30 23:28:47 PDT
Re-opened since this is blocked by bug 123558
Comment 17 Brent Fulgham 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?
Comment 18 Brent Fulgham 2016-03-22 14:09:33 PDT
The implementation of this empty string function is totally different now.