Properly check visibility of check_connection_header

chrome/browser/net/errorpage_browsertest.cc

Index: chrome/browser/net/errorpage_browsertest.cc
diff --git a/chrome/browser/net/errorpage_browsertest.cc b/chrome/browser/net/errorpage_browsertest.cc
index ec0fdef5a5333b3dc856ed8f9c991a09569bad3f..4b2bdd1dd308a4ad299e798cbeddbef7a00ab24b 100644
--- a/chrome/browser/net/errorpage_browsertest.cc
+++ b/chrome/browser/net/errorpage_browsertest.cc
@@ -105,6 +105,20 @@ bool WARN_UNUSED_RESULT IsDisplayingText(Browser* browser,
   return result;
 }
 
+// Returns true if #details is displayed on the page |browser| is currently
+// displaying.
+bool WARN_UNUSED_RESULT IsDisplayingDetails(Browser* browser) {

[mmenke, 2017/05/03 15:10:59]

Could we instead make IsDiplayingText check if any parent node of the one
displaying the text has the hidden tag?  I'm concerned about this regressing and
silently causing tests to pass, particularly since it's only used twice.

[Nate Fischer, 2017/05/03 17:54:43]

In general, it seems rather difficult to get a node (and therefore parent nodes)
by innerText. For Safe Browsing, we look up nodes by ID:
https://cs.chromium.org/chromium/src/chrome/browser/safe_browsing/safe_browsi...

Unfortunately, this particular chunk of text doesn't have an associated ID. So
we'd first need to change the tests to check elements with IDs, in addition to
adding code to recursively check parent nodes. Is that the route we want to go?

[mmenke, 2017/05/04 19:03:33]

I don't suppose something like this would work:

var node = document.evaluate("//div[contains(text(),'ERR_FAILED')]", document,
null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeValue;
domAutomationController.send(node && !node.classList.contains('hidden'))

If we're hiding a higher level node, we could also traverse the node hierarchy
upwards from the node we find, which will be the innermost one.

+  std::string command = R"(
+      var node = document.getElementById('details');
+      var isVisible = !node.classList.contains('hidden');
+      domAutomationController.send(isVisible);
+  )";
+  bool result = false;
+  EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
+      browser->tab_strip_model()->GetActiveWebContents(), command, &result));
+  return result;
+}
+
 // Expands the more box on the currently displayed error page.
 void ToggleHelpBox(Browser* browser) {
   EXPECT_TRUE(content::ExecuteScript(
@@ -1123,8 +1137,11 @@ IN_PROC_BROWSER_TEST_F(ErrorPageAutoReloadTest, ManualReloadNotSuppressed) {
   EXPECT_EQ(2, interceptor()->requests());
 
   ToggleHelpBox(browser());
-  EXPECT_TRUE(IsDisplayingText(browser(), l10n_util::GetStringUTF8(
-      IDS_ERRORPAGES_SUGGESTION_CHECK_CONNECTION_HEADER)));
+  EXPECT_TRUE(
+      IsDisplayingText(
+          browser(), l10n_util::GetStringUTF8(
+                         IDS_ERRORPAGES_SUGGESTION_CHECK_CONNECTION_HEADER)) &&
+      IsDisplayingDetails(browser()));
 
   content::WebContents* web_contents =
     browser()->tab_strip_model()->GetActiveWebContents();
@@ -1132,8 +1149,11 @@ IN_PROC_BROWSER_TEST_F(ErrorPageAutoReloadTest, ManualReloadNotSuppressed) {
   web_contents->GetMainFrame()->ExecuteJavaScriptForTests(
       base::ASCIIToUTF16("document.getElementById('reload-button').click();"));
   nav_observer.Wait();
-  EXPECT_FALSE(IsDisplayingText(browser(), l10n_util::GetStringUTF8(
-      IDS_ERRORPAGES_SUGGESTION_CHECK_CONNECTION_HEADER)));
+  EXPECT_FALSE(
+      IsDisplayingText(
+          browser(), l10n_util::GetStringUTF8(
+                         IDS_ERRORPAGES_SUGGESTION_CHECK_CONNECTION_HEADER)) &&
+      IsDisplayingDetails(browser()));
 }
 
 // Make sure that a same document navigation does not cause issues with the