Bindings: Add ScalarValueString support

Source/wtf/text/StringImpl.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller ( mueller@kde.org )
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2013 Apple Inc. All rights reserved.
  * Copyright (C) 2006 Andrew Wellington (proton@wiretapped.net)
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 1875 matching lines @@
     } else {
         // Cases 2 & 4.
         memcpy(data + dstOffset, characters16() + srcSegmentStart, srcSegmentLength * sizeof(UChar));
     }
 
     ASSERT(dstOffset + srcSegmentLength == newImpl->length());
 
     return newImpl.release();
 }
 
+PassRefPtr<StringImpl> StringImpl::replaceUnpairedSurrogates()

[Nils Barth (inactive), 2014/06/11 03:54:26]

Could you link to spec, and quote spec in comments?
http://heycam.github.io/webidl/#dfn-obtain-unicode

(Also aligning variable names with spec: S, n, U)

[jsbell, 2014/06/12 17:45:55]

Done.

+{
+    if (is8Bit())
+        return this;
+
+    const UChar* characters = characters16();
+    const unsigned length = m_length;
+
+    // Scan to see if there are any unmatched surrogates.

[Nils Barth (inactive), 2014/06/11 03:54:26]

Could you comment that this is a Blink-specific optimization to avoid copying if
no changes needed?

[jsbell, 2014/06/12 17:45:55]

Done.

+    bool unmatched = false;

[Nils Barth (inactive), 2014/06/11 03:54:26]

This block would be a bit simpler as a helper function, no?

Then we could just say:
if (!hasUnpairedSurrogates())
    return this;
...and just have 'if ... return true;' instead of lots of unmatched = true and
breaks.

Also, unmatched vs. unpaired (standard).

[jsbell, 2014/06/12 17:45:55]

Done.

+    for (unsigned i = 0; i < length; ++i) {
+        UChar c = characters[i];
+        if (c < 0xD800 || c > 0xDFFF) {

[Nils Barth (inactive), 2014/06/11 03:54:26]

Understand "want to match below", though style-wise I'd generally write
if ... continue;
...to avoid a sequence of else ifs and make it a bit flatter.

[jsbell, 2014/06/12 17:45:56]

Done.

+            // Non-surrogate - no-op.
+        } else if (0xDC00 <= c && c <= 0xDFFF) {
+            // Unmatched trail surrogate.
+            unmatched = true;
+            break;
+        } else if (i == length - 1) {
+            // Unmatched lead surrogate at EOF.
+            unmatched = true;
+            break;
+        } else {
+            UChar d = characters[i + 1];
+            if (0xDC00 <= d && d <= 0xDFFF) {
+                // Matching trail surrogate.
+                ++i;
+                continue;
+            }
+            // Unmatched lead.
+            unmatched = true;
+            break;
+        }
+    }
+    if (!unmatched)
+        return this;
+
+    UChar* data;
+    RefPtr<StringImpl> newImpl = createUninitialized(length, data);
+    for (unsigned i = 0; i < length; ++i) {
+        UChar c = characters[i];
+        if (c < 0xD800 || c > 0xDFFF) {
+            // Non-surrogate.
+            data[i] = c;
+        } else if (0xDC00 <= c && c <= 0xDFFF) {
+            // Unmatched trail surrogate.
+            data[i] = Unicode::replacementCharacter;
+        } else if (i == length - 1) {

[Nils Barth (inactive), 2014/06/11 03:54:26]

Could you add a comment stating:
// 0xD800 <= c && c <= 0xDBFF (Lead surrogate)

Also, the term "surrogate pair" is confusing if unfamiliar; could you link to
the spec?
http://www.unicode.org/versions/Unicode6.2.0/ch03.pdf#G2630

[jsbell, 2014/06/12 17:45:56]

I added ASSERTs instead.
Done.

+            // Unmatched lead surrogate at EOF.
+            data[i] = Unicode::replacementCharacter;
+        } else {
+            UChar d = characters[i + 1];
+            if (0xDC00 <= d && d <= 0xDFFF) {
+                // Matching trail surrogate.
+                data[i] = c;

[Nils Barth (inactive), 2014/06/11 03:54:26]

Could you comment that we're not translating the surrogate pair to a Unicode
character here (and note where it happens instead)?

[jsbell, 2014/06/12 17:45:55]

Done.

+                data[i + 1] = d;
+                ++i;
+            } else {
+                // Unmatched lead.
+                data[i] = Unicode::replacementCharacter;
+            }
+        }
+    }
+
+    return newImpl.release();
+}
+

[Nils Barth (inactive), 2014/06/11 03:54:26]

nit: one blank line, right?

[jsbell, 2014/06/12 17:45:55]

Done.

+
 PassRefPtr<StringImpl> StringImpl::upconvertedString()
 {
     if (is8Bit())
         return String::make16BitFrom8BitSource(characters8(), m_length).releaseImpl();
     return this;
 }
 
 static inline bool stringImplContentEqual(const StringImpl* a, const StringImpl* b)
 {
     unsigned aLength = a->length();
@@ skipping 193 matching lines @@
 
 size_t StringImpl::sizeInBytes() const
 {
     size_t size = length();
     if (!is8Bit())
         size *= 2;
     return size + sizeof(*this);
 }
 
 } // namespace WTF