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..f520c0bd6aa506e95e91c208aa654d397fd04bae 100644
--- a/chrome/browser/net/errorpage_browsertest.cc
+++ b/chrome/browser/net/errorpage_browsertest.cc
@@ -94,11 +94,24 @@ namespace {
 // space handling may be weird.
 bool WARN_UNUSED_RESULT IsDisplayingText(Browser* browser,
                                          const std::string& text) {
-  std::string command = base::StringPrintf(
-      "var textContent = document.body.innerText.toLowerCase();"
-      "var hasText = textContent.indexOf('%s'.toLowerCase()) >= 0;"
-      "domAutomationController.send(hasText);",
-      text.c_str());
+  // clang-format off
+  std::string command = base::StringPrintf(R"(
+    function isNodeVisible(node) {
+      if (!node) return 'node not found';

[mmenke, 2017/05/05 14:47:20]

nit:  split condition onto two lines.

[mmenke, 2017/05/05 14:47:20]

This seems a little weird to me.

Can we instead do:

if (!node)
  return false;
...
if (!node.parentElement)
  return true;
return isNodeVisible(node.parentElement);

(Could also combine the final three above lines, if you prefer)

[Nate Fischer, 2017/05/05 17:05:55]

Done.

[Nate Fischer, 2017/05/05 17:05:55]

Done.

+      if (node.classList.contains('hidden')) return false;
+      // Otherwise, we must check all parent nodes
+      var parentVisibility = isNodeVisible(node.parentElement);
+      if (parentVisibility === 'node not found') {
+        return true; // none of the parents are set invisible
+      }

[mmenke, 2017/05/05 14:47:20]

nit:  Just for consistency with this file, may want to not use braces for this
if.

[Nate Fischer, 2017/05/05 17:05:56]

Done.

+      return parentVisibility;
+    }
+    var node = document.evaluate("//*[contains(text(),'%s')]", document,
+      null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeValue;
+    var isVisible = Boolean(node && isNodeVisible(node));

[mmenke, 2017/05/05 14:52:44]

If you follow my suggestions above, this just becomes:

var isVisible = idNodeVisible(node);

And could even optionally inline it below.

[Nate Fischer, 2017/05/05 17:05:56]

Done.

+    domAutomationController.send(isVisible);
+  )", text.c_str());
+  // clang-format on
   bool result = false;
   EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
       browser->tab_strip_model()->GetActiveWebContents(), command, &result));
@@ -1123,8 +1136,9 @@ 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)));
 
   content::WebContents* web_contents =
     browser()->tab_strip_model()->GetActiveWebContents();
@@ -1132,8 +1146,9 @@ 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)));
 }
 
 // Make sure that a same document navigation does not cause issues with the