Moved text decoding to the parser thread

Source/core/html/parser/HTMLIdentifier.h

 /*
  * Copyright (C) 2013 Google, Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 38 matching lines @@
         if (m_string.impl())
             return;
         if (width == Likely8Bit)
             m_string = StringImpl::create8BitIfPossible(vector);
         else if (width == Force8Bit)
             m_string = String::make8BitFrom16BitSource(vector);
         else
             m_string = String(vector);
     }
 
+    HTMLIdentifier(const AtomicString& str, CharacterWidth width)

[abarth-chromium, 2013/11/27 18:18:08]

str -> string

Please use complete works in variable names.

It seems very strange that we would want to take an AtomicString parameter here.
 Why not a String parameter?

[oystein (OOO til 10th of July), 2013/11/27 18:46:33]

Done.
If I need to handle both the 8bit and the 16bit case, then yeah a String
parameter would make sense. If I was correct in the assumption below then maybe
it should be a QualifiedName parameter.

+        : m_string(findIfKnown(str.characters16(), str.length()))

[abarth-chromium, 2013/11/27 18:18:08]

How do you know it's safe to call characters16?  What if the string consists of
8 bit characters?

[oystein (OOO til 10th of July), 2013/11/27 18:46:33]

This is only ever used by the QualifiedName -> HTMLIdentifier conversion in
PageSerializer.cpp, I was assuming that'd always be unicode but if that's
incorrect I can add some handling of both (why do we have so many
almost-identical string classes, and keep having to convert between them?).

+    {
+        if (m_string.impl())
+            return;
+        if (width == Likely8Bit)
+            m_string = StringImpl::create8BitIfPossible(str.characters16(), str.length());
+        else if (width == Force8Bit)
+            m_string = String::make8BitFrom16BitSource(str.characters16(), str.length());
+        else
+            m_string = str;
+    }
+
     // asString should only be used on the main thread.
     const String& asString() const;
     // asStringImpl() is safe to call from any thread.
     const StringImpl* asStringImpl() const;
 
     static void init();
 
     bool isSafeToSendToAnotherThread() const { return m_string.isSafeToSendToAnotherThread(); }
 
+    bool operator==(const QualifiedName&) const;
+    bool operator!=(const QualifiedName& b) const;

[abarth-chromium, 2013/11/27 18:18:08]

Why does one of these functions have a formal parameter name while the other
does not?

I'd prefer we omitted these operators.  It's better for callers to be explicit
about what comparison operators they want to perform.

[oystein (OOO til 10th of July), 2013/11/27 18:46:33]

Hm what ambiguity is there when comparing a QualifiedName object and a
HTMLIdentifier object?

+
 #ifndef NDEBUG
     static bool isKnown(const StringImpl*);
 #endif
 
 private:
     static unsigned maxNameLength;
     static StringImpl* findIfKnown(const UChar* characters, unsigned length);
     static void addNames(const QualifiedName* const* names, unsigned namesCount);
 
     String m_string;
 };
 
 }
 
 #endif