Tweaking WebPageSerializerImpl to emit a BOM for UTF16/32.

chrome/browser/download/save_page_browsertest.cc

 // Copyright (c) 2012 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/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
@@ skipping 955 matching lines @@
     EXPECT_EQ(expected_number_of_frames, actual_number_of_frames);
 
     for (const auto& expected_substring : expected_substrings) {
       int actual_number_of_matches = ui_test_utils::FindInPage(
           GetCurrentTab(browser()), base::UTF8ToUTF16(expected_substring),
           true,  // |forward|
           true,  // |case_sensitive|
           nullptr, nullptr);
 
       EXPECT_EQ(1, actual_number_of_matches)
-          << "Verifying if \"" << expected_substring << "\" appears "
-          << "exactly once in the web-contents text";
+          << "Verifying that \"" << expected_substring << "\" appears "
+          << "exactly once in the text of web contents";
     }
 
-    int actual_number_of_errors = ui_test_utils::FindInPage(
-        GetCurrentTab(browser()), base::UTF8ToUTF16("err"),
-        true,   // |forward|
-        false,  // |case_sensitive|
-        nullptr, nullptr);
-    EXPECT_EQ(0, actual_number_of_errors);
+    std::vector<std::string> forbidden_substrings{

[jsbell, 2015/12/08 01:19:07]

http://chromium-cpp.appspot.com/ says that the "uniform initialization syntax"
is banned. :(

[Łukasz Anforowicz, 2015/12/09 00:50:59]

Yes, sorry about that.  I should have rebased onto crrev.com/1441553002 (which
transitions to arrays) earlier.

+        "head"  // Html markup should not be visible.
+        "err",  // "err" is a prefix of error messages + is included as text
+                // content of <object> elements in some test files (text content
+                // would be rendered in case the object itself doesn't work).
+    };
+    for (const auto& forbidden_substring : forbidden_substrings) {
+      int actual_number_of_matches = ui_test_utils::FindInPage(
+          GetCurrentTab(browser()), base::UTF8ToUTF16(forbidden_substring),
+          true,  // |forward|
+          true,  // |case_sensitive|
+          nullptr, nullptr);
+
+      EXPECT_EQ(0, actual_number_of_matches)
+          << "Verifying that \"" << forbidden_substring << "\" doesn't "
+          << "appear in the text of web contents";
+    }
   }
 
   static void IncrementInteger(int* i, content::RenderFrameHost* /* unused */) {
     (*i)++;
   }
 };
 
 // Test coverage for OOPIFs for CompleteHtml (crbug.com/526786) and
 // MHTML (crbug.com/538766) as well as for redirected iframes saved
 // as MHTML (crbug.com/539936).
@@ skipping 73 matching lines @@
       "frames-nested2.htm: 6d23dc47-f283-4977-96ec-66bcf72301a4",
       "b.htm: 3a35f7fa-96a9-4487-9f18-4470263907fa",
   };
 
   GURL url(
       embedded_test_server()->GetURL("a.com", "/save_page/frames-nested.htm"));
 
   TestMultiFramePage(save_page_type, url, 3, expected_substrings);
 }
 
+// Test for saving frames with various encodings (crbug.com/541699).

[jsbell, 2015/12/08 01:19:07]

I don't think including the bug# here is necessary; this comment explains what's
going on.

[Łukasz Anforowicz, 2015/12/09 00:50:59]

Done.

+// - iso-8859-2: encoding declared via <meta> element
+// - utf16-le-bom.htm, utf16-be-bom.htm: encoding detected via BOM
+// - utf16-le-nobom.htm, utf16-le-nobom.htm, utf32.htm - encoding declared via
+//                                                       mocked http headers
+IN_PROC_BROWSER_TEST_P(SavePageMultiFrameBrowserTest, Encoding) {
+  content::SavePageType save_page_type = GetParam();
+
+  std::vector<std::string> expected_substrings{

[jsbell, 2015/12/08 01:19:07]

(see above)

[Łukasz Anforowicz, 2015/12/09 00:50:59]

Done.

+      "frames-encodings.htm: f53295dd-a95b-4b32-85f5-b6e15377fb20",
+      "iso-8859-2.htm: Zażółć gęślą jaźń",

[jsbell, 2015/12/08 01:19:07]

Use \x## escapes so that the source is ASCII when possible, as not all
developers configure editors for UTF-8.

[Łukasz Anforowicz, 2015/12/09 00:50:59]

Is \u escape ok?  It fits into 80 columns limit better...

[jsbell, 2015/12/09 01:02:47]

https://google.github.io/styleguide/cppguide.html#Non-ASCII_Characters seems to
say that you need to use u8"...." to ensure it's encoded as UTF-8, and the u8
prefix is on the banned list https://chromium-cpp.appspot.com/ since it doesn't
work in MSVS2013 yet.

So... looks like no. :(

[Łukasz Anforowicz, 2015/12/09 01:27:36]

Done.

+      "utf16-le-nobom.htm: Zażółć gęślą jaźń",
+      "utf16-le-bom.htm: Zażółć gęślą jaźń",
+      "utf16-be-nobom.htm: Zażółć gęślą jaźń",
+      "utf16-be-bom.htm: Zażółć gęślą jaźń",
+      "utf32.htm: Zażółć gęślą jaźń",
+  };
+
+  GURL url(embedded_test_server()->GetURL("a.com",
+                                          "/save_page/frames-encodings.htm"));
+
+  // TODO(lukasza): crbug.com/541699: MHTML needs to handle multi-byte encodings
+  // by either:
+  // 1. Continuing to preserve the original encoding, but starting to round-trip
+  //    the encoding declaration (in Content-Type MIME/MHTML header?)
+  // 2. Saving html docs in UTF8.
+  // 3. Saving the BOM (this won't help for all cases though).
+  if (save_page_type == content::SAVE_PAGE_TYPE_AS_MHTML)
+    return;
+
+  TestMultiFramePage(save_page_type, url, 7, expected_substrings);
+}
+
 INSTANTIATE_TEST_CASE_P(
     ,
     SavePageMultiFrameBrowserTest,
     ::testing::Values(content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML,
                       content::SAVE_PAGE_TYPE_AS_MHTML));
 
 }  // namespace