Commit eb3847fd authored by Gauthier Ambard's avatar Gauthier Ambard Committed by Commit Bot

[iOS] Fix NTP visibility for cancelled navigations

When a navigation is cancelled, the NTP should still be presented.
Currently, as the navigation starts, the NTP is hidden and it is not
restored when the navigation is cancelled.

Fixed: 1063154
Change-Id: I0844fa735ae4e0f9e184d46ac6c96e809b873807
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2120352Reviewed-by: default avatarJustin Cohen <justincohen@chromium.org>
Reviewed-by: default avatarDavid Jean <djean@chromium.org>
Reviewed-by: default avatarStepan Khapugin <stkhapugin@chromium.org>
Auto-Submit: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754010}
parent ae091561
...@@ -56,6 +56,7 @@ class NewTabPageTabHelper : public web::WebStateObserver, ...@@ -56,6 +56,7 @@ class NewTabPageTabHelper : public web::WebStateObserver,
web::NavigationContext* navigation_context) override; web::NavigationContext* navigation_context) override;
void DidFinishNavigation(web::WebState* web_state, void DidFinishNavigation(web::WebState* web_state,
web::NavigationContext* navigation_context) override; web::NavigationContext* navigation_context) override;
void DidStopLoading(web::WebState* web_state) override;
// Enable or disable the tab helper. // Enable or disable the tab helper.
void SetActive(bool active); void SetActive(bool active);
......
...@@ -138,6 +138,12 @@ void NewTabPageTabHelper::DidFinishNavigation( ...@@ -138,6 +138,12 @@ void NewTabPageTabHelper::DidFinishNavigation(
SetActive(IsNTPURL(web_state->GetLastCommittedURL())); SetActive(IsNTPURL(web_state->GetLastCommittedURL()));
} }
void NewTabPageTabHelper::DidStopLoading(web::WebState* web_state) {
if (IsNTPURL(web_state->GetVisibleURL())) {
SetActive(true);
}
}
#pragma mark - Private #pragma mark - Private
void NewTabPageTabHelper::SetActive(bool active) { void NewTabPageTabHelper::SetActive(bool active) {
......
...@@ -206,6 +206,29 @@ id<GREYMatcher> OmniboxWidthBetween(CGFloat width, CGFloat margin) { ...@@ -206,6 +206,29 @@ id<GREYMatcher> OmniboxWidthBetween(CGFloat width, CGFloat margin) {
performAction:grey_tap()]; performAction:grey_tap()];
} }
// Tests that when loading an invalid URL, the NTP is still displayed.
// Prevents regressions from https://crbug.com/1063154 .
- (void)testInvalidURL {
NSString* URL = @"app-settings://test-test-test/";
// The URL needs to be typed to trigger the bug.
[[EarlGrey selectElementWithMatcher:chrome_test_util::FakeOmnibox()]
performAction:grey_typeText(URL)];
// The first suggestion is a search, the second suggestion is the URL.
[[EarlGrey
selectElementWithMatcher:
grey_allOf(
grey_accessibilityID(@"omnibox suggestion 1"),
grey_kindOfClassName(@"OmniboxPopupRowCell"),
grey_descendant(
chrome_test_util::StaticTextWithAccessibilityLabel(URL)),
grey_sufficientlyVisible(), nil)] performAction:grey_tap()];
[[EarlGrey selectElementWithMatcher:chrome_test_util::FakeOmnibox()]
assertWithMatcher:grey_sufficientlyVisible()];
}
// Tests that the fake omnibox width is correctly updated after a rotation. // Tests that the fake omnibox width is correctly updated after a rotation.
- (void)testOmniboxWidthRotation { - (void)testOmniboxWidthRotation {
// TODO(crbug.com/652465): Enable the test for iPad when rotation bug is // TODO(crbug.com/652465): Enable the test for iPad when rotation bug is
......
...@@ -40,6 +40,7 @@ source_set("util") { ...@@ -40,6 +40,7 @@ source_set("util") {
] ]
deps = [ deps = [
"//ios/chrome/browser", "//ios/chrome/browser",
"//ios/chrome/browser/ntp",
"//ios/web/public", "//ios/web/public",
] ]
} }
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#import "ios/chrome/browser/ui/ntp/ntp_util.h" #import "ios/chrome/browser/ui/ntp/ntp_util.h"
#include "ios/chrome/browser/chrome_url_constants.h" #include "ios/chrome/browser/chrome_url_constants.h"
#import "ios/chrome/browser/ntp/new_tab_page_tab_helper.h"
#import "ios/web/public/navigation/navigation_item.h" #import "ios/web/public/navigation/navigation_item.h"
#import "ios/web/public/navigation/navigation_manager.h" #import "ios/web/public/navigation/navigation_manager.h"
#import "ios/web/public/web_state.h" #import "ios/web/public/web_state.h"
...@@ -21,7 +22,9 @@ bool IsURLNewTabPage(const GURL& url) { ...@@ -21,7 +22,9 @@ bool IsURLNewTabPage(const GURL& url) {
bool IsVisibleURLNewTabPage(web::WebState* web_state) { bool IsVisibleURLNewTabPage(web::WebState* web_state) {
if (!web_state) if (!web_state)
return false; return false;
return IsURLNewTabPage(web_state->GetVisibleURL()); NewTabPageTabHelper* ntp_helper =
NewTabPageTabHelper::FromWebState(web_state);
return ntp_helper && ntp_helper->IsActive();
} }
bool IsNTPWithoutHistory(web::WebState* web_state) { bool IsNTPWithoutHistory(web::WebState* web_state) {
......
...@@ -73,6 +73,7 @@ source_set("unit_tests") { ...@@ -73,6 +73,7 @@ source_set("unit_tests") {
"//ios/chrome/browser/browser_state", "//ios/chrome/browser/browser_state",
"//ios/chrome/browser/browser_state:test_support", "//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/main:test_support", "//ios/chrome/browser/main:test_support",
"//ios/chrome/browser/ntp",
"//ios/chrome/browser/search_engines", "//ios/chrome/browser/search_engines",
"//ios/chrome/browser/tabs", "//ios/chrome/browser/tabs",
"//ios/chrome/browser/web", "//ios/chrome/browser/web",
......
...@@ -12,6 +12,8 @@ ...@@ -12,6 +12,8 @@
#include "base/strings/sys_string_conversions.h" #include "base/strings/sys_string_conversions.h"
#import "ios/chrome/browser/browser_state/test_chrome_browser_state.h" #import "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#include "ios/chrome/browser/main/test_browser.h" #include "ios/chrome/browser/main/test_browser.h"
#import "ios/chrome/browser/ntp/new_tab_page_tab_helper.h"
#import "ios/chrome/browser/ntp/new_tab_page_tab_helper_delegate.h"
#import "ios/chrome/browser/tabs/tab_helper_util.h" #import "ios/chrome/browser/tabs/tab_helper_util.h"
#import "ios/chrome/browser/tabs/tab_model.h" #import "ios/chrome/browser/tabs/tab_model.h"
#import "ios/chrome/browser/url_loading/app_url_loading_service.h" #import "ios/chrome/browser/url_loading/app_url_loading_service.h"
...@@ -169,6 +171,8 @@ TEST_F(URLLoadingBrowserAgentTest, TestSwitchToTabFromNTP) { ...@@ -169,6 +171,8 @@ TEST_F(URLLoadingBrowserAgentTest, TestSwitchToTabFromNTP) {
web_state_list->InsertWebState(0, std::move(web_state), web_state_list->InsertWebState(0, std::move(web_state),
WebStateList::INSERT_FORCE_INDEX, WebStateList::INSERT_FORCE_INDEX,
WebStateOpener()); WebStateOpener());
id mock_delegate = OCMProtocolMock(@protocol(NewTabPageTabHelperDelegate));
NewTabPageTabHelper::CreateForWebState(web_state_ptr, mock_delegate);
std::unique_ptr<web::TestWebState> web_state_2 = CreateTestWebState(); std::unique_ptr<web::TestWebState> web_state_2 = CreateTestWebState();
web::WebState* web_state_ptr_2 = web_state_2.get(); web::WebState* web_state_ptr_2 = web_state_2.get();
...@@ -202,6 +206,8 @@ TEST_F(URLLoadingBrowserAgentTest, TestSwitchToClosedTab) { ...@@ -202,6 +206,8 @@ TEST_F(URLLoadingBrowserAgentTest, TestSwitchToClosedTab) {
WebStateList::INSERT_FORCE_INDEX, WebStateList::INSERT_FORCE_INDEX,
WebStateOpener()); WebStateOpener());
web_state_list->ActivateWebStateAt(0); web_state_list->ActivateWebStateAt(0);
id mock_delegate = OCMProtocolMock(@protocol(NewTabPageTabHelperDelegate));
NewTabPageTabHelper::CreateForWebState(web_state_ptr, mock_delegate);
GURL url("http://test/2"); GURL url("http://test/2");
......
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