Change Add Supervision web dialog to use CanCloseDialog()

The Add Supervision dialog was using OnCloseContents() and
OnDialogCloseRequested() to catch attempts to close the
dialog, for the purpose of showing the signout confirmation
dialog. This failed when OnCloseContents() was called,
because by the time that callback was called, the web contents
were already disposed of, resulting in dialog contents becoming
unusable if the user chose to return to the dialog rather than
signing out.

Instead, capturing the close event in CanCloseDialog() occurs
before the contents are invalidated, therefore it is the correct
place to stop the closing of the dialog

Bug: 1001732
Change-Id: I50262bd4606e3380700c81f98df925ea3a61f87c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1790130
Commit-Queue: Danan S <danan@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694832}

chrome/browser/ui/webui/chromeos/add_supervision/add_supervision_metrics_recorder_browsertest.cc

@@ -42,11 +42,9 @@ class AddSupervisionMetricsRecorderTest : public InProcessBrowserTest {
   }
 
   void CloseAddSupervisionDialog() {
-    content::WebContents* web_contents = test_web_ui_.GetWebContents();
-    bool out_close_dialog;
-    AddSupervisionDialog::GetInstance()->OnCloseContents(web_contents,
-                                                         &out_close_dialog);
-    ASSERT_TRUE(out_close_dialog);
+    bool out_close_dialog =
+        AddSupervisionDialog::GetInstance()->OnDialogCloseRequested();
+    EXPECT_TRUE(out_close_dialog);
     AddSupervisionDialog::Close();
   }
 

chrome/browser/ui/webui/chromeos/add_supervision/add_supervision_ui.cc

@@ -119,29 +119,16 @@ void AddSupervisionDialog::GetDialogSize(gfx::Size* size) const {
   size->SetSize(kDialogWidthPx, kDialogHeightPx);
 }
 
-bool AddSupervisionDialog::OnDialogCloseRequested() {
+bool AddSupervisionDialog::CanCloseDialog() const {
   bool showing_confirm_dialog = MaybeShowConfirmSignoutDialog();
   return !showing_confirm_dialog;
 }
 
-void AddSupervisionDialog::OnCloseContents(content::WebContents* source,
-                                           bool* out_close_dialog) {
-  // This code gets called by a different path than OnDialogCloseRequested(),
-  // and actually masks the call to OnDialogCloseRequested() the first time the
-  // user clicks on the [x].  Because the first [x] click comes here, we need to
-  // show the confirmation dialog here and signal the caller to possibly close
-  // the dialog.  Subsequent clicks on [x] during the lifetime of the dialog
-  // will result in direct calls to OnDialogCloseRequested(), skipping this
-  // method.
-  *out_close_dialog = OnDialogCloseRequested();
-  // If |out_close_dialog|, then we are before the point of no return prior to
-  // completing enrollment, and that's the only time we want to record kClosed.
-  if (*out_close_dialog) {
-    // Record UMA metric that user has closed the Add Supervision dialog.
-    AddSupervisionMetricsRecorder::GetInstance()
-        ->RecordAddSupervisionEnrollment(
-            AddSupervisionMetricsRecorder::EnrollmentState::kClosed);
-  }
+bool AddSupervisionDialog::OnDialogCloseRequested() {
+  // Record UMA metric that user has closed the Add Supervision dialog.
+  AddSupervisionMetricsRecorder::GetInstance()->RecordAddSupervisionEnrollment(
+      AddSupervisionMetricsRecorder::EnrollmentState::kClosed);
+  return true;
 }
 
 AddSupervisionDialog::AddSupervisionDialog()

chrome/browser/ui/webui/chromeos/add_supervision/add_supervision_ui.h

@@ -37,8 +37,7 @@ class AddSupervisionDialog : public SystemWebDialogDelegate {
   ui::ModalType GetDialogModalType() const override;
   void GetDialogSize(gfx::Size* size) const override;
   bool OnDialogCloseRequested() override;
-  void OnCloseContents(content::WebContents* source,
-                       bool* out_close_dialog) override;
+  bool CanCloseDialog() const override;
 
  protected:
   AddSupervisionDialog();