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

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

Index: chrome/browser/ui/views/importer/import_lock_dialog_view.cc
diff --git a/chrome/browser/ui/views/importer/import_lock_dialog_view.cc b/chrome/browser/ui/views/importer/import_lock_dialog_view.cc
index 71392b5908635fdeeced6d4f8fab2bd1cf316843..84db831e7ab18b82f38c64c62f89324db5f6c674 100644
--- a/chrome/browser/ui/views/importer/import_lock_dialog_view.cc
+++ b/chrome/browser/ui/views/importer/import_lock_dialog_view.cc
@@ -30,13 +30,35 @@ void ShowImportLockDialog(gfx::NativeWindow parent,
   base::RecordAction(UserMetricsAction("ImportLockDialogView_Shown"));
 }
 
+void HideImportLockDialog() {
+  ImportLockDialogView::Hide();
+}
+
 }  // namespace importer
 
+// For storing widget ptr on creation of dialog widget
+// using method CreateDialogWidget.

[Ilya Sherman, 2017/04/04 16:47:13]

nit: Please move this comment to be within the namespace, just above the
g_widget declaration.  And, please simplify it a bit to something like "A
reference to the currently active widget backing the import lock dialog, or null
if the dialog is not open."

[nikhil.sahni, 2017/04/05 06:07:55]

Done.

+namespace {
+static views::Widget* g_widget = nullptr;
+}

[Ilya Sherman, 2017/04/04 16:47:13]

nit: Please move this to the top of the file, just below the "using" declaration
on line 23 (well, please also leave a blank line following the using declaration
=])

[nikhil.sahni, 2017/04/05 06:07:55]

Done.

+
+// static
+void ImportLockDialogView::Hide() {
+  if (g_widget) {
+    g_widget->Close();
+    g_widget = nullptr;

[Ilya Sherman, 2017/04/04 16:47:13]

nit: This line should now be unnecessary, since the observer should handle it.

[nikhil.sahni, 2017/04/05 06:07:55]

Done.

+  }
+}

[Ilya Sherman, 2017/04/04 16:47:13]

nit: Please move this to be below the Show() function, to match the order of
ShowImportLockDialog and HideImportLockDIalog.

[nikhil.sahni, 2017/04/05 06:07:54]

Done.

+
 // static
 void ImportLockDialogView::Show(gfx::NativeWindow parent,
                                 const base::Callback<void(bool)>& callback) {
-  views::DialogDelegate::CreateDialogWidget(
-      new ImportLockDialogView(callback), NULL, NULL)->Show();
+  ImportLockDialogView* import_lock_dialog_view =

[Ilya Sherman, 2017/04/04 16:47:13]

Optional nit: I'd name this simply "view" or "dialog_view".

[nikhil.sahni, 2017/04/05 06:07:54]

Done.

+      new ImportLockDialogView(callback);
+  g_widget = views::DialogDelegate::CreateDialogWidget(import_lock_dialog_view,
+                                                       NULL, NULL);
+  g_widget->Show();
+  g_widget->AddObserver(import_lock_dialog_view);

[Ilya Sherman, 2017/04/04 16:47:13]

nit: Please add the observer before showing the widget, just to be a bit safer.

[nikhil.sahni, 2017/04/05 06:07:55]

Done.

 }
 
 ImportLockDialogView::ImportLockDialogView(
@@ -53,6 +75,11 @@ ImportLockDialogView::ImportLockDialogView(
 ImportLockDialogView::~ImportLockDialogView() {
 }
 
+void ImportLockDialogView::OnWidgetDestroying(views::Widget* widget) {

[Ilya Sherman, 2017/04/04 16:47:13]

nit: Please also add DCHECK_EQ(widget, g_widget);

[nikhil.sahni, 2017/04/05 06:07:54]

Done.

+  g_widget = nullptr;
+  widget->RemoveObserver(this);
+}
+
 gfx::Size ImportLockDialogView::GetPreferredSize() const {
   return gfx::Size(views::Widget::GetLocalizedContentsSize(
       IDS_IMPORTLOCK_DIALOG_WIDTH_CHARS,