Bug 130438 - Small cleanup of empty string
Summary: Small cleanup of empty string
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-18 18:45 PDT by Gavin Barraclough
Modified: 2014-03-19 12:02 PDT (History)
4 users (show)

See Also:

Attachments
Fix (5.37 KB, patch)
2014-03-18 19:05 PDT, Gavin Barraclough 		andersca: review+
Details | Formatted Diff | Diff
Patch with review comments for EWS (5.72 KB, patch)
2014-03-18 19:41 PDT, Gavin Barraclough 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 Gavin Barraclough 2014-03-18 18:45:59 PDT 
Some prep for https://bugs.webkit.org/show_bug.cgi?id=128624.
Comment 1 Gavin Barraclough 2014-03-18 19:05:32 PDT 
Created attachment 227144 [details]
Fix
Comment 2 Andreas Kling 2014-03-18 19:08:22 PDT 
The PCRE workaround removal has been attempted before, see bug 123265.
Comment 3 Gavin Barraclough 2014-03-18 19:21:15 PDT 
(In reply to comment #2)
> The PCRE workaround removal has been attempted before, see bug 123265.

Hmmm, interesting. :-(

The tests mentioned in the bug pass for me, maybe I should try again. :-/
Comment 4 Anders Carlsson 2014-03-18 19:22:15 PDT 
Comment on attachment 227144 [details]
Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=227144&action=review

> Source/WTF/wtf/text/StringImpl.h:164
> +        , m_data8(reinterpret_cast<LChar*>(1))

Even though this is what the comment said, I think it's awkward. I think you should do what we do for the StringImpl(CreateEmptyUniqueTag) constructor instead:

// We expect m_length to be initialized to 0 as we use it
// to represent a null terminated buffer.
, m_data8(reinterpret_cast<const LChar*>(&m_length))

> Source/WTF/wtf/text/StringStatics.cpp:46
> +    DEPRECATED_DEFINE_STATIC_LOCAL(StringImpl, emptyString, (ConstructEmptyString));

Please change this to

static NeverDestroyed<StringImpl> emptyString(ConstructEmptyString);

instead.
Comment 5 Gavin Barraclough 2014-03-18 19:41:44 PDT 
Created attachment 227148 [details]
Patch with review comments for EWS
Comment 6 Gavin Barraclough 2014-03-19 12:02:56 PDT 
Fixed in r165906