Commit bba29adb authored by Tibor Goldschwendt's avatar Tibor Goldschwendt Committed by Commit Bot

WebUI NTP: redirect sign-in to chrome://newtab (reland #2)

This is a reland of 142274a8 with a fix
for DiceBrowserTest.EnableSyncAfterToken. Original change's info:
> Bug: 1015293
> Change-Id: Ic76e7911403a925fa46a5b5fc58eca95fa292269
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2209392
> Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
> Auto-Submit: Esmael Elmoslimany <aee@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#771116}

Bug: 1015293, 1085540
Change-Id: Ifad9f0583ef0f5ec82b20068bd186b923d4e4eaa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2216325Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Auto-Submit: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772253}
parent 25bd437f
...@@ -27,6 +27,7 @@ ...@@ -27,6 +27,7 @@
#include "chrome/browser/policy/cloud/user_policy_signin_service_internal.h" #include "chrome/browser/policy/cloud/user_policy_signin_service_internal.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/search/ntp_features.h"
#include "chrome/browser/signin/account_consistency_mode_manager.h" #include "chrome/browser/signin/account_consistency_mode_manager.h"
#include "chrome/browser/signin/account_reconcilor_factory.h" #include "chrome/browser/signin/account_reconcilor_factory.h"
#include "chrome/browser/signin/chrome_device_id_helper.h" #include "chrome/browser/signin/chrome_device_id_helper.h"
...@@ -63,7 +64,9 @@ ...@@ -63,7 +64,9 @@
#include "components/variations/variations_switches.h" #include "components/variations/variations_switches.h"
#include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/load_notification_details.h"
#include "content/public/browser/notification_service.h" #include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_types.h"
#include "content/public/common/content_switches.h" #include "content/public/common/content_switches.h"
#include "content/public/test/browser_test.h" #include "content/public/test/browser_test.h"
#include "google_apis/gaia/gaia_switches.h" #include "google_apis/gaia/gaia_switches.h"
...@@ -871,9 +874,24 @@ IN_PROC_BROWSER_TEST_F(DiceBrowserTest, EnableSyncAfterToken) { ...@@ -871,9 +874,24 @@ IN_PROC_BROWSER_TEST_F(DiceBrowserTest, EnableSyncAfterToken) {
GetDeviceId().c_str()), GetDeviceId().c_str()),
dice_request_header_); dice_request_header_);
ui_test_utils::UrlLoadObserver ntp_url_observer( content::WindowedNotificationObserver ntp_url_observer(
GURL(chrome::kChromeSearchLocalNtpUrl), content::NOTIFICATION_LOAD_STOP,
content::NotificationService::AllSources()); base::BindRepeating([](const content::NotificationSource&,
const content::NotificationDetails& details) {
auto url =
content::Details<content::LoadNotificationDetails>(details)->url;
// Some test flags (e.g. ForceWebRequestProxyForTest) can change whether
// the reported NTP URL is the virtual chrome://newtab or one of the
// concrete chrome://new-tab-page or
// chrome-search://local-ntp/local-ntp.html. As far as this test is
// concerned either URL is fine.
auto concrete_ntp_url =
base::FeatureList::IsEnabled(ntp_features::kWebUI)
? GURL(chrome::kChromeUINewTabPageURL)
: GURL(chrome::kChromeSearchLocalNtpUrl);
return url == concrete_ntp_url ||
url == GURL(chrome::kChromeUINewTabURL);
}));
WaitForSigninSucceeded(); WaitForSigninSucceeded();
EXPECT_EQ(GetMainAccountID(), GetIdentityManager()->GetPrimaryAccountId()); EXPECT_EQ(GetMainAccountID(), GetIdentityManager()->GetPrimaryAccountId());
...@@ -933,7 +951,7 @@ IN_PROC_BROWSER_TEST_F(DiceBrowserTest, MAYBE_EnableSyncBeforeToken) { ...@@ -933,7 +951,7 @@ IN_PROC_BROWSER_TEST_F(DiceBrowserTest, MAYBE_EnableSyncBeforeToken) {
dice_request_header_); dice_request_header_);
ui_test_utils::UrlLoadObserver ntp_url_observer( ui_test_utils::UrlLoadObserver ntp_url_observer(
GURL(chrome::kChromeSearchLocalNtpUrl), GURL(chrome::kChromeUINewTabURL),
content::NotificationService::AllSources()); content::NotificationService::AllSources());
WaitForSigninSucceeded(); WaitForSigninSucceeded();
......
...@@ -24,7 +24,7 @@ namespace { ...@@ -24,7 +24,7 @@ namespace {
void RedirectToNtp(content::WebContents* contents) { void RedirectToNtp(content::WebContents* contents) {
VLOG(1) << "RedirectToNtp"; VLOG(1) << "RedirectToNtp";
contents->GetController().LoadURL( contents->GetController().LoadURL(
GURL(chrome::kChromeSearchLocalNtpUrl), content::Referrer(), GURL(chrome::kChromeUINewTabURL), content::Referrer(),
ui::PAGE_TRANSITION_AUTO_TOPLEVEL, std::string()); ui::PAGE_TRANSITION_AUTO_TOPLEVEL, std::string());
} }
......
...@@ -240,9 +240,8 @@ TEST_P(ProcessDiceHeaderDelegateImplTestEnableSync, EnableSync) { ...@@ -240,9 +240,8 @@ TEST_P(ProcessDiceHeaderDelegateImplTestEnableSync, EnableSync) {
CreateDelegateAndNavigateToSignin(GetParam().signin_tab); CreateDelegateAndNavigateToSignin(GetParam().signin_tab);
delegate->EnableSync(account_id_); delegate->EnableSync(account_id_);
EXPECT_EQ(GetParam().callback_called, enable_sync_called_); EXPECT_EQ(GetParam().callback_called, enable_sync_called_);
GURL expected_url = GetParam().show_ntp GURL expected_url =
? GURL(chrome::kChromeSearchLocalNtpUrl) GetParam().show_ntp ? GURL(chrome::kChromeUINewTabURL) : signin_url_;
: signin_url_;
EXPECT_EQ(expected_url, web_contents()->GetVisibleURL()); EXPECT_EQ(expected_url, web_contents()->GetVisibleURL());
EXPECT_FALSE(show_error_called_); EXPECT_FALSE(show_error_called_);
// Check that the sync signin flow is complete. // Check that the sync signin flow is complete.
...@@ -283,9 +282,8 @@ TEST_P(ProcessDiceHeaderDelegateImplTestHandleTokenExchangeFailure, ...@@ -283,9 +282,8 @@ TEST_P(ProcessDiceHeaderDelegateImplTestHandleTokenExchangeFailure,
delegate->HandleTokenExchangeFailure(email_, auth_error_); delegate->HandleTokenExchangeFailure(email_, auth_error_);
EXPECT_FALSE(enable_sync_called_); EXPECT_FALSE(enable_sync_called_);
EXPECT_EQ(GetParam().callback_called, show_error_called_); EXPECT_EQ(GetParam().callback_called, show_error_called_);
GURL expected_url = GetParam().show_ntp GURL expected_url =
? GURL(chrome::kChromeSearchLocalNtpUrl) GetParam().show_ntp ? GURL(chrome::kChromeUINewTabURL) : signin_url_;
: signin_url_;
EXPECT_EQ(expected_url, web_contents()->GetVisibleURL()); EXPECT_EQ(expected_url, web_contents()->GetVisibleURL());
// Check that the sync signin flow is complete. // Check that the sync signin flow is complete.
if (GetParam().signin_tab) { if (GetParam().signin_tab) {
......
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