Document attribute charset and its aliases to return UTF-8

Source/core/dom/Document.cpp

Index: Source/core/dom/Document.cpp
diff --git a/Source/core/dom/Document.cpp b/Source/core/dom/Document.cpp
index 770524fd52e9b5655e3be6c233914a6481137d15..e01a4a46f0749f5d103fbb777574384414e1f83e 100644
--- a/Source/core/dom/Document.cpp
+++ b/Source/core/dom/Document.cpp
@@ -1128,7 +1128,7 @@ String Document::defaultCharset() const
 {
     if (Settings* settings = this->settings())
         return settings->defaultTextEncodingName();

[philipj_slow, 2015/06/15 15:31:38]

Is it possible that defaultTextEncodingName() is a null string? I think that's
the default state, right?

[Habib Virji, 2015/06/15 16:14:55]

Yes it is null string, with your below code snippet (
document.implementation.createHTMLDocument('').defaultCharset) it returns
undefined in the current code. With the change it returns UTF-8.

-    return String();
+    return String("UTF-8");

[philipj_slow, 2015/06/15 15:31:38]

It seems a bit strange return "UTF-8" here if that isn't the encoding that's
going to be used by other callers to defaultTextEncodingName(). For
|document.implementation.createHTMLDocument('').defaultCharset|, which code path
is being taken, is settings null?

[Habib Virji, 2015/06/15 16:14:55]

|document.implementation.createHTMLDocument('').defaultCharset| will be null
without this change as settings will be null. document-attribute-js-null.html
without this string will be null and thus updated it to return correct value. 

[philipj_slow, 2015/06/16 22:05:57]

This case is tricky, I'm unsure what to do. It's strange to return anything
other than the encoding that will actually be used, but in order to not depend
on the settings object we'd have to make the default encoding a part of
Document, and make createDocument() and createHTMLDocument() get the default
encoding from the creating context. However, if the same logic isn't used in
TextResourceDecoderBuilder::createDecoderInstance that would just be a fancy
lie...

Document.defaultCharset is non-standard so there's nothing to guide us here. I
think the best thing is probably to leave it out of this CL and only deal with
Document.characterSet and its many aliases here. When Document.defaultCharset is
the last remaining user of [TreatReturnedNullStringAs=Undefined] let's try
again.

 }
 
 void Document::setContentLanguage(const AtomicString& language)
@@ -4216,8 +4216,11 @@ void Document::setEncodingData(const DocumentEncodingData& newData)
         String correctlyDecodedTitle = codec->decode(originalBytes.data(), originalBytes.length(), DataEOF);
         m_titleElement->setTextContent(correctlyDecodedTitle);
     }
-
-    m_encodingData = newData;
+    if (!newData.encoding().name()) {
+        m_encodingData = DocumentEncodingData();

[philipj_slow, 2015/06/15 15:31:38]

Is this code path reachable? Is there a test that would fail if it wasn't here?

[Habib Virji, 2015/06/15 16:14:55]

With the change in DocumentEncoderData it should not be. 

[philipj_slow, 2015/06/16 22:05:57]

OK, so would an ASSERT(newData.encoding().isValid()) here always pass? There are
a few callers to Document::setEncodingData and it's not obvious at first glance.

+    } else {
+        m_encodingData = newData;
+    }
 
     // FIXME: Should be removed as part of https://code.google.com/p/chromium/issues/detail?id=319643
     bool shouldUseVisualOrdering = m_encodingData.encoding().usesVisualOrdering();