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/metrics/user_metrics.h"
 #include "base/single_thread_task_runner.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);
   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(
     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);
   AddChildView(description_label_);
 }
 
 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,
       IDS_IMPORTLOCK_DIALOG_HEIGHT_LINES));
 }
 
 void ImportLockDialogView::Layout() {
   gfx::Rect bounds(GetLocalBounds());
   bounds.Inset(views::kButtonHEdgeMarginNew,
                LayoutDelegate::Get()->GetMetric(
@@ skipping 15 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;
 }