[Profile creation] Add {back,reload} accelerators for navigation

This CL adds two accelerators into the profile creation flow to improve
accessibility and usability:
 - the back action for screens that actually show the back button
 - the reload action for the sign-in flow that happens over the internet
   and thus loading of the page can fail.

Bug: 1126913, 1167675
Change-Id: I3c197dea2859c0136db62249064249e0569a93da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2627403
Auto-Submit: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844695}

chrome/browser/ui/views/profiles/profile_picker_interactive_uitest.cc

@@ -74,6 +74,24 @@ class ProfilePickerInteractiveUiTest : public ProfilePickerTestBase {
         widget()->GetNativeWindow(), ui::VKEY_W, control, shift, /*alt=*/false,
         command));
   }
+
+  void SendBackKeyboardCommand() {
+    // Close window using keyboard.
+#if defined(OS_MAC)
+    // Use Cmd-[ on Mac.
+    bool alt = false;
+    bool command = true;
+    ui::KeyboardCode key = ui::VKEY_OEM_4;
+#else
+    // Use Ctrl-left on other platforms.
+    bool alt = true;
+    bool command = false;
+    ui::KeyboardCode key = ui::VKEY_LEFT;
+#endif
+    ASSERT_TRUE(ui_test_utils::SendKeyPressToWindowSync(
+        widget()->GetNativeWindow(), key, /*control=*/false,
+        /*shift=*/false, alt, command));
+  }
 };
 
 // Checks that the main picker view can be closed with keyboard shortcut.
@@ -165,3 +183,44 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerInteractiveUiTest,
   SendCloseWindowKeyboardCommand();
   WaitForPickerClosed();
 }
+
+// Checks that both the signin web view and the main picker view are able to
+// process a back keyboard event.
+IN_PROC_BROWSER_TEST_F(ProfilePickerInteractiveUiTest,
+                       NavigateBackWithKeyboard) {
+  // Simulate walking through the flow starting at the picker so that navigating
+  // back to the picker makes sense.
+  ProfilePicker::Show(ProfilePicker::EntryPoint::kProfileMenuManageProfiles);
+  WaitForLayoutWithoutToolbar();
+  WaitForFirstPaint(web_contents(), GURL("chrome://profile-picker"));
+  web_contents()->GetController().LoadURL(
+      GURL("chrome://profile-picker/new-profile"), content::Referrer(),
+      ui::PAGE_TRANSITION_AUTO_TOPLEVEL, std::string());
+  WaitForFirstPaint(web_contents(),
+                    GURL("chrome://profile-picker/new-profile"));
+
+  // Simulate a click on the signin button.
+  base::MockCallback<base::OnceCallback<void(bool)>> switch_finished_callback;
+  EXPECT_CALL(switch_finished_callback, Run(true));
+  ProfilePicker::SwitchToSignIn(SK_ColorRED, switch_finished_callback.Get());
+
+  // Switch to the signin webview.
+  WaitForLayoutWithToolbar();
+  WaitForFirstPaint(web_contents(),
+                    GaiaUrls::GetInstance()->signin_chrome_sync_dice());
+
+  // Navigate back with the keyboard.
+  SendBackKeyboardCommand();
+
+  WaitForLayoutWithoutToolbar();
+  WaitForFirstPaint(web_contents(),
+                    GURL("chrome://profile-picker/new-profile"));
+
+  // Navigate again back with the keyboard.
+  SendBackKeyboardCommand();
+  WaitForFirstPaint(web_contents(), GURL("chrome://profile-picker"));
+
+  // Navigating back once again does nothing.
+  SendBackKeyboardCommand();
+  EXPECT_EQ(web_contents()->GetController().GetPendingEntry(), nullptr);
+}

chrome/browser/ui/views/profiles/profile_picker_view.cc

@@ -100,7 +100,7 @@ constexpr base::TimeDelta kExtendedAccountInfoTimeout =
 
 constexpr int kSupportedAcceleratorCommands[] = {
     IDC_CLOSE_TAB,       IDC_CLOSE_WINDOW, IDC_EXIT,  IDC_FULLSCREEN,
-    IDC_MINIMIZE_WINDOW};
+    IDC_MINIMIZE_WINDOW, IDC_BACK,         IDC_RELOAD};
 
 void ShowCustomizationBubble(SkColor new_profile_color, Browser* browser) {
   views::View* anchor_view = BrowserView::GetBrowserViewForBrowser(browser)
@@ -543,7 +543,8 @@ void ProfilePickerView::SwitchToSyncConfirmation() {
   // the signin screen is no concern any more.
   ShowScreen(new_profile_contents_.get(),
              GURL(chrome::kChromeUISyncConfirmationURL),
-             /*show_toolbar=*/false);
+             /*show_toolbar=*/false,
+             /*enable_navigating_back=*/false);
 
   SyncConfirmationUI* sync_confirmation_ui = static_cast<SyncConfirmationUI*>(
       new_profile_contents_->GetWebUI()->GetController());
@@ -614,6 +615,21 @@ bool ProfilePickerView::AcceleratorPressed(const ui::Accelerator& accelerator) {
     case IDC_MINIMIZE_WINDOW:
       GetWidget()->Minimize();
       break;
+    case IDC_BACK: {
+      NavigateBack();
+      break;
+    }
+    // Always reload bypassing cache.
+    case IDC_RELOAD:
+    case IDC_RELOAD_BYPASSING_CACHE:
+    case IDC_RELOAD_CLEARING_CACHE: {
+      // Sign-in may fail due to connectivity issues, allow reloading.
+      if (IsSigningIn()) {
+        new_profile_contents_->GetController().Reload(
+            content::ReloadType::BYPASSING_CACHE, true);
+      }
+      break;
+    }
     default:
       NOTREACHED() << "Unexpected command_id: " << command_id;
       break;
@@ -707,7 +723,8 @@ void ProfilePickerView::UpdateToolbarColor() {
 
 void ProfilePickerView::ShowScreen(content::WebContents* contents,
                                    const GURL& url,
-                                   bool show_toolbar) {
+                                   bool show_toolbar,
+                                   bool enable_navigating_back) {
   web_view_->SetWebContents(contents);
   web_view_->RequestFocus();
 
@@ -715,6 +732,8 @@ void ProfilePickerView::ShowScreen(content::WebContents* contents,
   // it easier for tests to detect changing of the screen.
   toolbar_->SetVisible(show_toolbar);
 
+  enable_navigating_back_ = enable_navigating_back;
+
   if (!url.is_empty()) {
     // Make sure to load the url as the last step so that the UI state is
     // coherent upon the NavigationStateChanged notification.
@@ -725,18 +744,22 @@ void ProfilePickerView::ShowScreen(content::WebContents* contents,
 }
 
 void ProfilePickerView::BackButtonPressed(const ui::Event& event) {
-  if (!IsSigningIn()) {
+  NavigateBack();
+}
+
+void ProfilePickerView::NavigateBack() {
+  if (!enable_navigating_back_)
     return;
-  }
 
-  if (new_profile_contents_->GetController().CanGoBack()) {
-    new_profile_contents_->GetController().GoBack();
+  if (web_view_->GetWebContents()->GetController().CanGoBack()) {
+    web_view_->GetWebContents()->GetController().GoBack();
     return;
   }
 
   // Move from sign-in back to the previous screen of profile creation.
   // Do not load any url because the desired screen is still loaded in
   // `system_profile_contents_`.
+  if (IsSigningIn())
     ShowScreen(system_profile_contents_.get(), GURL(), /*show_toolbar=*/false);
 }
 
@@ -759,7 +782,7 @@ void ProfilePickerView::OnRefreshTokenUpdatedForAccount(
   // in some cases (such as managed signed-in), there are further delays before
   // any follow-up UI is shown.
   ShowScreen(new_profile_contents_.get(), GURL(url::kAboutBlankURL),
-             /*show_toolbar=*/true);
+             /*show_toolbar=*/true, /*enable_navigating_back=*/false);
 
   // Set up a timeout for extended account info (which cancels any existing
   // timeout closure).
@@ -851,7 +874,7 @@ void ProfilePickerView::FinishSignedInCreationFlowForSAML() {
   // Free up `new_profile_contents_` to be moved to a new browser window.
   new_profile_contents_->SetDelegate(nullptr);
   ShowScreen(system_profile_contents_.get(), GURL(url::kAboutBlankURL),
-             /*show_toolbar=*/false);
+             /*show_toolbar=*/false, /*enable_navigating_back=*/false);
   FinishSignedInCreationFlowImpl(
       base::BindOnce(&ContinueSAMLSignin, std::move(new_profile_contents_)),
       /*enterprise_sync_consent_needed=*/true);

chrome/browser/ui/views/profiles/profile_picker_view.h

@@ -117,9 +117,11 @@ class ProfilePickerView : public views::WidgetDelegateView,
   // `url` is empty, it only shows `contents` with its currently loaded url.
   void ShowScreen(content::WebContents* contents,
                   const GURL& url,
-                  bool show_toolbar);
+                  bool show_toolbar,
+                  bool enable_navigating_back = true);
 
   void BackButtonPressed(const ui::Event& event);
+  void NavigateBack();
 
   // Checks whether the sign-in flow is in progress.
   bool IsSigningIn() const;
@@ -173,6 +175,7 @@ class ProfilePickerView : public views::WidgetDelegateView,
 
   // A mapping between accelerators and command IDs.
   std::map<ui::Accelerator, int> accelerator_table_;
+  bool enable_navigating_back_ = true;
 
   // Handler for unhandled key events from renderer.
   views::UnhandledKeyboardEventHandler unhandled_keyboard_event_handler_;