[signin] Fix redirection to the NTP after enabling sync

The redirect was broken by CL: https://crrev.com/c/4732981

This bug crept in because of two issues:
1) `ShowDiceSigninTab()` had a default value for the redirect parameter,
   and would do no redirection by default. This default behavior made it
   hard to update all callers in the previous CL and one of them was
   missed.
2) The integration test was using the deprecated `ShowSignin()` function
   instead of `ShowDiceEnableSyncTab()`, and as a result the broken
   function was not tested.

This CL addresses the two issues:
- removing the default empty redirect, to make sure all callers pass the
  right value explicitly.
- the test is switched away from the deprecated function and tests the
  recommended function instead. The deprecated function is losing some
  coverage, but it still has some, and there is only one caller of that
  function anyway. The deprecated function will be removed as a follow
  up.

The CL also improves comments for the redirect behavior.

(cherry picked from commit 85e079b)

Fixed: 1472219
Change-Id: I468483145beaebe54d43bf38527da9bbd2947b83
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4773361
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1182575}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4774723
Auto-Submit: David Roger <droger@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5938@{#80}
Cr-Branched-From: 2b50cb4-refs/heads/main@{#1181205}

Author: David Roger

chrome/browser/signin/dice_browsertest.cc

@@ -942,8 +942,10 @@ IN_PROC_BROWSER_TEST_F(DiceBrowserTest, EnableSyncAfterToken) {
   // Signin using the Chrome Sync endpoint.
   signin_metrics::AccessPoint access_point =
       signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS;
-  browser()->signin_view_controller()->ShowSignin(
-      profiles::BUBBLE_VIEW_MODE_GAIA_SIGNIN, access_point);
+  browser()->signin_view_controller()->ShowDiceEnableSyncTab(
+      access_point,
+      signin_metrics::PromoAction::PROMO_ACTION_NEW_ACCOUNT_NO_EXISTING_ACCOUNT,
+      /*email_hint=*/std::string());
 
   // Receive token.
   EXPECT_FALSE(

chrome/browser/signin/dice_tab_helper.h

@@ -74,6 +74,9 @@ class DiceTabHelper : public content::WebContentsUserData<DiceTabHelper>,
 
   // Initializes the DiceTabHelper for a new signin flow. Must be called once
   // per signin flow happening in the tab, when the signin URL is being loaded.
+  // The `redirect_url` is used after enabling Sync or in case of errors ; it is
+  // not used after a successful signin without sync, and in this case the
+  // page will navigate to the `continue_url` parameter from `signin_url`.
   void InitializeSigninFlow(const GURL& signin_url,
                             signin_metrics::AccessPoint access_point,
                             signin_metrics::Reason reason,

chrome/browser/ui/signin_view_controller.cc

@@ -424,15 +424,16 @@ void SigninViewController::ShowDiceEnableSyncTab(
             .email;
     DCHECK(email_hint.empty() || gaia::AreEmailsSame(email_hint, email_to_use));
   }
-  ShowDiceSigninTab(reason, access_point, promo_action, email_to_use);
+  ShowDiceSigninTab(reason, access_point, promo_action, email_to_use,
+                    GURL(chrome::kChromeUINewTabURL));
 }
 
 void SigninViewController::ShowDiceAddAccountTab(
     signin_metrics::AccessPoint access_point,
     const std::string& email_hint) {
   ShowDiceSigninTab(signin_metrics::Reason::kAddSecondaryAccount, access_point,
                     signin_metrics::PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO,
-                    email_hint);
+                    email_hint, /*redirect_url=*/GURL());
 }
 
 void SigninViewController::ShowGaiaLogoutTab(

chrome/browser/ui/signin_view_controller.h

@@ -194,11 +194,14 @@ class SigninViewController {
 #if BUILDFLAG(ENABLE_DICE_SUPPORT)
   // Shows the DICE-specific sign-in flow: opens a Gaia sign-in webpage in a new
   // tab attached to |browser_|. |email_hint| may be empty.
+  // If `redirect_url` is empty, the Google search URL is used as continue_url.
+  // Internal URLs such as the NTP are only supported when `signin_reason` is
+  // `signin_metrics::Reason::kSigninPrimaryAccount`.
   void ShowDiceSigninTab(signin_metrics::Reason signin_reason,
                          signin_metrics::AccessPoint access_point,
                          signin_metrics::PromoAction promo_action,
                          const std::string& email_hint,
-                         const GURL& redirect_url = GURL::EmptyGURL());
+                         const GURL& redirect_url);
 #endif  // BUILDFLAG(ENABLE_DICE_SUPPORT)
 
   // Returns the web contents of the modal dialog.