[go: nahoru, domu]

Don't pass in the parent window for the parent access code help dialog

When the parent access code feature first launched, there was no parent
passed in, and this regression was introduced in
https://crrev.com/c/2010950.
The parent window for this particular help dialog is small, and it is ok
for the help dialog fall back on the default size for dialogs without a
parent.

Bug: 1128724
Change-Id: I7fc75cb66ea51d4208065bd4e75da72843e25aab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2982647
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Commit-Queue: Courtney Wong <courtneywong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#897559}
diff --git a/ash/child_accounts/parent_access_controller_impl.cc b/ash/child_accounts/parent_access_controller_impl.cc
index 978ac9cf..501a7fa 100644
--- a/ash/child_accounts/parent_access_controller_impl.cc
+++ b/ash/child_accounts/parent_access_controller_impl.cc
@@ -260,18 +260,14 @@
       ParentAccessControllerImpl::UMAAction::kCanceledByUser);
 }
 
-void ParentAccessControllerImpl::OnHelp(gfx::NativeWindow parent_window) {
+void ParentAccessControllerImpl::OnHelp() {
   RecordParentAccessAction(ParentAccessControllerImpl::UMAAction::kGetHelp);
   // TODO(https://crbug.com/999387): Remove this when handling touch
   // cancellation is fixed for system modal windows.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::BindOnce(
-          [](gfx::NativeWindow parent_window) {
-            Shell::Get()->login_screen_controller()->ShowParentAccessHelpApp(
-                parent_window);
-          },
-          parent_window));
+      FROM_HERE, base::BindOnce([]() {
+        Shell::Get()->login_screen_controller()->ShowParentAccessHelpApp();
+      }));
 }
 
 bool ParentAccessControllerImpl::ShowWidget(
diff --git a/ash/child_accounts/parent_access_controller_impl.h b/ash/child_accounts/parent_access_controller_impl.h
index 381bb024..3d1dad7 100644
--- a/ash/child_accounts/parent_access_controller_impl.h
+++ b/ash/child_accounts/parent_access_controller_impl.h
@@ -82,7 +82,7 @@
   PinRequestView::SubmissionResult OnPinSubmitted(
       const std::string& pin) override;
   void OnBack() override;
-  void OnHelp(gfx::NativeWindow parent_window) override;
+  void OnHelp() override;
 
   // ParentAccessController:
   bool ShowWidget(const AccountId& child_account_id,
diff --git a/ash/child_accounts/parent_access_controller_impl_unittest.cc b/ash/child_accounts/parent_access_controller_impl_unittest.cc
index 5a92733..3ff0879 100644
--- a/ash/child_accounts/parent_access_controller_impl_unittest.cc
+++ b/ash/child_accounts/parent_access_controller_impl_unittest.cc
@@ -281,7 +281,7 @@
       ParentAccessControllerImpl::UMAValidationResult::kInvalid,
       SupervisedAction::kUnlockTimeLimits, 1, 1);
 
-  EXPECT_CALL(*login_client_, ShowParentAccessHelpApp(_)).Times(1);
+  EXPECT_CALL(*login_client_, ShowParentAccessHelpApp()).Times(1);
   SimulateButtonPress(PinRequestView::TestApi(view_).help_button());
   ExpectUMAActionReported(ParentAccessControllerImpl::UMAAction::kGetHelp, 1,
                           2);
diff --git a/ash/login/login_screen_controller.cc b/ash/login/login_screen_controller.cc
index dd35ffd..671ec86 100644
--- a/ash/login/login_screen_controller.cc
+++ b/ash/login/login_screen_controller.cc
@@ -436,9 +436,8 @@
   client_->ShowAccountAccessHelpApp(parent_window);
 }
 
-void LoginScreenController::ShowParentAccessHelpApp(
-    gfx::NativeWindow parent_window) {
-  client_->ShowParentAccessHelpApp(parent_window);
+void LoginScreenController::ShowParentAccessHelpApp() {
+  client_->ShowParentAccessHelpApp();
 }
 
 void LoginScreenController::ShowLockScreenNotificationSettings() {
diff --git a/ash/login/login_screen_controller.h b/ash/login/login_screen_controller.h
index 51a2bb1..4a66273a 100644
--- a/ash/login/login_screen_controller.h
+++ b/ash/login/login_screen_controller.h
@@ -94,7 +94,7 @@
                                            const std::string& locale);
   void HandleAccelerator(ash::LoginAcceleratorAction action);
   void ShowAccountAccessHelpApp(gfx::NativeWindow parent_window);
-  void ShowParentAccessHelpApp(gfx::NativeWindow parent_window);
+  void ShowParentAccessHelpApp();
   void ShowLockScreenNotificationSettings();
   void FocusOobeDialog();
   void NotifyUserActivity();
diff --git a/ash/login/mock_login_screen_client.h b/ash/login/mock_login_screen_client.h
index 8efe30a..eb9593e 100644
--- a/ash/login/mock_login_screen_client.h
+++ b/ash/login/mock_login_screen_client.h
@@ -105,7 +105,7 @@
               (ash::LoginAcceleratorAction action),
               (override));
   MOCK_METHOD(void, ShowAccountAccessHelpApp, (gfx::NativeWindow), (override));
-  MOCK_METHOD(void, ShowParentAccessHelpApp, (gfx::NativeWindow), (override));
+  MOCK_METHOD(void, ShowParentAccessHelpApp, (), (override));
   MOCK_METHOD(void, ShowLockScreenNotificationSettings, (), (override));
   MOCK_METHOD(void, FocusOobeDialog, (), (override));
   MOCK_METHOD(void, OnFocusLeavingSystemTray, (bool reverse), (override));
diff --git a/ash/login/security_token_request_controller.cc b/ash/login/security_token_request_controller.cc
index 693b80321..b3ba478 100644
--- a/ash/login/security_token_request_controller.cc
+++ b/ash/login/security_token_request_controller.cc
@@ -63,7 +63,7 @@
   ClosePinUi();
 }
 
-void SecurityTokenRequestController::OnHelp(gfx::NativeWindow parent_window) {
+void SecurityTokenRequestController::OnHelp() {
   NOTREACHED();
 }
 
diff --git a/ash/login/security_token_request_controller.h b/ash/login/security_token_request_controller.h
index f01125a..1050d2d 100644
--- a/ash/login/security_token_request_controller.h
+++ b/ash/login/security_token_request_controller.h
@@ -30,7 +30,7 @@
   PinRequestView::SubmissionResult OnPinSubmitted(
       const std::string& pin) override;
   void OnBack() override;
-  void OnHelp(gfx::NativeWindow parent_window) override;
+  void OnHelp() override;
 
   // Shows the PIN dialog configured by |request|. If there already is a
   // SecurityTokenPinRequest in progress, keeps the dialog open and updates the
diff --git a/ash/login/ui/pin_request_view.cc b/ash/login/ui/pin_request_view.cc
index 57a77b50..1495f38 100644
--- a/ash/login/ui/pin_request_view.cc
+++ b/ash/login/ui/pin_request_view.cc
@@ -356,10 +356,7 @@
 
   help_button_ = new FocusableLabelButton(
       base::BindRepeating(
-          [](PinRequestView* view) {
-            view->delegate_->OnHelp(view->GetWidget()->GetNativeWindow());
-          },
-          this),
+          [](PinRequestView* view) { view->delegate_->OnHelp(); }, this),
       l10n_util::GetStringUTF16(IDS_ASH_LOGIN_PIN_REQUEST_HELP));
   help_button_->SetPaintToLayer();
   help_button_->layer()->SetFillsBoundsOpaquely(false);
diff --git a/ash/login/ui/pin_request_view.h b/ash/login/ui/pin_request_view.h
index 43dc2fc..5336615 100644
--- a/ash/login/ui/pin_request_view.h
+++ b/ash/login/ui/pin_request_view.h
@@ -87,7 +87,7 @@
    public:
     virtual SubmissionResult OnPinSubmitted(const std::string& pin) = 0;
     virtual void OnBack() = 0;
-    virtual void OnHelp(gfx::NativeWindow parent_window) = 0;
+    virtual void OnHelp() = 0;
 
    protected:
     virtual ~Delegate() = default;
diff --git a/ash/login/ui/pin_request_view_unittest.cc b/ash/login/ui/pin_request_view_unittest.cc
index c8c0a73..8b37cb4 100644
--- a/ash/login/ui/pin_request_view_unittest.cc
+++ b/ash/login/ui/pin_request_view_unittest.cc
@@ -82,9 +82,7 @@
 
   void OnBack() override { ++back_action_; }
 
-  void OnHelp(gfx::NativeWindow parent_window) override {
-    ++help_dialog_opened_;
-  }
+  void OnHelp() override { ++help_dialog_opened_; }
 
   void StartView(absl::optional<int> pin_length = 6) {
     PinRequest request;
diff --git a/ash/public/cpp/login_screen_client.h b/ash/public/cpp/login_screen_client.h
index 20c22f6..d19f42a 100644
--- a/ash/public/cpp/login_screen_client.h
+++ b/ash/public/cpp/login_screen_client.h
@@ -146,7 +146,7 @@
   virtual void ShowAccountAccessHelpApp(gfx::NativeWindow parent_window) = 0;
 
   // Shows help app for users that have trouble using parent access code.
-  virtual void ShowParentAccessHelpApp(gfx::NativeWindow parent_window) = 0;
+  virtual void ShowParentAccessHelpApp() = 0;
 
   // Show the lockscreen notification settings page.
   virtual void ShowLockScreenNotificationSettings() = 0;
diff --git a/chrome/browser/ui/ash/login_screen_client_impl.cc b/chrome/browser/ui/ash/login_screen_client_impl.cc
index 5fea754..d80ca09 100644
--- a/chrome/browser/ui/ash/login_screen_client_impl.cc
+++ b/chrome/browser/ui/ash/login_screen_client_impl.cc
@@ -246,10 +246,11 @@
       ->ShowHelpTopic(chromeos::HelpAppLauncher::HELP_CANT_ACCESS_ACCOUNT);
 }
 
-void LoginScreenClientImpl::ShowParentAccessHelpApp(
-    gfx::NativeWindow parent_window) {
+void LoginScreenClientImpl::ShowParentAccessHelpApp() {
+  // Don't pass in a parent window so that the size of the help dialog is not
+  // bounded by its parent window.
   scoped_refptr<chromeos::HelpAppLauncher>(
-      new chromeos::HelpAppLauncher(parent_window))
+      new chromeos::HelpAppLauncher(/*parent_window=*/nullptr))
       ->ShowHelpTopic(chromeos::HelpAppLauncher::HELP_PARENT_ACCESS_CODE);
 }
 
diff --git a/chrome/browser/ui/ash/login_screen_client_impl.h b/chrome/browser/ui/ash/login_screen_client_impl.h
index adf5048..95c109a 100644
--- a/chrome/browser/ui/ash/login_screen_client_impl.h
+++ b/chrome/browser/ui/ash/login_screen_client_impl.h
@@ -120,7 +120,7 @@
                                            const std::string& locale) override;
   void HandleAccelerator(ash::LoginAcceleratorAction action) override;
   void ShowAccountAccessHelpApp(gfx::NativeWindow parent_window) override;
-  void ShowParentAccessHelpApp(gfx::NativeWindow parent_window) override;
+  void ShowParentAccessHelpApp() override;
   void ShowLockScreenNotificationSettings() override;
   void OnFocusLeavingSystemTray(bool reverse) override;
   void OnSystemTrayBubbleShown() override;