[Oobe WebUI] Remove EnableAdbSideloadingScreen::OnViewDestroyed

Instead store the pointer to the EnableAdbSideloadingScreenView as a base::WeakPtr. Bug: 1312869, 1309022, 1298392 Change-Id: Ic1fea2a73b769b5d18be9e02d72c57a86ab86167 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3567082 Commit-Queue: Roman Sorokin <rsorokin@chromium.org> Auto-Submit: Roman Sorokin <rsorokin@chromium.org> Reviewed-by: Danila Kuzmin <dkuzmin@google.com> Commit-Queue: Danila Kuzmin <dkuzmin@google.com> Cr-Commit-Position: refs/heads/main@{#988440}
chromium · Apr 4, 2022 · c8822fa · c8822fa
1 parent 0eec9b0
commit c8822fa
Show file tree

Hide file tree

Showing 8 changed files with 26 additions and 36 deletions.
diff --git a/chrome/browser/ash/login/screens/enable_adb_sideloading_screen.cc b/chrome/browser/ash/login/screens/enable_adb_sideloading_screen.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/ash/login/screens/enable_adb_sideloading_screen.h"
 
 #include "base/logging.h"
+#include "base/memory/weak_ptr.h"
 #include "base/metrics/histogram_functions.h"
 #include "chrome/browser/ash/login/ui/login_display_host.h"
 #include "chrome/browser/ash/login/wizard_controller.h"
@@ -39,11 +40,11 @@ void LogEvent(AdbSideloadingPromptEvent action) {
 }  // namespace
 
 EnableAdbSideloadingScreen::EnableAdbSideloadingScreen(
-    EnableAdbSideloadingScreenView* view,
+    base::WeakPtr<EnableAdbSideloadingScreenView> view,
     const base::RepeatingClosure& exit_callback)
     : BaseScreen(EnableAdbSideloadingScreenView::kScreenId,
                  OobeScreenPriority::SCREEN_DEVICE_DEVELOPER_MODIFICATION),
-      view_(view),
+      view_(std::move(view)),
       exit_callback_(exit_callback) {
   if (view_)
     view_->Bind(this);
@@ -167,10 +168,4 @@ void EnableAdbSideloadingScreen::OnLearnMore() {
   help_app_->ShowHelpTopic(topic);
 }
 
-void EnableAdbSideloadingScreen::OnViewDestroyed(
-    EnableAdbSideloadingScreenView* view) {
-  if (view_ == view)
-    view_ = nullptr;
-}
-
 }  // namespace ash
diff --git a/chrome/browser/ash/login/screens/enable_adb_sideloading_screen.h b/chrome/browser/ash/login/screens/enable_adb_sideloading_screen.h
@@ -23,7 +23,7 @@ namespace ash {
 // adb sideloading screen to users.
 class EnableAdbSideloadingScreen : public BaseScreen {
  public:
-  EnableAdbSideloadingScreen(EnableAdbSideloadingScreenView* view,
+  EnableAdbSideloadingScreen(base::WeakPtr<EnableAdbSideloadingScreenView> view,
                              const base::RepeatingClosure& exit_callback);
 
   EnableAdbSideloadingScreen(const EnableAdbSideloadingScreen&) = delete;
@@ -32,9 +32,6 @@ class EnableAdbSideloadingScreen : public BaseScreen {
 
   ~EnableAdbSideloadingScreen() override;
 
-  // Called by EnableAdbSideloadingHandler.
-  void OnViewDestroyed(EnableAdbSideloadingScreenView* view);
-
   // Registers Local State preferences.
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
@@ -60,7 +57,7 @@ class EnableAdbSideloadingScreen : public BaseScreen {
   // Help application used for help dialogs.
   scoped_refptr<HelpAppLauncher> help_app_;
 
-  EnableAdbSideloadingScreenView* view_;
+  base::WeakPtr<EnableAdbSideloadingScreenView> view_;
   base::RepeatingClosure exit_callback_;
   base::WeakPtrFactory<EnableAdbSideloadingScreen> weak_ptr_factory_{this};
 };

diff --git a/chrome/browser/ash/login/screens/mock_enable_adb_sideloading_screen.cc b/chrome/browser/ash/login/screens/mock_enable_adb_sideloading_screen.cc
@@ -3,29 +3,29 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/ash/login/screens/mock_enable_adb_sideloading_screen.h"
+#include "base/memory/weak_ptr.h"
 
 namespace ash {
 
 using ::testing::_;
 using ::testing::AtLeast;
 
 MockEnableAdbSideloadingScreen::MockEnableAdbSideloadingScreen(
-    EnableAdbSideloadingScreenView* view,
+    base::WeakPtr<EnableAdbSideloadingScreenView> view,
     const base::RepeatingClosure& exit_callback)
-    : EnableAdbSideloadingScreen(view, exit_callback) {}
+    : EnableAdbSideloadingScreen(std::move(view), exit_callback) {}
 
-MockEnableAdbSideloadingScreen::~MockEnableAdbSideloadingScreen() {}
+MockEnableAdbSideloadingScreen::~MockEnableAdbSideloadingScreen() = default;
 
 void MockEnableAdbSideloadingScreen::ExitScreen() {
   exit_callback()->Run();
 }
 
-MockEnableAdbSideloadingScreenView::MockEnableAdbSideloadingScreenView() {}
+MockEnableAdbSideloadingScreenView::MockEnableAdbSideloadingScreenView() =
+    default;
 
-MockEnableAdbSideloadingScreenView::~MockEnableAdbSideloadingScreenView() {
-  if (screen_)
-    screen_->OnViewDestroyed(this);
-}
+MockEnableAdbSideloadingScreenView::~MockEnableAdbSideloadingScreenView() =
+    default;
 
 void MockEnableAdbSideloadingScreenView::Bind(
     EnableAdbSideloadingScreen* screen) {

diff --git a/chrome/browser/ash/login/screens/mock_enable_adb_sideloading_screen.h b/chrome/browser/ash/login/screens/mock_enable_adb_sideloading_screen.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_ASH_LOGIN_SCREENS_MOCK_ENABLE_ADB_SIDELOADING_SCREEN_H_
 #define CHROME_BROWSER_ASH_LOGIN_SCREENS_MOCK_ENABLE_ADB_SIDELOADING_SCREEN_H_
 
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/ash/login/screens/enable_adb_sideloading_screen.h"
 #include "chrome/browser/ui/webui/chromeos/login/enable_adb_sideloading_screen_handler.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -13,8 +14,9 @@ namespace ash {
 
 class MockEnableAdbSideloadingScreen : public EnableAdbSideloadingScreen {
  public:
-  MockEnableAdbSideloadingScreen(EnableAdbSideloadingScreenView* view,
-                                 const base::RepeatingClosure& exit_callback);
+  MockEnableAdbSideloadingScreen(
+      base::WeakPtr<EnableAdbSideloadingScreenView> view,
+      const base::RepeatingClosure& exit_callback);
   ~MockEnableAdbSideloadingScreen() override;
 
   MOCK_METHOD(void, ShowImpl, ());

diff --git a/chrome/browser/ash/login/wizard_controller.cc b/chrome/browser/ash/login/wizard_controller.cc
@@ -582,7 +582,7 @@ std::vector<std::unique_ptr<BaseScreen>> WizardController::CreateScreens() {
       base::BindRepeating(&WizardController::OnDemoSetupScreenExit,
                           weak_factory_.GetWeakPtr())));
   append(std::make_unique<EnableAdbSideloadingScreen>(
-      oobe_ui->GetView<EnableAdbSideloadingScreenHandler>(),
+      oobe_ui->GetView<EnableAdbSideloadingScreenHandler>()->AsWeakPtr(),
       base::BindRepeating(&WizardController::OnEnableAdbSideloadingScreenExit,
                           weak_factory_.GetWeakPtr())));
   append(std::make_unique<EnableDebuggingScreen>(

diff --git a/chrome/browser/ash/login/wizard_controller_browsertest.cc b/chrome/browser/ash/login/wizard_controller_browsertest.cc
@@ -608,7 +608,7 @@ class WizardControllerFlowTest : public WizardControllerTest {
     ExpectBindUnbind(mock_enable_adb_sideloading_screen_view_.get());
     mock_enable_adb_sideloading_screen_ = MockScreenExpectLifecycle(
         std::make_unique<MockEnableAdbSideloadingScreen>(
-            mock_enable_adb_sideloading_screen_view_.get(),
+            mock_enable_adb_sideloading_screen_view_->AsWeakPtr(),
             base::BindRepeating(
                 &WizardController::OnEnableAdbSideloadingScreenExit,
                 base::Unretained(wizard_controller))));

diff --git a/chrome/browser/ui/webui/chromeos/login/enable_adb_sideloading_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/enable_adb_sideloading_screen_handler.cc
@@ -23,10 +23,8 @@ EnableAdbSideloadingScreenHandler::EnableAdbSideloadingScreenHandler()
   set_user_acted_method_path("login.EnableAdbSideloadingScreen.userActed");
 }
 
-EnableAdbSideloadingScreenHandler::~EnableAdbSideloadingScreenHandler() {
-  if (screen_)
-    screen_->OnViewDestroyed(this);
-}
+EnableAdbSideloadingScreenHandler::~EnableAdbSideloadingScreenHandler() =
+    default;
 
 void EnableAdbSideloadingScreenHandler::Show() {
   if (!page_is_ready()) {
@@ -40,12 +38,10 @@ void EnableAdbSideloadingScreenHandler::Hide() {}
 
 void EnableAdbSideloadingScreenHandler::Bind(
     EnableAdbSideloadingScreen* screen) {
-  screen_ = screen;
-  BaseScreenHandler::SetBaseScreen(screen_);
+  BaseScreenHandler::SetBaseScreen(screen);
 }
 
 void EnableAdbSideloadingScreenHandler::Unbind() {
-  screen_ = nullptr;
   BaseScreenHandler::SetBaseScreen(nullptr);
 }
 

diff --git a/chrome/browser/ui/webui/chromeos/login/enable_adb_sideloading_screen_handler.h b/chrome/browser/ui/webui/chromeos/login/enable_adb_sideloading_screen_handler.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_UI_WEBUI_CHROMEOS_LOGIN_ENABLE_ADB_SIDELOADING_SCREEN_HANDLER_H_
 #define CHROME_BROWSER_UI_WEBUI_CHROMEOS_LOGIN_ENABLE_ADB_SIDELOADING_SCREEN_HANDLER_H_
 
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/ui/webui/chromeos/login/base_screen_handler.h"
 
 namespace ash {
@@ -14,7 +15,8 @@ class EnableAdbSideloadingScreen;
 namespace chromeos {
 
 // Interface between enable adb sideloading screen and its representation.
-class EnableAdbSideloadingScreenView {
+class EnableAdbSideloadingScreenView
+    : public base::SupportsWeakPtr<EnableAdbSideloadingScreenView> {
  public:
   constexpr static StaticOobeScreenId kScreenId{"adb-sideloading"};
 
@@ -24,7 +26,7 @@ class EnableAdbSideloadingScreenView {
     UI_STATE_SETUP = 2,
   };
 
-  virtual ~EnableAdbSideloadingScreenView() {}
+  virtual ~EnableAdbSideloadingScreenView() = default;
 
   virtual void Show() = 0;
   virtual void Hide() = 0;
@@ -61,8 +63,6 @@ class EnableAdbSideloadingScreenHandler : public EnableAdbSideloadingScreenView,
   void Initialize() override;
 
  private:
-  ash::EnableAdbSideloadingScreen* screen_ = nullptr;
-
   // Keeps whether screen should be shown right after initialization.
   bool show_on_init_ = false;
 };