Top-level domain-based default encoding

third_party/WebKit/Source/core/loader/TextResourceDecoderBuilder.cpp

 /*
  * 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:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 16 matching lines @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "core/loader/TextResourceDecoderBuilder.h"
 
 #include "core/dom/Document.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
 #include "platform/weborigin/SecurityOrigin.h"
+#include "wtf/HashMap.h"
 
 namespace blink {
 
 static inline bool canReferToParentFrameEncoding(const LocalFrame* frame, const LocalFrame* parentFrame)
 {
     return parentFrame && parentFrame->document()->securityOrigin()->canAccess(frame->document()->securityOrigin());
 }
 
+static WTF::HashMap<String, WTF::TextEncoding>* getEncodingMap()

[esprehn, 2016/02/26 09:43:13]

if you use a Map you should return by const ref here

[Jinsuk Kim, 2016/03/02 03:46:08]

I chose to use a simple array. Please see if it looks okay.

+{
+    static WTF::HashMap<String, WTF::TextEncoding>* map {};
+    if (!map) {
+        map = new WTF::HashMap<String, WTF::TextEncoding>();

[aelias_OOO_until_Jul13, 2016/02/26 09:07:24]

Hmm, in Chromium we would use something like LAZY_INSTANCE_INITIALIZER for this,
but it seems there's no Blink equivalent and I find this mutable static object
that's never deleted kind of sketchy.

Given that the performance advantage of HashMap is really negligible here --
indeed the cost of constructing it might overwhelm it -- how about just having a
simple const array of pairs of const char*s, and doing a linear search on it? 
You would create the WTF::TextEncoding object after the search, not before.

[esprehn, 2016/02/26 09:43:13]

This should use DEFINE_STATIC_LOCAL and map.isEmpty() for the init path

[Jinsuk Kim, 2016/03/02 03:46:08]

Please see the reply above.

[Jinsuk Kim, 2016/03/02 03:46:08]

I agree. It got complicated that I had wished. Rewrote with an array with a
linear search.

+        const WTF::TextEncoding windows1250("windows-1250");
+        const WTF::TextEncoding windows1251("windows-1251");
+        const WTF::TextEncoding windows1252("windows-1252");
+        const WTF::TextEncoding windows1255("windows-1255");
+        const WTF::TextEncoding windows1256("windows-1256");
+        const WTF::TextEncoding windows1257("windows-1257");
+        const WTF::TextEncoding windows1258("windows-1258");
+        const WTF::TextEncoding windows874("windows-874");
+        const WTF::TextEncoding windows949("windows-949");
+        const WTF::TextEncoding iso88592("ISO-8859-2");
+        const WTF::TextEncoding iso88597("ISO-8859-7");
+        const WTF::TextEncoding iso88599("ISO-8859-9");
+        const WTF::TextEncoding big5("Big5");
+        const WTF::TextEncoding gbk("GBK");
+        const WTF::TextEncoding shiftJis("Shift_JIS");
+
+        struct {
+            const char* domain;
+            const WTF::TextEncoding& encoding;
+        } encodingMap[] = {
+            { "au", windows1252 },
+            { "az", iso88599 },
+            { "bd", windows1252 },
+            { "bg", windows1251 },
+            { "br", windows1252 },
+            { "ca", windows1252 },
+            { "ch", windows1252 },
+            { "cn", gbk },
+            { "cz", windows1250 },
+            { "de", windows1252 },
+            { "dk", windows1252 },
+            { "ee", windows1257 },
+            { "eg", windows1256 },
+            { "et", windows1252 },
+            { "fi", windows1252 },
+            { "fr", windows1252 },
+            { "gb", windows1252 },
+            { "gr", iso88597 },
+            { "hk", big5 },
+            { "hr", windows1250 },
+            { "hu", iso88592 },
+            { "il", windows1255 },
+            { "ir", windows1256 },
+            { "is", windows1252 },
+            { "it", windows1252 },
+            { "jp", shiftJis },
+            { "kr", windows949 },
+            { "lt", windows1257 },
+            { "lv", windows1257 },
+            { "mk", windows1251 },
+            { "nl", windows1252 },
+            { "no", windows1252 },
+            { "pl", iso88592 },
+            { "pt", windows1252 },
+            { "ro", iso88592 },
+            { "rs", windows1251 },
+            { "ru", windows1251 },
+            { "se", windows1252 },
+            { "si", iso88592 },
+            { "sk", windows1250 },
+            { "th", windows874 },
+            { "tr", iso88599 },
+            { "tw", big5 },
+            { "tz", windows1252 },
+            { "ua", windows1251 },
+            { "us", windows1252 },
+            { "vn", windows1258 },
+            { "xa", windows1252 },
+            { "xb", windows1256 }
+        };
+        for (size_t i = 0; i < WTF_ARRAY_LENGTH(encodingMap); ++i)
+            map->add(String(encodingMap[i].domain), encodingMap[i].encoding);

[esprehn, 2016/02/26 09:43:13]

lets not do this, make this method into something like findEncoding(String) and
use std::lower_bound

[Jinsuk Kim, 2016/03/02 03:46:08]

Rewrote it with an array.

+    }
+    return map;
+}
+
+static const WTF::TextEncoding getEncodingFromDomain(const KURL& url)
+{
+    Vector<String> tokens;
+    url.host().split(String(".", 1), tokens);

[esprehn, 2016/02/26 09:43:13]

you don't need to do String(".", 1), just pass "." and it'll implicitly
construct, or better yet do split('.', tokens) which will take the split(UChar
separator, ...) fast path.

[Jinsuk Kim, 2016/03/02 03:46:08]

Better. Thanks. Done.

+    if (tokens.size() > 0) {
+        String tld = tokens[tokens.size()-1];
+        auto map = getEncodingMap();
+        if (map->find(tld) != map->end())
+            return map->get(tld);

[esprehn, 2016/02/26 09:43:13]

lets use std::lower_bound on a static list instead, you can construct a fresh
TextEncoding on return.

[Jinsuk Kim, 2016/03/02 03:46:08]

Rewrote it with an array.

+    }
+    return WTF::TextEncoding();
+}
 
 TextResourceDecoderBuilder::TextResourceDecoderBuilder(const AtomicString& mimeType, const AtomicString& encoding)
     : m_mimeType(mimeType)
     , m_encoding(encoding)
 {
 }
 
 TextResourceDecoderBuilder::~TextResourceDecoderBuilder()
 {
 }
 
 
 inline PassOwnPtr<TextResourceDecoder> TextResourceDecoderBuilder::createDecoderInstance(Document* document)
 {
+    const WTF::TextEncoding encodingFromDomain = getEncodingFromDomain(document->url());
     if (LocalFrame* frame = document->frame()) {
         if (Settings* settings = frame->settings())
-            return TextResourceDecoder::create(m_mimeType, settings->defaultTextEncodingName(), settings->usesEncodingDetector());
+            return TextResourceDecoder::create(m_mimeType, encodingFromDomain.isValid() ? encodingFromDomain : settings->defaultTextEncodingName(), settings->usesEncodingDetector());

[aelias_OOO_until_Jul13, 2016/02/26 09:07:24]

Looks OK, can you add a test verifying (for one or two of the domains, doesn't
need to be all) that the domain-based one is used instead of the
settings->defaultTextEncodingName()?

[Jinsuk Kim, 2016/03/02 03:46:08]

Added tests.

     }
 
-    return TextResourceDecoder::create(m_mimeType, String());
+    return TextResourceDecoder::create(m_mimeType, encodingFromDomain);
 }
 
 inline void TextResourceDecoderBuilder::setupEncoding(TextResourceDecoder* decoder, Document* document)
 {
     LocalFrame* frame = document->frame();
     LocalFrame* parentFrame = 0;
     if (frame && frame->tree().parent() && frame->tree().parent()->isLocalFrame())
         parentFrame = toLocalFrame(frame->tree().parent());
 
     if (!m_encoding.isEmpty())
@@ skipping 23 matching lines @@
     setupEncoding(decoder.get(), document);
     return decoder.release();
 }
 
 void TextResourceDecoderBuilder::clear()
 {
     m_encoding = nullAtom;
 }
 
 } // namespace blink