Firefox overlay is seen opened even if Import Overlay is Cancelled.

chrome/browser/ui/views/importer/import_lock_dialog_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/importer/import_lock_dialog_view.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 12 matching lines @@
 using base::UserMetricsAction;
 
 namespace importer {
 
 void ShowImportLockDialog(gfx::NativeWindow parent,
                           const base::Callback<void(bool)>& callback) {
   ImportLockDialogView::Show(parent, callback);
   content::RecordAction(UserMetricsAction("ImportLockDialogView_Shown"));
 }
 
+void HideImportLockDialog() {

[Ilya Sherman, 2017/03/27 15:21:52]

You'll need to also implement this method for Mac, within
chrome/browser/ui/cocoa/importer/import_lock_dialog_cocoa.mm.

[nikhil.sahni, 2017/03/28 11:22:00]

Hi Ilya Sherman,

Thanks for pointing this out , I have implemented this function in
import_lock_dialog_cocoa.mm but implementing it I am facing problem. I made the
lock_alert as global variable and trying to access the same in
HideImportLockDialog but getting Error:declaration requires an exit-time
destructor.
I am not aware of Mac programming , I tried to implement seeing some examples.
Please help me on this.
Thanks

+  ImportLockDialogView::Hide();
+}
+
 }  // namespace importer
 
+views::Widget* ImportLockDialogView::m_widget = NULL;

[Ilya Sherman, 2017/03/27 15:21:52]

nit: s/NULL/nullptr

[nikhil.sahni, 2017/03/28 11:22:00]

Done.

+
+// static
+void ImportLockDialogView::Hide() {
+  if (m_widget) {

[Ilya Sherman, 2017/03/27 15:21:52]

It looks like the widget can be destroyed without m_widget ever being updated. 
I think you'll need to register a WidgetObserver that can listen for
OnWidgetDestroying() and reset this pointer to null.

[nikhil.sahni, 2017/03/28 11:22:00]

Here I guess there won't be any issue as even if it is called without calling
ShowImportLockDialog m_widget will be NULL so further API execution will not
happen. and ShowImportLockDialog cannot be called multiple times as the button
gets Disabled in this Scenario untill Hide happens.

[Ilya Sherman, 2017/03/29 06:29:02]

At a minimum, you'll want to reset g_widget to null after closing it.  But, what
happens if the user goes through the normal flow, and upon seeing that dialog,
closes Firefox?  I *think* the dialog will be destroyed after the user presses
"Continue".  Then, at some later time, the code will call
HideImportLockDialog(), which will reach this code path.  This code should not
try to close the dialog if it has already been destroyed.  Am I correct that
this is currently an issue?  If not, what am I misunderstanding?

[nikhil.sahni, 2017/03/30 12:13:35]

yes you are right this case needs to be handled, but no need of Observer as you
already have ImportLockDialogView::Accept function which gets called on pressing
Continue as you pointed out , there I am setting g_widget = nullptr .

+    m_widget->Hide();
+    m_widget->CloseNow();

[Ilya Sherman, 2017/03/27 15:21:52]

Hmm, is it actually necessary to call both of these methods?  I'm not very
familiar with Views, but it seems a bit surprising that you'd need both.  It
looks like the Close() method should do what you need.

[nikhil.sahni, 2017/03/28 11:22:00]

Done.

+  }
+}
+
 // static
 void ImportLockDialogView::Show(gfx::NativeWindow parent,
                                 const base::Callback<void(bool)>& callback) {
-  views::DialogDelegate::CreateDialogWidget(
-      new ImportLockDialogView(callback), NULL, NULL)->Show();
+  m_widget = views::DialogDelegate::CreateDialogWidget(
+      new ImportLockDialogView(callback), NULL, NULL);
+  m_widget->Show();
 }
 
 ImportLockDialogView::ImportLockDialogView(
     const base::Callback<void(bool)>& callback)
     : description_label_(NULL),
       callback_(callback) {
   description_label_ = new views::Label(
       l10n_util::GetStringUTF16(IDS_IMPORTER_LOCK_TEXT));
   description_label_->SetMultiLine(true);
   description_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
@@ skipping 31 matching lines @@
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
                                                 base::Bind(callback_, true));
   return true;
 }
 
 bool ImportLockDialogView::Cancel() {
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
                                                 base::Bind(callback_, false));
   return true;
 }