Top-level domain-based default encoding

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

Index: third_party/WebKit/Source/core/loader/TextResourceDecoderBuilder.cpp
diff --git a/third_party/WebKit/Source/core/loader/TextResourceDecoderBuilder.cpp b/third_party/WebKit/Source/core/loader/TextResourceDecoderBuilder.cpp
index 077ad6d0f5013afd3cd332a3a30ec748e3eb6825..63f29db7f71adec747f84e721a36989574117e13 100644
--- a/third_party/WebKit/Source/core/loader/TextResourceDecoderBuilder.cpp
+++ b/third_party/WebKit/Source/core/loader/TextResourceDecoderBuilder.cpp
@@ -34,6 +34,7 @@
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
 #include "platform/weborigin/SecurityOrigin.h"
+#include "wtf/HashMap.h"
 
 namespace blink {
 
@@ -42,6 +43,99 @@ static inline bool canReferToParentFrameEncoding(const LocalFrame* frame, const
     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)
@@ -56,12 +150,13 @@ 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)