Commit c75a03a3 authored by Reilly Grant's avatar Reilly Grant Committed by Commit Bot

Revert "[iOS] Fix NTP presentation on bad ssl pages"

This reverts commit 677e751d.

Reason for revert: Failures on ios-simulator-noncq.

https://ci.chromium.org/p/chromium/builders/ci/ios-simulator-noncq/3637

Original change's description:
> [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}

TBR=justincohen@chromium.org,gambard@chromium.org,livvielin@chromium.org

Change-Id: I62d19870a0ad15fbfc4725f43de6eae28950f04e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2145763Reviewed-by: default avatarReilly Grant <reillyg@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758227}
parent 023255df
......@@ -15,7 +15,6 @@ 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",
]
......
......@@ -57,8 +57,6 @@ 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);
......
......@@ -14,7 +14,6 @@
#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"
......@@ -139,23 +138,6 @@ 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);
......
......@@ -369,7 +369,6 @@ 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",
......@@ -473,7 +472,6 @@ 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",
......
// 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment