ColorChooserWin::End should act like the dialog has closed

chrome/browser/ui/views/color_chooser_win.cc

Index: chrome/browser/ui/views/color_chooser_win.cc
diff --git a/chrome/browser/ui/views/color_chooser_win.cc b/chrome/browser/ui/views/color_chooser_win.cc
index b895c5e2b6e621ecbf8460d00a3757cef85fea6b..38486aa7004016a0eddcb9930e9aba90fae89ae7 100644
--- a/chrome/browser/ui/views/color_chooser_win.cc
+++ b/chrome/browser/ui/views/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:
@@ -66,6 +66,16 @@ ColorChooserWin::~ColorChooserWin() {
   DCHECK(!color_chooser_dialog_);
 }
 
+void ColorChooserWin::End() {
+  // While we can't programmatically close the dialog, we should at least have
+  // the dialog finalize state and inform listeners that it's closed, so that
+  // ColorChooserDialog::DidCloseDialog won't call the listener after it has
+  // been destroyed.
+  // If we don't do this we the ColorChooserDialog::DidCloseDialog will try to
+  // call the listener after it has been destroyed.

[Peter Kasting, 2013/08/30 19:35:47]

Grammar issue and redundancy.  Here's a cleanup attempt, now that I know a
little better what you're trying to solve.

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.)

[keishi, 2013/08/30 19:42:43]

This is better. Thanks. Done.

+  OnColorChooserDialogClosed();
+}
+
 void ColorChooserWin::OnColorChosen(SkColor color) {
   if (web_contents_)
     web_contents_->DidChooseColorInColorChooser(color);