Have SSLErrorHandler decide which type of interstitial to display

chrome/browser/ssl/ssl_error_handler.cc

Index: chrome/browser/ssl/ssl_error_handler.cc
diff --git a/chrome/browser/ssl/ssl_error_handler.cc b/chrome/browser/ssl/ssl_error_handler.cc
index e19b02bd5bad39066ca22ee530fd9aad1cb6c7b6..cb77d51ecff6a481fe6552c4233572d1a55981c7 100644
--- a/chrome/browser/ssl/ssl_error_handler.cc
+++ b/chrome/browser/ssl/ssl_error_handler.cc
@@ -8,11 +8,14 @@
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/stringprintf.h"
+#include "base/time/clock.h"
 #include "base/time/time.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ssl/bad_clock_blocking_page.h"
 #include "chrome/browser/ssl/ssl_blocking_page.h"
 #include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "chrome/browser/ssl/ssl_error_classification.h"
+#include "chrome/browser/ssl/ssl_error_info.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/render_frame_host.h"
@@ -40,6 +43,9 @@ int64 g_interstitial_delay_in_milliseconds = 2000;
 // Callback to call when the interstitial timer is started. Used for testing.
 SSLErrorHandler::TimerStartedCallback* g_timer_started_callback = nullptr;
 
+// The clock to use when deciding which error type to display. Used for testing.
+base::Clock* g_testing_clock = nullptr;
+
 // Events for UMA.
 enum SSLErrorHandlerEvent {
   HANDLE_ALL,
@@ -50,6 +56,7 @@ enum SSLErrorHandlerEvent {
   WWW_MISMATCH_FOUND,
   WWW_MISMATCH_URL_AVAILABLE,
   WWW_MISMATCH_URL_NOT_AVAILABLE,
+  SHOW_BAD_CLOCK,
   SSL_ERROR_HANDLER_EVENT_COUNT
 };
 
@@ -125,6 +132,19 @@ bool IsSSLCommonNameMismatchHandlingEnabled() {
          "Enabled";
 }
 
+bool IsErrorDueToBadClock(const base::Time& now, int error) {
+  if (SSLErrorInfo::NetErrorToErrorType(error) !=
+      SSLErrorInfo::CERT_DATE_INVALID) {
+    return false;
+  }
+  if (g_testing_clock) {
+    SSLErrorClassification::SetBuildTimeForTesting(

[estark, 2015/09/01 13:28:28]

nit: I'm curious if you considered calling this from the test instead of calling
it here. The advantages I see of doing that are:
1. you can make both the Set*ForTesting clock methods private and
FRIEND_TEST_ALL_PREFIXES the test.
2. it's not obvious from the outset that calling
SSLErrorHandler::SetClockForTesting() also affects the build time that'll be
used. So maybe SetClockForTesting() should be renamed
SetClockAndBuildTimeForTesting() to make it more clear, or the test itself could
call SetBuildTimeForTesting() to make it more explicit.

[felt, 2015/09/01 15:49:41]

Moved into the test.

+        base::Time::NowFromSystemTime());
+  }
+  return SSLErrorClassification::IsUserClockInThePast(now) ||
+         SSLErrorClassification::IsUserClockInTheFuture(now);
+}
+
 }  // namespace
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(SSLErrorHandler);
@@ -158,6 +178,11 @@ void SSLErrorHandler::SetInterstitialTimerStartedCallbackForTest(
   g_timer_started_callback = callback;
 }
 
+// static
+void SSLErrorHandler::SetClockForTest(base::Clock* testing_clock) {
+  g_testing_clock = testing_clock;
+}
+
 SSLErrorHandler::SSLErrorHandler(content::WebContents* web_contents,
                                  int cert_error,
                                  const net::SSLInfo& ssl_info,
@@ -181,6 +206,14 @@ SSLErrorHandler::~SSLErrorHandler() {
 void SSLErrorHandler::StartHandlingError() {
   RecordUMA(HANDLE_ALL);
 
+  const base::Time now = g_testing_clock == nullptr
+                             ? base::Time::NowFromSystemTime()
+                             : g_testing_clock->Now();
+  if (IsErrorDueToBadClock(now, cert_error_)) {
+    ShowBadClockInterstitial(now);
+    return;  // |this| is deleted after showing the interstitial.
+  }
+
   std::vector<std::string> dns_names;
   ssl_info_.cert->GetDNSNames(&dns_names);
   DCHECK(!dns_names.empty());
@@ -306,6 +339,16 @@ void SSLErrorHandler::ShowSSLInterstitial() {
   web_contents_->RemoveUserData(UserDataKey());
 }
 
+void SSLErrorHandler::ShowBadClockInterstitial(const base::Time& now) {
+  RecordUMA(SHOW_BAD_CLOCK);
+  (new BadClockBlockingPage(web_contents_, cert_error_, ssl_info_, request_url_,
+                            now, callback_))
+      ->Show();
+  // Once an interstitial is displayed, no need to keep the handler around.
+  // This is the equivalent of "delete this".
+  web_contents_->RemoveUserData(UserDataKey());
+}
+
 void SSLErrorHandler::CommonNameMismatchHandlerCallback(
     const CommonNameMismatchHandler::SuggestedUrlCheckResult& result,
     const GURL& suggested_url) {