[iOS] Fix NTP presentation on bad ssl pages

This CL makes sure that we don't display the NTP if the loaded page is
an interstitial page.
This is caused by the behaviour of interstitial when the committed
interstitial feature isn't enabled. In that case the workflow is:
1. about:newtab is loaded
2. User loads bad.badssl.com
3. Navigation did start for bad.badssl.com
4. Navigation ends for bad.badssl.com
5. This is a bad ssl page so Chrome actually reverts to about:newtab
6. Load stops, but at this point the visible URL is about:newtab
7. The interstitial is displayed and the visible URL is updated to
   bad.badssl.com
8. The observers are notified through DidChangeVisibleSecurityState
9. No further callbacks

So the main issue is that there is not callback notifying that the
visible URL has changed.
This CL fixes it by adding a callback on DidChangeVisibleSecurityState
and also on LoadStop to avoid having the NTP flashing between 6 and 7.

Fixed: 1067250
Change-Id: Ifc57addf5a6fa9e6295446da34e117028add73fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2141971
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Livvie Lin <livvielin@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758163}

ios/chrome/browser/ntp/BUILD.gn

@@ -15,6 +15,7 @@ source_set("ntp") {
     "//components/strings:components_strings_grit",
     "//ios/chrome/browser",
     "//ios/chrome/browser/ui:feature_flags",
+    "//ios/web/common:features",
     "//ios/web/public",
     "//ui/base:base",
   ]

ios/chrome/browser/ntp/new_tab_page_tab_helper.h

@@ -57,6 +57,8 @@ class NewTabPageTabHelper : public web::WebStateObserver,
   void DidFinishNavigation(web::WebState* web_state,
                            web::NavigationContext* navigation_context) override;
   void DidStopLoading(web::WebState* web_state) override;
+  void DidChangeVisibleSecurityState(web::WebState* web_state) override;
+  void DidStartLoading(web::WebState* web_state) override;
 
   // Enable or disable the tab helper.
   void SetActive(bool active);

ios/chrome/browser/ntp/new_tab_page_tab_helper.mm

@@ -14,6 +14,7 @@
 #include "ios/chrome/browser/chrome_url_constants.h"
 #include "ios/chrome/browser/ntp/features.h"
 #include "ios/chrome/browser/ntp/new_tab_page_tab_helper_delegate.h"
+#include "ios/web/common/features.h"
 #import "ios/web/public/navigation/navigation_context.h"
 #import "ios/web/public/navigation/navigation_item.h"
 #import "ios/web/public/navigation/navigation_manager.h"
@@ -138,6 +139,23 @@ void NewTabPageTabHelper::DidFinishNavigation(
   SetActive(IsNTPURL(web_state->GetLastCommittedURL()));
 }
 
+void NewTabPageTabHelper::DidChangeVisibleSecurityState(
+    web::WebState* web_state) {
+  if (base::FeatureList::IsEnabled(web::features::kSSLCommittedInterstitials))
+    return;
+  // This is the only callback we receive when loading a bad ssl page.
+  if (!IsNTPURL(web_state->GetVisibleURL())) {
+    SetActive(false);
+  }
+}
+
+void NewTabPageTabHelper::DidStartLoading(web::WebState* web_state) {
+  // This is needed to avoid flashing the NTP when loading error pages.
+  if (!IsNTPURL(web_state->GetVisibleURL())) {
+    SetActive(false);
+  }
+}
+
 void NewTabPageTabHelper::DidStopLoading(web::WebState* web_state) {
   if (IsNTPURL(web_state->GetVisibleURL())) {
     SetActive(true);

ios/chrome/browser/web/BUILD.gn

@@ -369,6 +369,7 @@ source_set("eg_tests") {
     "progress_indicator_egtest.mm",
     "push_and_replace_state_navigation_egtest.mm",
     "restore_egtest.mm",
+    "ssl_egtest.mm",
     "stop_loading_egtest.mm",
     "tab_order_egtest.mm",
     "visible_url_egtest.mm",
@@ -472,6 +473,7 @@ source_set("eg2_tests") {
     "progress_indicator_egtest.mm",
     "push_and_replace_state_navigation_egtest.mm",
     "restore_egtest.mm",
+    "ssl_egtest.mm",
     "stop_loading_egtest.mm",
     "tab_order_egtest.mm",
     "visible_url_egtest.mm",

ios/chrome/browser/web/ssl_egtest.mm

+// Copyright 2020 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 "base/bind.h"
+#include "components/strings/grit/components_strings.h"
+#import "ios/chrome/test/earl_grey/chrome_earl_grey.h"
+#import "ios/chrome/test/earl_grey/chrome_test_case.h"
+#import "ios/testing/earl_grey/earl_grey_test.h"
+#include "ios/testing/embedded_test_server_handlers.h"
+#include "net/test/embedded_test_server/default_handlers.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
+#include "net/test/embedded_test_server/request_handler_util.h"
+#include "ui/base/l10n/l10n_util.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+@interface SSLTestCase : ChromeTestCase {
+  std::unique_ptr<net::test_server::EmbeddedTestServer> _HTTPSServer;
+}
+
+@end
+
+@implementation SSLTestCase
+
+- (void)setUp {
+  [super setUp];
+  _HTTPSServer = std::make_unique<net::test_server::EmbeddedTestServer>(
+      net::test_server::EmbeddedTestServer::TYPE_HTTPS);
+  RegisterDefaultHandlers(_HTTPSServer.get());
+}
+
+// Test loading a page with a bad SSL certificate from the NTP, to avoid
+// https://crbug.com/1067250 from regressing.
+- (void)testBadSSLOnNTP {
+  GREYAssertTrue(_HTTPSServer->Start(), @"Test server failed to start.");
+
+  const GURL pageURL = _HTTPSServer->GetURL("/echo");
+  [ChromeEarlGrey loadURL:pageURL];
+
+  [ChromeEarlGrey waitForWebStateContainingText:l10n_util::GetStringUTF8(
+                                                    IDS_SSL_V2_HEADING)];
+}
+
+@end