Save-Page-As-Complete-Html: Each frame links to a distinct local file.

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 <stddef.h>
 #include <stdint.h>
 #include <string>
 #include <utility>
 #include <vector>
 
@@ skipping 1096 matching lines @@
   std::string arr[] = {
       "frames-runtime-changes.htm: 4388232f-8d45-4d2e-9807-721b381be153",
       "subframe1: 21595339-61fc-4854-b6df-0668328ea263",
       "subframe2: adf55719-15e7-45be-9eda-d12fe782a1bd",
       "subframe3: 50e294bf-3a5b-499d-8772-651ead26952f",
       "subframe4: e0ea9289-7467-4d32-ba5c-c604e8d84cb7",
   };
   std::vector<std::string> expected_substrings(std::begin(arr), std::end(arr));
 
   if (save_page_type == content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML) {
-    // TODO(lukasza): crbug.com/106364: Expand complete-html test beyond just
-    // being a crash test.  In particular, the |complete_html_arr| below should
-    // be the same as the |arr| above (and at this point the special-casing of
+    // TODO(lukasza): crbug.com/106364: Expand complete-html test to cover all
+    // test frames.  In particular, the |complete_html_arr| below should be the

[Randy Smith (Not in Mondays), 2016/01/22 15:28:39]

I don't understand what is missing from this CL to enable this functionality? 
Are we dependent in the code somewhere on there being an associated URL for
frames even after this change?

[Łukasz Anforowicz, 2016/01/22 17:39:40]

This CL almost gets us there - the missing thing is handling of frames without
src attribute - these won't get rewritten to point to a locally saved file.  The
CL to handle src/srcdoc will come next, after the current CL lands.  A draft is
at crrev.com/1500103002.

+    // same as the |arr| above (and at this point the special-casing of
     // complete-html can be removed).
     // Draft CLs with fix proposals that should accomplish this:
     // - crrev.com/1502563004
     // - crrev.com/1500103002
     std::string complete_html_arr[] = {
         "frames-runtime-changes.htm: 4388232f-8d45-4d2e-9807-721b381be153",
+        "subframe1: 21595339-61fc-4854-b6df-0668328ea263",
+        "subframe2: adf55719-15e7-45be-9eda-d12fe782a1bd",
     };
     expected_substrings = std::vector<std::string>(
         std::begin(complete_html_arr), std::end(complete_html_arr));
   }
 
   GURL url(embedded_test_server()->GetURL(
       "a.com", "/save_page/frames-runtime-changes.htm?do_runtime_changes=1"));

[Randy Smith (Not in Mondays), 2016/01/22 15:28:39]

nit, not this CL: This usage implies that do_runtime_changes=0 would inhibit the
runtime changes, but if I understand the test JS correctly, it's just the
existence of the "do_runtime_changes" string that initiates the runtime changes.

[Łukasz Anforowicz, 2016/01/22 17:39:39]

Oh, right.  Yes - I am not quite happy with how this is done.  Basically, I want
to make sure that original page (the one being saved) gets its content populated
at runtime by javascript, while the saved page doesn't (i.e. the saved page
should have the final content already).

I wish javascript had a better way of handling query parameters.  Still - it
shouldn't be too difficult to add "=1" to the javascript in the test file.  I'll
try to remember to do it on Monday (when I get back my desktop PC after the move
to a new building).

[Łukasz Anforowicz, 2016/01/25 19:04:05]

Done (tweaked the test html / javascript so that do_runtime_changes will only
take effect if its set to 1).

 
   TestMultiFramePage(save_page_type, url, 5, expected_substrings);
 }
 
 // Test for saving frames with various encodings:
 // - 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) {
@@ skipping 46 matching lines @@
   TestMultiFramePage(save_page_type, url, 6, expected_substrings);
 }
 
 INSTANTIATE_TEST_CASE_P(
     SaveType,
     SavePageMultiFrameBrowserTest,
     ::testing::Values(content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML,
                       content::SAVE_PAGE_TYPE_AS_MHTML));
 
 }  // namespace