Add extra logging and assertions to track flakiness of SavePage tests.

This CL tries to track down the flakiness of the following tests:
- SavePageOriginalVsSavedComparisonTest.ObjectElementsViaHttp
- SavePageOriginalVsSavedComparisonTest.CrossSiteObject
where the test will sometimes/flakily see that all the expected frames
are present, but find-in-page will fail to find text that is expected to
be found in one of the subframes.

After this CL, we should be able to verify if the problematic subframe
has finished loading - if it executed `window.name = ...` or not.

Bug: 1070597, 1070886
Change-Id: I7d499cbb228663b63030fac7dab37140b7c9be7f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2510913Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824222}

chrome/browser/download/save_page_browsertest.cc

@@ -1131,7 +1131,8 @@ class SavePageOriginalVsSavedComparisonTest
     DLOG(INFO) << "Verifying test expectations for original page... : "
                << GetCurrentTab(browser())->GetLastCommittedURL();
     AssertExpectationsAboutCurrentTab(
-        expected_number_of_frames_in_original_page, expected_substrings);
+        expected_number_of_frames_in_original_page, expected_substrings,
+        save_page_type);
 
     // Save the page.
     base::FilePath full_file_name, dir;
@@ -1159,7 +1160,7 @@ class SavePageOriginalVsSavedComparisonTest
         expected_number_of_frames_in_mhtml_page :
         expected_number_of_frames_in_original_page;
     AssertExpectationsAboutCurrentTab(expected_number_of_frames_in_saved_page,
-                                      expected_substrings);
+                                      expected_substrings, save_page_type);
 
     if (GetParam() == content::SAVE_PAGE_TYPE_AS_MHTML) {
       std::set<url::Origin> origins;
@@ -1175,8 +1176,9 @@ class SavePageOriginalVsSavedComparisonTest
     ui_test_utils::NavigateToURL(browser(), GURL("data:text/html,foo"));
     chrome::GoBack(browser(), WindowOpenDisposition::CURRENT_TAB);
     EXPECT_TRUE(content::WaitForLoadStop(GetCurrentTab(browser())));
+    DLOG(INFO) << "Verifying test expectations after history navigation...";
     AssertExpectationsAboutCurrentTab(expected_number_of_frames_in_saved_page,
-                                      expected_substrings);
+                                      expected_substrings, save_page_type);
   }
 
   // Helper method to deduplicate some code across 2 tests.
@@ -1219,10 +1221,10 @@ class SavePageOriginalVsSavedComparisonTest
  private:
   void AssertExpectationsAboutCurrentTab(
       int expected_number_of_frames,
-      const std::vector<std::string>& expected_substrings) {
-    int actual_number_of_frames = 0;
-    GetCurrentTab(browser())->ForEachFrame(base::BindRepeating(
-        &IncrementInteger, base::Unretained(&actual_number_of_frames)));
+      const std::vector<std::string>& expected_substrings,
+      content::SavePageType save_page_type) {
+    int actual_number_of_frames =
+        GetCurrentTab(browser())->GetAllFrames().size();
     EXPECT_EQ(expected_number_of_frames, actual_number_of_frames);
 
     for (const auto& expected_substring : expected_substrings) {
@@ -1235,6 +1237,22 @@ class SavePageOriginalVsSavedComparisonTest
       EXPECT_EQ(1, actual_number_of_matches)
           << "Verifying that \"" << expected_substring << "\" appears "
           << "exactly once in the text of web contents";
+
+      // TODO(lukasza): https://crbug.com/1070597 and https://crbug.com/1070886:
+      // Remove the extra test assertions below (and maybe also the
+      // |save_page_type| parameter) after we get a better understanding of the
+      // root cause of test flakiness.
+      if (expected_substring == "a.htm: 1b8aae2b-e164-462f-bd5b-98aa366205f2" &&
+          save_page_type == content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML) {
+        DLOG(INFO) << "Verifying that a.htm frame has fully loaded...";
+        std::vector<std::string> frame_names;
+        for (content::RenderFrameHost* frame :
+             GetCurrentTab(browser())->GetAllFrames()) {
+          frame_names.push_back(frame->GetFrameName());
+        }
+
+        EXPECT_THAT(frame_names, testing::Contains("Frame name of a.htm"));
+      }
     }
 
     std::string forbidden_substrings[] = {
@@ -1256,10 +1274,6 @@ class SavePageOriginalVsSavedComparisonTest
     }
   }
 
-  static void IncrementInteger(int* i, content::RenderFrameHost* /* unused */) {
-    (*i)++;
-  }
-
   static void CheckFrameForMHTML(std::set<url::Origin>* origins,
                                  content::RenderFrameHost* host) {
     // See RFC n°2557, section-8.3: "Use of the Content-ID header and CID URLs".
@@ -1468,9 +1482,11 @@ IN_PROC_BROWSER_TEST_P(SavePageOriginalVsSavedComparisonTest,
 }
 
 INSTANTIATE_TEST_SUITE_P(
-    SaveType,
+    SaveAsCompleteHtml,
     SavePageOriginalVsSavedComparisonTest,
-    ::testing::Values(content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML,
-                      content::SAVE_PAGE_TYPE_AS_MHTML));
+    ::testing::Values(content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML));
+INSTANTIATE_TEST_SUITE_P(SaveAsMhtml,
+                         SavePageOriginalVsSavedComparisonTest,
+                         ::testing::Values(content::SAVE_PAGE_TYPE_AS_MHTML));
 
 }  // namespace

chrome/test/data/frame_tree/cross_origin_but_same_site_frames.html

@@ -10,7 +10,7 @@
   <hr>
   Same origin, but different path:
   <iframe name="SameOrigin-DifferentPath"
-          src="/save_page/a.htm"></iframe>
+          src="/title1.html"></iframe>
   <hr>
   other.site.com site:
   <iframe name="OtherSubdomain-SameSite"

chrome/test/data/save_page/a.htm

@@ -5,9 +5,14 @@
     </title>
   </head>
   <body>
-    Content verification marker:
-    a.htm: 1b8aae2b-e164-462f-bd5b-98aa366205f2
+    <p>
+      Content verification marker:
+      a.htm: 1b8aae2b-e164-462f-bd5b-98aa366205f2
 
-    Can you see this sentence?
+      Can you see this sentence?
+    </p>
+    <script>
+      window.name = 'Frame name of a.htm';
+    </script>
   </body>
 </html>