Document attribute charset and its aliases to return UTF-8

Source/core/dom/Document.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) Research In Motion Limited 2010-2011. All rights reserved.
@@ skipping 1109 matching lines @@
 {
     // TextEncoding::name() returns a char*, no need to allocate a new
     // String for it each time.
     // FIXME: We should fix TextEncoding to speak AtomicString anyway.
     return AtomicString(encoding().name());
 }
 
 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)
 {
     if (m_contentLanguage == language)
         return;
     m_contentLanguage = language;
 
     // Document's style depends on the content language.
     setNeedsStyleRecalc(SubtreeStyleChange, StyleChangeReasonForTracing::create(StyleChangeReason::Language));
@@ skipping 3067 matching lines @@
         && encoding() != newData.encoding()
         && !ElementTraversal::firstWithin(*m_titleElement)
         && encoding() == Latin1Encoding()
         && m_titleElement->textContent().containsOnlyLatin1()) {
 
         CString originalBytes = m_titleElement->textContent().latin1();
         OwnPtr<TextCodec> codec = newTextCodec(newData.encoding());
         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();
     if (shouldUseVisualOrdering != m_visuallyOrdered) {
         m_visuallyOrdered = shouldUseVisualOrdering;
         // FIXME: How is possible to not have a layoutObject here?
         if (layoutView())
             layoutView()->mutableStyleRef().setRTLOrdering(m_visuallyOrdered ? VisualOrder : LogicalOrder);
         setNeedsStyleRecalc(SubtreeStyleChange, StyleChangeReasonForTracing::create(StyleChangeReason::VisuallyOrdered));
     }
@@ skipping 1507 matching lines @@
 #ifndef NDEBUG
 using namespace blink;
 void showLiveDocumentInstances()
 {
     WeakDocumentSet& set = liveDocumentSet();
     fprintf(stderr, "There are %u documents currently alive:\n", set.size());
     for (Document* document : set)
         fprintf(stderr, "- Document %p URL: %s\n", document, document->url().string().utf8().data());
 }
 #endif