ColorChooserWin::End should act like the dialog has closed

Cong Liu · Cong Liu · commit 3cc0d75ff917 · 2015-05-06T15:04:00.000+08:00
Bug fix copied from 5ecc8d42ff888ff8b459df566208e7e01a3be5ba
diff --git a/src/browser/color_chooser_dialog.cc b/src/browser/color_chooser_dialog.cc
@@ -12,6 +12,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "skia/ext/skia_utils_win.h"
 #include "ui/views/color_chooser/color_chooser_listener.h"
+#include "ui/views/win/hwnd_util.h"
 
 using content::BrowserThread;
 
@@ -32,17 +33,19 @@ ColorChooserDialog::ColorChooserDialog(views::ColorChooserListener* listener,
     : listener_(listener) {
   DCHECK(listener_);
   CopyCustomColors(g_custom_colors, custom_colors_);
-  ExecuteOpenParams execute_params(initial_color, BeginRun((HWND)owning_window),
-                                   (HWND)owning_window);
+  HWND owning_hwnd = views::HWNDForNativeWindow(owning_window);
+  ExecuteOpenParams execute_params(initial_color, BeginRun(owning_hwnd),
+                                   owning_hwnd);
   execute_params.run_state.dialog_thread->message_loop()->PostTask(FROM_HERE,
       base::Bind(&ColorChooserDialog::ExecuteOpen, this, execute_params));
 }
 
 ColorChooserDialog::~ColorChooserDialog() {
 }
 
-bool ColorChooserDialog::IsRunning(gfx::NativeWindow owning_hwnd) const {
-  return listener_ && IsRunningDialogForOwner((HWND)owning_hwnd);
+bool ColorChooserDialog::IsRunning(gfx::NativeWindow owning_window) const {
+  return listener_ && IsRunningDialogForOwner(
+      views::HWNDForNativeWindow(owning_window));
 }
 
 void ColorChooserDialog::ListenerDestroyed() {
@@ -68,13 +71,13 @@ void ColorChooserDialog::ExecuteOpen(const ExecuteOpenParams& params) {
 void ColorChooserDialog::DidCloseDialog(bool chose_color,
                                         SkColor color,
                                         RunState run_state) {
-  if (!listener_)
-    return;
   EndRun(run_state);
   CopyCustomColors(custom_colors_, g_custom_colors);
-  if (chose_color)
-    listener_->OnColorChosen(color);
-  listener_->OnColorChooserDialogClosed();
+  if (listener_) {
+    if (chose_color)
+      listener_->OnColorChosen(color);
+    listener_->OnColorChooserDialogClosed();
+  }
 }
 
 void ColorChooserDialog::CopyCustomColors(COLORREF* src, COLORREF* dst) {
diff --git a/src/browser/color_chooser_dialog.h b/src/browser/color_chooser_dialog.h
@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_UI_VIEWS_COLOR_CHOOSER_DIALOG_H_
-#define CHROME_BROWSER_UI_VIEWS_COLOR_CHOOSER_DIALOG_H_
+#ifndef NW_BROWSER_COLOR_CHOOSER_DIALOG_H_
+#define NW_BROWSER_COLOR_CHOOSER_DIALOG_H_
 
 #include "base/memory/ref_counted.h"
 #include "content/nw/src/browser/color_chooser_dialog.h"
@@ -70,4 +70,4 @@ class ColorChooserDialog
   DISALLOW_COPY_AND_ASSIGN(ColorChooserDialog);
 };
 
-#endif  // CHROME_BROWSER_UI_VIEWS_COLOR_CHOOSER_DIALOG_H_
+#endif  // NW_BROWSER_COLOR_CHOOSER_DIALOG_H_
diff --git a/src/browser/color_chooser_win.cc b/src/browser/color_chooser_win.cc
@@ -24,7 +24,7 @@ class ColorChooserWin : public content::ColorChooser,
   ~ColorChooserWin();
 
   // content::ColorChooser overrides:
-  virtual void End() OVERRIDE {}
+  virtual void End() OVERRIDE;
   virtual void SetSelectedColor(SkColor color) OVERRIDE {}
 
   // views::ColorChooserListener overrides:
@@ -46,8 +46,9 @@ ColorChooserWin* ColorChooserWin::current_color_chooser_ = NULL;
 
 ColorChooserWin* ColorChooserWin::Open(content::WebContents* web_contents,
                                        SkColor initial_color) {
-  if (!current_color_chooser_)
-    current_color_chooser_ = new ColorChooserWin(web_contents, initial_color);
+  if (current_color_chooser_)
+    return NULL;
+  current_color_chooser_ = new ColorChooserWin(web_contents, initial_color);
   return current_color_chooser_;
 }
 
@@ -66,6 +67,16 @@ ColorChooserWin::~ColorChooserWin() {
   DCHECK(!color_chooser_dialog_);
 }
 
+void ColorChooserWin::End() {
+  // The ColorChooserDialog's listener is going away.  Ideally we'd
+  // programmatically close the dialog at this point.  Since that's impossible,
+  // we instead tell the dialog its listener is going away, so that the dialog
+  // doesn't try to communicate with a destroyed listener later.  (We also tell
+  // the renderer the dialog is closed, since from the renderer's perspective
+  // it effectively is.)
+  OnColorChooserDialogClosed();
+}
+
 void ColorChooserWin::OnColorChosen(SkColor color) {
   if (web_contents_)
     web_contents_->DidChooseColorInColorChooser(color);
