Made UI changes to chrome://connection-help

Changed chrome://connection-help UI to automatically expand a section
if it matches the interstitial that caused the redirection.
Also removed windows only section on non-windows platforms.

Change-Id: I1287a4bdbc95d9cd5052212194450e72a3ebb18e
Reviewed-on: https://chromium-review.googlesource.com/988838
Commit-Queue: Carlos IL <carlosil@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548248}

chrome/browser/ssl/connection_help_tab_helper.cc

@@ -22,11 +22,15 @@ const char kHelpCenterConnectionHelpUrl[] =
 const char kBundledConnectionHelpUrl[] = "chrome://connection-help";
 
 void MaybeRedirectToBundledHelp(content::WebContents* web_contents) {
-  if (base::FeatureList::IsEnabled(features::kBundledConnectionHelpFeature)) {
+  if (!base::FeatureList::IsEnabled(features::kBundledConnectionHelpFeature))
+    return;
+  GURL::Replacements replacements;
+  std::string error_code = web_contents->GetURL().ref();
+  replacements.SetRefStr(error_code);
   web_contents->GetController().LoadURL(
-        GURL(kBundledConnectionHelpUrl), content::Referrer(),
-        ui::PageTransition::PAGE_TRANSITION_AUTO_TOPLEVEL, std::string());
-  }
+      GURL(kBundledConnectionHelpUrl).ReplaceComponents(replacements),
+      content::Referrer(), ui::PageTransition::PAGE_TRANSITION_AUTO_TOPLEVEL,
+      std::string());
 }
 }  // namespace
 
@@ -35,7 +39,10 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(ConnectionHelpTabHelper);
 ConnectionHelpTabHelper::~ConnectionHelpTabHelper() {}
 
 void ConnectionHelpTabHelper::DidAttachInterstitialPage() {
-  if (web_contents()->GetURL() == GetHelpCenterURL()) {
+  GURL::Replacements replacements;
+  replacements.ClearRef();
+  if (web_contents()->GetURL().ReplaceComponents(replacements) ==
+      GetHelpCenterURL()) {
     UMA_HISTOGRAM_ENUMERATION(
         "SSL.CertificateErrorHelpCenterVisited",
         ConnectionHelpTabHelper::LearnMoreClickResult::kFailedWithInterstitial,
@@ -46,7 +53,10 @@ void ConnectionHelpTabHelper::DidAttachInterstitialPage() {
 
 void ConnectionHelpTabHelper::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
-  if (web_contents()->GetURL() == GetHelpCenterURL()) {
+  GURL::Replacements replacements;
+  replacements.ClearRef();
+  if (web_contents()->GetURL().ReplaceComponents(replacements) ==
+      GetHelpCenterURL()) {
     LearnMoreClickResult histogram_value;
     if (navigation_handle->IsErrorPage()) {
       if (base::CommandLine::ForCurrentProcess()->HasSwitch(

chrome/browser/ssl/connection_help_tab_helper_browsertest.cc

@@ -201,3 +201,89 @@ IN_PROC_BROWSER_TEST_P(ConnectionHelpTabHelperTest, NetworkErrorOnSupportURL) {
       kHistogramName,
       ConnectionHelpTabHelper::LearnMoreClickResult::kFailedOther, 1);
 }
+
+// Tests that if the help content site is opened with an error code that refers
+// to a certificate error, the certificate error section is automatically
+// expanded.
+IN_PROC_BROWSER_TEST_P(ConnectionHelpTabHelperTest,
+                       CorrectlyExpandsCertErrorSection) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kBundledConnectionHelpFeature);
+
+  GURL expired_url = https_expired_server()->GetURL("/title2.html#-200");
+  GURL::Replacements replacements;
+  replacements.ClearRef();
+  SetHelpCenterUrl(browser(), expired_url.ReplaceComponents(replacements));
+
+  // Since ui_test_utils::NavigateToURL uses a TestNavigationObserver to wait
+  // for navigations, and TestNavigationObserver counts interstitials as a
+  // navigation, we need to wait for two navigations (the interstitial, and the
+  // help content) in the non-committed interstitial case. For committed
+  // interstitials, since the redirect happens before the original navigation
+  // finishes, we only need to wait for one.
+  if (AreCommittedInterstitialsEnabled()) {
+    ui_test_utils::NavigateToURL(browser(), expired_url);
+  } else {
+    ui_test_utils::NavigateToURLBlockUntilNavigationsComplete(browser(),
+                                                              expired_url, 2);
+  }
+
+  // Check that we got redirected to the offline help content.
+  base::string16 tab_title;
+  ui_test_utils::GetCurrentTabTitle(browser(), &tab_title);
+  EXPECT_EQ(base::UTF16ToUTF8(tab_title),
+            l10n_util::GetStringUTF8(IDS_CONNECTION_HELP_TITLE));
+
+  // Check that the cert error details section is not hidden.
+  std::string cert_error_is_hidden_js =
+      "var certSection = document.getElementById('details-certerror'); "
+      "window.domAutomationController.send(certSection.className == 'hidden');";
+  bool cert_error_is_hidden;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      cert_error_is_hidden_js, &cert_error_is_hidden));
+  EXPECT_FALSE(cert_error_is_hidden);
+}
+
+// Tests that if the help content site is opened with an error code that refers
+// to an expired certificate, the clock section is automatically expanded.
+IN_PROC_BROWSER_TEST_P(ConnectionHelpTabHelperTest,
+                       CorrectlyExpandsClockSection) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kBundledConnectionHelpFeature);
+
+  GURL expired_url = https_expired_server()->GetURL("/title2.html#-201");
+  GURL::Replacements replacements;
+  replacements.ClearRef();
+  SetHelpCenterUrl(browser(), expired_url.ReplaceComponents(replacements));
+
+  // Since ui_test_utils::NavigateToURL uses a TestNavigationObserver to wait
+  // for navigations, and TestNavigationObserver counts interstitials as a
+  // navigation, we need to wait for two navigations (the interstitial, and the
+  // help content) in the non-committed interstitial case. For committed
+  // interstitials, since the redirect happens before the original navigation
+  // finishes, we only need to wait for one.
+  if (AreCommittedInterstitialsEnabled()) {
+    ui_test_utils::NavigateToURL(browser(), expired_url);
+  } else {
+    ui_test_utils::NavigateToURLBlockUntilNavigationsComplete(browser(),
+                                                              expired_url, 2);
+  }
+
+  // Check that we got redirected to the offline help content.
+  base::string16 tab_title;
+  ui_test_utils::GetCurrentTabTitle(browser(), &tab_title);
+  EXPECT_EQ(base::UTF16ToUTF8(tab_title),
+            l10n_util::GetStringUTF8(IDS_CONNECTION_HELP_TITLE));
+
+  // Check that the clock details section is not hidden.
+  std::string clock_is_hidden_js =
+      "var clockSection = document.getElementById('details-clock');  "
+      "window.domAutomationController.send(clockSection.className == "
+      "'hidden');";
+  bool clock_is_hidden;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
+      browser()->tab_strip_model()->GetActiveWebContents(), clock_is_hidden_js,
+      &clock_is_hidden));
+  EXPECT_FALSE(clock_is_hidden);
+}

chrome/browser/ssl/ssl_browsertest.cc

@@ -4186,6 +4186,25 @@ IN_PROC_BROWSER_TEST_P(SSLUITest, ProceedLinkOverridable) {
   ASSERT_NO_FATAL_FAILURE(CheckProceedLinkExists(tab));
 }
 
+IN_PROC_BROWSER_TEST_P(SSLUITest, TestLearnMoreLinkContainsErrorCode) {
+  ASSERT_TRUE(https_server_expired_.Start());
+
+  // Navigate to a site that causes an interstitial.
+  ui_test_utils::NavigateToURL(browser(),
+                               https_server_expired_.GetURL("/title1.html"));
+  WaitForInterstitial(browser()->tab_strip_model()->GetActiveWebContents());
+
+  // Simulate clicking the learn more link.
+  SendInterstitialCommand(browser()->tab_strip_model()->GetActiveWebContents(),
+                          security_interstitials::CMD_OPEN_HELP_CENTER);
+  EXPECT_EQ(browser()
+                ->tab_strip_model()
+                ->GetActiveWebContents()
+                ->GetVisibleURL()
+                .ref(),
+            std::to_string(net::ERR_CERT_DATE_INVALID));
+}
+
 // Verifies that an overridable committed interstitial has a proceed link.
 IN_PROC_BROWSER_TEST_P(SSLUITestCommitted, ProceedLinkOverridable) {
   ASSERT_TRUE(https_server_expired_.Start());

components/security_interstitials/content/connection_help_ui.cc

@@ -4,6 +4,7 @@
 
 #include "components/security_interstitials/content/connection_help_ui.h"
 
+#include "build/build_config.h"
 #include "components/grit/components_resources.h"
 #include "components/security_interstitials/content/urls.h"
 #include "components/strings/grit/components_strings.h"
@@ -11,6 +12,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
 #include "content/public/browser/web_ui_data_source.h"
+#include "net/base/net_errors.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace security_interstitials {
@@ -20,6 +22,15 @@ ConnectionHelpUI::ConnectionHelpUI(content::WebUI* web_ui)
   content::WebUIDataSource* html_source =
       content::WebUIDataSource::Create(kChromeUIConnectionHelpHost);
 
+  // JS code needs these constants to decide which section to expand.
+  html_source->AddInteger("certCommonNameInvalid",
+                          net::ERR_CERT_COMMON_NAME_INVALID);
+  html_source->AddInteger("certExpired", net::ERR_CERT_DATE_INVALID);
+  html_source->AddInteger("certAuthorityInvalid",
+                          net::ERR_CERT_AUTHORITY_INVALID);
+  html_source->AddInteger("certWeakSignatureAlgorithm",
+                          net::ERR_CERT_WEAK_SIGNATURE_ALGORITHM);
+
   html_source->AddLocalizedString("connectionHelpTitle",
                                   IDS_CONNECTION_HELP_TITLE);
   html_source->AddLocalizedString("connectionHelpHeading",
@@ -43,10 +54,20 @@ ConnectionHelpUI::ConnectionHelpUI(content::WebUI* web_ui)
                                   IDS_CONNECTION_HELP_INCORRECT_CLOCK_TITLE);
   html_source->AddLocalizedString("connectionHelpIncorrectClockDetails",
                                   IDS_CONNECTION_HELP_INCORRECT_CLOCK_DETAILS);
+
+// The superfish section should only be added on Windows.
+#if defined(OS_WIN)
+  html_source->AddBoolean("isWindows", true);
   html_source->AddLocalizedString("connectionHelpMitmSoftwareTitle",
                                   IDS_CONNECTION_HELP_MITM_SOFTWARE_TITLE);
   html_source->AddLocalizedString("connectionHelpMitmSoftwareDetails",
                                   IDS_CONNECTION_HELP_MITM_SOFTWARE_DETAILS);
+#else
+  html_source->AddBoolean("isWindows", false);
+  html_source->AddString("connectionHelpMitmSoftwareTitle", "");
+  html_source->AddString("connectionHelpMitmSoftwareDetails", "");
+#endif
+
   html_source->AddLocalizedString("connectionHelpShowMore",
                                   IDS_CONNECTION_HELP_SHOW_MORE);
   html_source->AddLocalizedString("connectionHelpShowLess",

components/security_interstitials/content/resources/connection_help.html

@@ -41,7 +41,7 @@
           </div>
           <button id="details-clock-button" class="small-link">$i18n{connectionHelpShowMore}</button>
         </li>
-        <li>
+        <li id="windows-only" class="hidden">
             $i18nRaw{connectionHelpMitmSoftwareTitle}
             <div id="details-mitmsoftware" class="hidden">
               $i18nRaw{connectionHelpMitmSoftwareDetails}

components/security_interstitials/content/resources/connection_help.js

@@ -14,9 +14,22 @@ function setupEvents() {
   $('details-clock-button').addEventListener('click', function(event) {
     toggleHidden('details-clock', 'details-clock-button');
   });
+  if (loadTimeData.getBoolean('isWindows')) {
+    $('windows-only').classList.remove(HIDDEN_CLASS);
     $('details-mitmsoftware-button').addEventListener('click', function(event) {
       toggleHidden('details-mitmsoftware', 'details-mitmsoftware-button');
     });
+  }
+  switch (window.location.hash) {
+    case '#' + loadTimeData.getInteger('certCommonNameInvalid'):
+    case '#' + loadTimeData.getInteger('certAuthorityInvalid'):
+    case '#' + loadTimeData.getInteger('certWeakSignatureAlgorithm'):
+      toggleHidden('details-certerror', 'details-certerror-button');
+      break;
+    case '#' + loadTimeData.getInteger('certExpired'):
+      toggleHidden('details-clock', 'details-clock-button');
+      break;
+  }
 }
 
 function toggleHidden(className, buttonName) {

components/security_interstitials/core/ssl_error_ui.cc

@@ -165,13 +165,14 @@ void SSLErrorUI::PopulateNonOverridableStrings(
 
 void SSLErrorUI::HandleCommand(SecurityInterstitialCommand command) {
   switch (command) {
-    case CMD_DONT_PROCEED:
+    case CMD_DONT_PROCEED: {
       controller_->metrics_helper()->RecordUserDecision(
           MetricsHelper::DONT_PROCEED);
       user_made_decision_ = true;
       controller_->GoBack();
       break;
-    case CMD_PROCEED:
+    }
+    case CMD_PROCEED: {
       if (hard_override_enabled_) {
         controller_->metrics_helper()->RecordUserDecision(
             MetricsHelper::PROCEED);
@@ -179,50 +180,68 @@ void SSLErrorUI::HandleCommand(SecurityInterstitialCommand command) {
         user_made_decision_ = true;
       }
       break;
-    case CMD_DO_REPORT:
+    }
+    case CMD_DO_REPORT: {
       controller_->SetReportingPreference(true);
       break;
-    case CMD_DONT_REPORT:
+    }
+    case CMD_DONT_REPORT: {
       controller_->SetReportingPreference(false);
       break;
-    case CMD_SHOW_MORE_SECTION:
+    }
+    case CMD_SHOW_MORE_SECTION: {
       controller_->metrics_helper()->RecordUserInteraction(
           security_interstitials::MetricsHelper::SHOW_ADVANCED);
       break;
-    case CMD_OPEN_HELP_CENTER:
+    }
+    case CMD_OPEN_HELP_CENTER: {
       controller_->metrics_helper()->RecordUserInteraction(
           security_interstitials::MetricsHelper::SHOW_LEARN_MORE);
 
+      // Add cert error code as a ref to support URL, this is used to expand the
+      // right section if the user is redirected to chrome://connection-help.
+      GURL::Replacements replacements;
+      // This has to be stored in a separate variable, otherwise asan throws a
+      // use-after-scope error
+      std::string cert_error_string = std::to_string(cert_error_);
+      replacements.SetRefStr(cert_error_string);
       // If |support_url_| is invalid, use the default help center url.
       controller_->OpenUrlInNewForegroundTab(
-          support_url_.is_valid()
+          (support_url_.is_valid()
                ? support_url_
-              : controller_->GetBaseHelpCenterUrl().Resolve(kHelpPath));
+               : controller_->GetBaseHelpCenterUrl().Resolve(kHelpPath))
+              .ReplaceComponents(replacements));
       break;
-    case CMD_RELOAD:
+    }
+    case CMD_RELOAD: {
       controller_->metrics_helper()->RecordUserInteraction(
           security_interstitials::MetricsHelper::RELOAD);
       controller_->Reload();
       break;
-    case CMD_OPEN_REPORTING_PRIVACY:
+    }
+    case CMD_OPEN_REPORTING_PRIVACY: {
       controller_->OpenExtendedReportingPrivacyPolicy(true);
       break;
-    case CMD_OPEN_WHITEPAPER:
+    }
+    case CMD_OPEN_WHITEPAPER: {
       controller_->OpenExtendedReportingWhitepaper(true);
       break;
+    }
     case CMD_OPEN_DATE_SETTINGS:
     case CMD_OPEN_DIAGNOSTIC:
     case CMD_OPEN_LOGIN:
-    case CMD_REPORT_PHISHING_ERROR:
+    case CMD_REPORT_PHISHING_ERROR: {
       // Not supported by the SSL error page.
       NOTREACHED() << "Unsupported command: " << command;
       break;
+    }
     case CMD_ERROR:
     case CMD_TEXT_FOUND:
-    case CMD_TEXT_NOT_FOUND:
+    case CMD_TEXT_NOT_FOUND: {
       // Commands are for testing.
       break;
     }
+  }
 }
 
 }  // security_interstitials

components/security_interstitials_strings.grdp

@@ -334,9 +334,6 @@
   <message name="IDS_CONNECTION_HELP_INCORRECT_CLOCK_TITLE" desc="Title for incorrect clock section in chrome://connection-help. Matches the heading for the fifth section in https://support.google.com/chrome/answer/6098869">
     "Your clock is behind" or "Your clock is ahead" or "&lt;span class="error-code"&gt;NET::ERR_CERT_DATE_INVALID&lt;/span&gt;"
   </message>
-  <message name="IDS_CONNECTION_HELP_MITM_SOFTWARE_TITLE" desc="Title for MITM software section in chrome://connection-help. Matches the heading for the eight section in https://support.google.com/chrome/answer/6098869">
-    "Software on your computer is stopping Chrome from safely connecting to the web" (Windows computers only)
-  </message>
   <message name="IDS_CONNECTION_HELP_CONNECTION_NOT_PRIVATE_DETAILS" desc="Details for your connection is not private section in chrome://connection-help. Matches the third section in https://support.google.com/chrome/answer/6098869">
     &lt;h4&gt;Step 1: Sign in to the portal&lt;/h4&gt;
     &lt;p&gt;Wi-Fi networks at places like cafes or airports need you to sign in. To see the sign-in page, visit a page that uses &lt;code&gt;http://&lt;/code&gt;.&lt;/p&gt;
@@ -364,7 +361,11 @@
     &lt;p&gt;You'll see this error if your computer or mobile device's date and time are inaccurate.&lt;/p&gt;
     &lt;p&gt;To fix the error, open your device's clock. Make sure the time and date are correct.&lt;/p&gt;
   </message>
-  <message name="IDS_CONNECTION_HELP_MITM_SOFTWARE_DETAILS" desc="Details for MITM software section in chrome://connection-help. Matches the eight section in https://support.google.com/chrome/answer/6098869">
+  <if expr="is_win">
+    <message name="IDS_CONNECTION_HELP_MITM_SOFTWARE_TITLE" desc="Title for MITM software section in chrome://connection-help. Matches the heading for the eighth section in https://support.google.com/chrome/answer/6098869">
+    "Software on your computer is stopping Chrome from safely connecting to the web" (Windows computers only)
+  </message>
+    <message name="IDS_CONNECTION_HELP_MITM_SOFTWARE_DETAILS" desc="Details for MITM software section in chrome://connection-help. Matches the eighth section in https://support.google.com/chrome/answer/6098869">
       &lt;p&gt;You'll see this error if you have Superfish software on your Windows computer.&lt;/p&gt;
       &lt;p&gt;Follow these steps to temporarily disable the software so you can get on the web. You'll need administrator privileges.&lt;/p&gt;
       &lt;ol&gt;
@@ -376,4 +377,5 @@
       &lt;li&gt;Visit the &lt;a href="https://support.google.com/chrome/answer/6098869"&gt;Chrome help center&lt;/a&gt; to learn how to permanently remove the software from your computer
       &lt;/ol&gt;
     </message>
+  </if>
 </grit-part>