Properly check visibility of check_connection_header

chrome/browser/net/errorpage_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 <memory>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
@@ skipping 76 matching lines @@
 using net::URLRequestFailedJob;
 using net::URLRequestTestJob;
 
 namespace {
 
 // Returns true if |text| is displayed on the page |browser| is currently
 // displaying.  Uses "innerText", so will miss hidden text, and whitespace
 // 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));
   return result;
 }
 
 // Expands the more box on the currently displayed error page.
 void ToggleHelpBox(Browser* browser) {
   EXPECT_TRUE(content::ExecuteScript(
       browser->tab_strip_model()->GetActiveWebContents(),
@@ skipping 1004 matching lines @@
   GURL test_url("http://error.page.auto.reload");
   const int kRequestsToFail = 3;
   InstallInterceptor(test_url, kRequestsToFail);
   ui_test_utils::NavigateToURLBlockUntilNavigationsComplete(
       browser(), test_url, 2);
 
   EXPECT_EQ(2, interceptor()->failures());
   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();
   content::TestNavigationObserver nav_observer(web_contents, 1);
   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
 // auto-reload timer.  Note that this test was added due to this case causing
 // a crash.  On regression, this test may hang due to a crashed renderer.
 IN_PROC_BROWSER_TEST_F(ErrorPageAutoReloadTest, IgnoresSameDocumentNavigation) {
   GURL test_url("http://error.page.auto.reload");
   InstallInterceptor(test_url, 2);
 
   // Wait for the error page and first autoreload, which happens immediately.
@@ skipping 386 matching lines @@
                        Http09WeirdPortEnabled) {
   const char kHttp09Response[] = "JumboShrimp";
   ASSERT_TRUE(embedded_test_server()->Start());
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(std::string("/echo-raw?") +
                                                 kHttp09Response));
   EXPECT_TRUE(IsDisplayingText(browser(), kHttp09Response));
 }
 
 }  // namespace