[Passwords Tutorial] Switch to direct-link to Password Settings page

Merge to release branch for https://chromium-review.googlesource.com/c/chromium/src/+/4632060 This CL has a number of minor structural changes to support determining if a Tutorial is running, and then using that functionality to jump straight to the Password Manager Settings page when running the new Password Manager Shortcut tutorial. This bypasses issues around different screen sizes causing the Password Manager page to render differently, potentially breaking tutorial flows. (cherry picked from commit ba5c121) Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4632060 Commit-Queue: Dana Fried <dfried@chromium.org> Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com> Reviewed-by: John Lee <johntlee@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1161987} Change-Id: Ic8964ae0d8401fb373725beaf66e9e51dd93968c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4666776 Commit-Queue: Mickey Burks <mickeyburks@chromium.org> Reviewed-by: Dana Fried <dfried@chromium.org> Cr-Commit-Position: refs/branch-heads/5845@{#336} Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}
chromium · Jul 6, 2023 · d21ee81 · d21ee81
1 parent dcd33c9
commit d21ee81
Show file tree

Hide file tree

Showing 12 changed files with 84 additions and 100 deletions.
diff --git a/chrome/browser/resources/password_manager/settings_section.html b/chrome/browser/resources/password_manager/settings_section.html
@@ -106,7 +106,8 @@ <h2 class="page-title">$i18n{settings}</h2>
   <template is="dom-if" if="[[hasPasswordsToExport_]]" restamp>
     <passwords-exporter></passwords-exporter>
   </template>
-  <template is="dom-if" if="[[canAddShortcut_]]">
+  <template is="dom-if" if="[[canAddShortcut_]]"
+      on-dom-change="onShortcutBannerDomChanged_" restamp>
     <cr-link-row id="addShortcutBanner" class="cr-row settings-cr-link-row"
         on-click="onAddShortcutClick_" label="$i18n{addShortcut}"
         sub-label="$i18n{addShortcutDescription}" role-description="button">

diff --git a/chrome/browser/resources/password_manager/settings_section.ts b/chrome/browser/resources/password_manager/settings_section.ts
@@ -180,7 +180,9 @@ export class SettingsSectionElement extends SettingsSectionElementBase {
       const params = new URLSearchParams();
       Router.getInstance().updateRouterParams(params);
     }
+  }
 
+  private onShortcutBannerDomChanged_() {
     const addShortcutBanner = this.root!.querySelector('#addShortcutBanner');
     if (addShortcutBanner) {
       this.registerHelpBubble(

diff --git a/chrome/browser/ui/chrome_pages.cc b/chrome/browser/ui/chrome_pages.cc
@@ -439,8 +439,9 @@ void ShowPasswordManager(Browser* browser) {
         UserEducationServiceFactory::GetForProfile(browser->profile());
     if (service) {
       auto* tutorial_service = &service->tutorial_service();
-      if (tutorial_service && tutorial_service->IsRunningTutorial()) {
-        ShowSingletonTab(browser, GURL(kChromeUIPasswordManagerURL));
+      if (tutorial_service &&
+          tutorial_service->IsRunningTutorial(kPasswordManagerTutorialId)) {
+        ShowSingletonTab(browser, GURL(kChromeUIPasswordManagerSettingsURL));
         return;
       }
     }

diff --git a/chrome/browser/ui/user_education/user_education_service.cc b/chrome/browser/ui/user_education/user_education_service.cc
@@ -10,6 +10,13 @@
 #include "components/user_education/common/help_bubble_factory_registry.h"
 #include "components/user_education/common/tutorial_registry.h"
 
+const char kSidePanelCustomizeChromeTutorialId[] =
+    "Side Panel Customize Chrome Tutorial";
+const char kTabGroupTutorialId[] = "Tab Group Tutorial";
+const char kTabGroupWithExistingGroupTutorialId[] =
+    "Tab Group With Existing Group Tutorial";
+const char kPasswordManagerTutorialId[] = "Password Manager Tutorial";
+
 UserEducationService::UserEducationService()
     : tutorial_service_(&tutorial_registry_, &help_bubble_factory_registry_) {}
 

diff --git a/chrome/browser/ui/user_education/user_education_service.h b/chrome/browser/ui/user_education/user_education_service.h
@@ -12,6 +12,11 @@
 #include "components/user_education/common/tutorial.h"
 #include "components/user_education/common/tutorial_registry.h"
 
+extern const char kTabGroupTutorialId[];
+extern const char kTabGroupWithExistingGroupTutorialId[];
+extern const char kSidePanelCustomizeChromeTutorialId[];
+extern const char kPasswordManagerTutorialId[];
+
 class UserEducationService : public KeyedService {
  public:
   UserEducationService();

diff --git a/chrome/browser/ui/views/user_education/browser_user_education_service.cc b/chrome/browser/ui/views/user_education/browser_user_education_service.cc
@@ -77,8 +77,6 @@ const char kSidePanelReadingListTutorialMetricPrefix[] = "SidePanelReadingList";
 const char kCustomizeChromeTutorialMetricPrefix[] = "CustomizeChromeSidePanel";
 const char kSideSearchTutorialMetricPrefix[] = "SideSearch";
 const char kPasswordManagerTutorialMetricPrefix[] = "PasswordManager";
-const char kPasswordManagerSmallViewportTutorialMetricPrefix[] =
-    "PasswordManagerSmallViewport";
 constexpr char kTabGroupHeaderElementName[] = "TabGroupHeader";
 constexpr char kReadingListItemElementName[] = "ReadingListItem";
 constexpr char kChromeThemeBackElementName[] = "ChromeThemeBackElement";
@@ -183,17 +181,9 @@ DEFINE_FRAMEWORK_SPECIFIC_METADATA(FloatingWebUIHelpBubbleFactoryBrowser)
 
 }  // namespace
 
-const char kSidePanelCustomizeChromeTutorialId[] =
-    "Side Panel Customize Chrome Tutorial";
-const char kTabGroupTutorialId[] = "Tab Group Tutorial";
-const char kTabGroupWithExistingGroupTutorialId[] =
-    "Tab Group With Existing Group Tutorial";
 const char kSidePanelReadingListTutorialId[] =
     "Side Panel Reading List Tutorial";
 const char kSideSearchTutorialId[] = "Side Search Tutorial";
-const char kPasswordManagerTutorialId[] = "Password Manager Tutorial";
-const char kPasswordManagerSmallViewportTutorialId[] =
-    "Password Manager Small Viewport Tutorial";
 
 user_education::HelpBubbleDelegate* GetHelpBubbleDelegate() {
   static base::NoDestructor<BrowserHelpBubbleDelegate> delegate;
@@ -732,84 +722,49 @@ void MaybeRegisterChromeTutorials(
                                   std::move(side_search_tutorial));
   }
 
-  {  // Password Manager tutorial
-    auto before_branch_steps = TutorialDescription::Steps(
-        // Bubble step - Browser app menu
-        TutorialDescription::BubbleStep(kAppMenuButtonElementId)
-            .SetBubbleBodyText(IDS_TUTORIAL_PASSWORD_MANAGER_OPEN_APP_MENU)
-            .SetBubbleArrow(HelpBubbleArrow::kTopRight),
+  // Password Manager tutorial
+  tutorial_registry.AddTutorial(
+      kPasswordManagerTutorialId,
+      TutorialDescription::Create<kPasswordManagerTutorialMetricPrefix>(
+          // Bubble step - Browser app menu
+          TutorialDescription::BubbleStep(kAppMenuButtonElementId)
+              .SetBubbleBodyText(IDS_TUTORIAL_PASSWORD_MANAGER_OPEN_APP_MENU)
+              .SetBubbleArrow(HelpBubbleArrow::kTopRight),
 
-        // Bubble step - "Password Manager" menu item
-        TutorialDescription::BubbleStep(AppMenuModel::kPasswordManagerMenuItem)
-            .SetBubbleBodyText(
-                IDS_TUTORIAL_PASSWORD_MANAGER_CLICK_PASSWORD_MANAGER)
-            .SetBubbleArrow(HelpBubbleArrow::kRightCenter)
-            .AbortIfVisibilityLost(false));
-
-    // Bubble step - Optional "Overflow menu"
-    auto small_viewport_overflow_step =
-        TutorialDescription::BubbleStep(
-            PasswordManagerUI::kOverflowMenuElementId)
-            .SetBubbleBodyText(IDS_TUTORIAL_PASSWORD_MANAGER_OPEN_MENU)
-            .SetBubbleArrow(HelpBubbleArrow::kTopLeft)
-            .InAnyContext();
-
-    // Bubble step - "Settings" menu item
-    auto small_viewport_settings_step =
-        TutorialDescription::BubbleStep(
-            PasswordManagerUI::kSettingsMenuItemElementId)
-            .SetBubbleBodyText(IDS_TUTORIAL_PASSWORD_MANAGER_SELECT_SETTINGS)
-            .SetBubbleArrow(HelpBubbleArrow::kTopCenter)
-            .InAnyContext();
-
-    // Bubble step - "Settings" menu item
-    auto settings_step =
-        TutorialDescription::BubbleStep(
-            PasswordManagerUI::kSettingsMenuItemElementId)
-            .SetBubbleBodyText(IDS_TUTORIAL_PASSWORD_MANAGER_SELECT_SETTINGS)
-            .SetBubbleArrow(HelpBubbleArrow::kLeftCenter)
-            .InAnyContext();
-
-    auto after_branch_steps = TutorialDescription::Steps(
-        // Bubble step - "Add shortcut" row
-        TutorialDescription::BubbleStep(
-            PasswordManagerUI::kAddShortcutElementId)
-            .SetBubbleBodyText(IDS_TUTORIAL_PASSWORD_MANAGER_ADD_SHORTCUT)
-            .SetBubbleArrow(HelpBubbleArrow::kTopCenter)
-            .InAnyContext(),
-
-        // Event step - Click on "Add shortcut"
-        TutorialDescription::EventStep(
-            PasswordManagerUI::kAddShortcutCustomEventId)
-            .InSameContext(),
-
-        // Bubble step - "Install" row
-        TutorialDescription::BubbleStep(
-            PWAConfirmationBubbleView::kInstallButton)
-            .SetBubbleBodyText(IDS_TUTORIAL_PASSWORD_MANAGER_CLICK_INSTALL)
-            .SetBubbleArrow(HelpBubbleArrow::kTopRight),
+          // Bubble step - "Password Manager" menu item
+          TutorialDescription::BubbleStep(
+              AppMenuModel::kPasswordManagerMenuItem)
+              .SetBubbleBodyText(
+                  IDS_TUTORIAL_PASSWORD_MANAGER_CLICK_PASSWORD_MANAGER)
+              .SetBubbleArrow(HelpBubbleArrow::kRightCenter)
+              .AbortIfVisibilityLost(false),
 
-        // Event step - Click on "Add shortcut"
-        TutorialDescription::EventStep(
-            PWAConfirmationBubbleView::kInstalledPWAEventId)
-            .InSameContext(),
+          // Bubble step - "Add shortcut" row
+          TutorialDescription::BubbleStep(
+              PasswordManagerUI::kAddShortcutElementId)
+              .SetBubbleBodyText(IDS_TUTORIAL_PASSWORD_MANAGER_ADD_SHORTCUT)
+              .SetBubbleArrow(HelpBubbleArrow::kTopCenter)
+              .InAnyContext(),
 
-        // Completion of the tutorial.
-        TutorialDescription::BubbleStep(kTopContainerElementId)
-            .SetBubbleTitleText(IDS_TUTORIAL_GENERIC_SUCCESS_TITLE)
-            .SetBubbleBodyText(IDS_TUTORIAL_PASSWORD_MANAGER_SUCCESS_BODY)
-            .SetBubbleArrow(HelpBubbleArrow::kNone));
+          // Event step - Click on "Add shortcut"
+          TutorialDescription::EventStep(
+              PasswordManagerUI::kAddShortcutCustomEventId)
+              .InSameContext(),
 
-    tutorial_registry.AddTutorial(
-        kPasswordManagerTutorialId,
-        TutorialDescription::Create<kPasswordManagerTutorialMetricPrefix>(
-            before_branch_steps, settings_step, after_branch_steps));
+          // Bubble step - "Install" row
+          TutorialDescription::BubbleStep(
+              PWAConfirmationBubbleView::kInstallButton)
+              .SetBubbleBodyText(IDS_TUTORIAL_PASSWORD_MANAGER_CLICK_INSTALL)
+              .SetBubbleArrow(HelpBubbleArrow::kTopRight),
 
-    tutorial_registry.AddTutorial(
-        kPasswordManagerSmallViewportTutorialId,
-        TutorialDescription::Create<
-            kPasswordManagerSmallViewportTutorialMetricPrefix>(
-            before_branch_steps, small_viewport_overflow_step,
-            small_viewport_settings_step, after_branch_steps));
-  }
+          // Event step - Click on "Add shortcut"
+          TutorialDescription::EventStep(
+              PWAConfirmationBubbleView::kInstalledPWAEventId)
+              .InSameContext(),
+
+          // Completion of the tutorial.
+          TutorialDescription::BubbleStep(kTopContainerElementId)
+              .SetBubbleTitleText(IDS_TUTORIAL_GENERIC_SUCCESS_TITLE)
+              .SetBubbleBodyText(IDS_TUTORIAL_PASSWORD_MANAGER_SUCCESS_BODY)
+              .SetBubbleArrow(HelpBubbleArrow::kNone)));
 }
diff --git a/chrome/browser/ui/views/user_education/browser_user_education_service.h b/chrome/browser/ui/views/user_education/browser_user_education_service.h
@@ -14,11 +14,6 @@ class TutorialRegistry;
 class HelpBubbleDelegate;
 }  // namespace user_education
 
-extern const char kTabGroupTutorialId[];
-extern const char kTabGroupWithExistingGroupTutorialId[];
-extern const char kSidePanelCustomizeChromeTutorialId[];
-extern const char kPasswordManagerTutorialId[];
-
 extern user_education::HelpBubbleDelegate* GetHelpBubbleDelegate();
 extern void RegisterChromeHelpBubbleFactories(
     user_education::HelpBubbleFactoryRegistry& registry);

diff --git a/chrome/browser/ui/webui/browser_command/browser_command_handler_unittest.cc b/chrome/browser/ui/webui/browser_command/browser_command_handler_unittest.cc
@@ -170,13 +170,16 @@ class TestTutorialService : public user_education::TutorialService {
       ui::ElementContext context,
       base::OnceClosure completed_callback = base::DoNothing(),
       base::OnceClosure aborted_callback = base::DoNothing()) override {
-    running_ = true;
+    running_id_ = id;
   }
 
-  bool IsRunningTutorial() const override { return running_; }
+  bool IsRunningTutorial(
+      absl::optional<user_education::TutorialIdentifier> id) const override {
+    return id.has_value() ? id == running_id_ : running_id_.has_value();
+  }
 
  private:
-  bool running_ = false;
+  absl::optional<user_education::TutorialIdentifier> running_id_;
 };
 
 class MockTutorialService : public TestTutorialService {
@@ -196,7 +199,8 @@ class MockTutorialService : public TestTutorialService {
   MOCK_METHOD(void,
               LogStartedFromWhatsNewPage,
               (user_education::TutorialIdentifier, bool));
-  MOCK_CONST_METHOD0(IsRunningTutorial, bool());
+  MOCK_CONST_METHOD1(IsRunningTutorial,
+                     bool(absl::optional<user_education::TutorialIdentifier>));
 };
 
 class MockCommandHandler : public TestCommandHandler {

diff --git a/chrome/common/webui_url_constants.cc b/chrome/common/webui_url_constants.cc
@@ -157,6 +157,8 @@ const char kChromeUIPasswordManagerInternalsHost[] =
 const char kChromeUIPasswordManagerURL[] = "chrome://password-manager";
 const char kChromeUIPasswordManagerCheckupURL[] =
     "chrome://password-manager/checkup?start=true";
+const char kChromeUIPasswordManagerSettingsURL[] =
+    "chrome://password-manager/settings";
 const char kChromeUIPerformanceSettingsURL[] = "chrome://settings/performance";
 const char kChromeUIPolicyHost[] = "policy";
 const char kChromeUIPolicyURL[] = "chrome://policy/";

diff --git a/chrome/common/webui_url_constants.h b/chrome/common/webui_url_constants.h
@@ -159,6 +159,7 @@ extern const char kChromeUIOsUrlAppURL[];
 extern const char kChromeUIPasswordManagerInternalsHost[];
 extern const char kChromeUIPasswordManagerURL[];
 extern const char kChromeUIPasswordManagerCheckupURL[];
+extern const char kChromeUIPasswordManagerSettingsURL[];
 extern const char kChromeUIPerformanceSettingsURL[];
 extern const char kChromeUIPolicyHost[];
 extern const char kChromeUIPolicyURL[];

diff --git a/components/user_education/common/tutorial_service.cc b/components/user_education/common/tutorial_service.cc
@@ -85,6 +85,7 @@ void TutorialService::StartTutorial(TutorialIdentifier id,
                      base::Unretained(this)));
 
   // Start the tutorial and mark the params used to created it for restarting.
+  most_recent_tutorial_id_ = id;
   running_tutorial_->Start();
 }
 
@@ -226,8 +227,12 @@ void TutorialService::HideCurrentBubbleIfShowing() {
   currently_displayed_bubble_.reset();
 }
 
-bool TutorialService::IsRunningTutorial() const {
-  return running_tutorial_ != nullptr;
+bool TutorialService::IsRunningTutorial(
+    absl::optional<TutorialIdentifier> id) const {
+  if (!running_tutorial_) {
+    return false;
+  }
+  return !id.has_value() || id.value() == most_recent_tutorial_id_;
 }
 
 void TutorialService::ResetRunningTutorial() {

diff --git a/components/user_education/common/tutorial_service.h b/components/user_education/common/tutorial_service.h
@@ -40,7 +40,10 @@ class TutorialService {
   using AbortedCallback = base::OnceClosure;
 
   // Returns true if there is a currently running tutorial.
-  virtual bool IsRunningTutorial() const;
+  // If `id` is specified, specifically returns whether *that* tutorial is
+  // running.
+  virtual bool IsRunningTutorial(
+      absl::optional<TutorialIdentifier> id = absl::nullopt) const;
 
   // Sets the current help bubble stored by the service.
   void SetCurrentBubble(std::unique_ptr<HelpBubble> bubble, bool is_last_step);
@@ -120,6 +123,9 @@ class TutorialService {
   // be stored in the service.
   std::unique_ptr<Tutorial> running_tutorial_;
 
+  // Set to the ID of the current or most recent tutorial to run.
+  TutorialIdentifier most_recent_tutorial_id_;
+
   // Was restarted denotes that the current running tutorial was restarted,
   // and when logging that the tutorial aborts, instead should log as completed.
   bool running_tutorial_was_restarted_ = false;