FTP: Multiple fixes for localized directory listings:

base/i18n/icu_encoding_detection.cc

Index: base/i18n/icu_encoding_detection.cc
diff --git a/base/i18n/icu_encoding_detection.cc b/base/i18n/icu_encoding_detection.cc
index d579af280a7af7959ca6d48a3f125c87b98cb13c..4bce18f5bdfd09f5968208f2d96dabab1a4080c0 100644
--- a/base/i18n/icu_encoding_detection.cc
+++ b/base/i18n/icu_encoding_detection.cc
@@ -1,9 +1,11 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/i18n/icu_encoding_detection.h"
 
+#include <set>
+
 #include "base/string_util.h"
 #include "unicode/ucsdet.h"
 
@@ -45,6 +47,13 @@ bool DetectAllEncodings(const std::string& text,
     return false;
   }
 
+  // ICU has some heuristics for encoding detection, such that the more likely
+  // encodings should be returned first. However, it doesn't always return
+  // all encodings that properly decode |text|, so we'll append more encodings
+  // later. To make that efficient, keep track of encodings sniffed in this
+  // first phase.

[jungshik at Google, 2011/04/01 18:28:41]

hmm... if ucsdet_detectAll does not return all the detectable encodings that are
compatible with an input text, that's a bug. Could you send me a couple of
sample input strings to me by email? 

[Paweł Hajdan Jr., 2011/04/01 18:45:42]

See the raw listing attached to the bug. That was the test case I used.

+  std::set<std::string> sniffed_encodings;
+
   encodings->clear();
   for (int i = 0; i < matches_count; i++) {
     UErrorCode get_name_status = U_ZERO_ERROR;
@@ -55,7 +64,23 @@ bool DetectAllEncodings(const std::string& text,
       continue;
 
     encodings->push_back(encoding_name);
+    sniffed_encodings.insert(encoding_name);
+  }
+
+  // Append all encodings not included earlier, in arbitrary order.

[jungshik at Google, 2011/04/01 18:28:41]

This behavior should be documented in the header file. Adding all the detectable
encodings (regardless of whether or not they're compatible with the input text)
is not what customers of this function would expect. 

Or, instead of adding all the detectable encodings in the loop below, each
encoding to add should be tested to see if they can be used to convert the input
before being added.

Anyway, the above ICU 'bug' is fixed. This should not be necessary once it's
fixed. So, please send me details about cases where 'ucsdet_detectAll' does not
behave as advertised. 

[Paweł Hajdan Jr., 2011/04/01 18:45:42]

Well, even before this change that was true, i.e. ucsdet_detectAll can return
encodings that fail to decode the text and it is documented by ICU.
The FTP code has to do that anyway, so for efficiency I didn't include that code
here. However, it could indeed be a better API if only "valid" encodings were
returned by this function. If you think it's worth to change, I can do that.
Right, this shouldn't be necessary. Do you want me to add a TODO(jshin)?

+  UEnumeration* detectable_encodings = ucsdet_getAllDetectableCharsets(detector,
+                                                                       &status);
+  int detectable_count = uenum_count(detectable_encodings, &status);
+  for (int i = 0; i < detectable_count; i++) {
+    int name_length;
+    const char* name_raw = uenum_next(detectable_encodings,
+                                      &name_length,
+                                      &status);
+    std::string name(name_raw, name_length);
+    if (sniffed_encodings.find(name) == sniffed_encodings.end())
+      encodings->push_back(name);
   }
+  uenum_close(detectable_encodings);
 
   ucsdet_close(detector);
   return !encodings->empty();