Fix color chooser cleanup process.

- In case of crbug.com/139441, End() should close the window but it also schedules OnColorChooserDialogClosed() and this has been released already.  That call isn't necessary in this case, so set_listener to NULL.
- tab is expected to call DidEndColorChooser() everytime it ends, thus it should be called in End() too. the callback to prevent the calling


BUG=139441


Review URL: https://chromiumcodereview.appspot.com/10821087

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149380 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/views/color_chooser_aura.cc

@@ -47,6 +47,7 @@ ColorChooserAura::ColorChooserAura(int identifier,
                                    SkColor initial_color)
     : ColorChooser(identifier),
       tab_(tab) {
+  DCHECK(tab_);
   view_ = new views::ColorChooserView(this, initial_color);
   widget_ = views::Widget::CreateWindow(view_);
   widget_->SetAlwaysOnTop(true);
@@ -54,23 +55,24 @@ ColorChooserAura::ColorChooserAura(int identifier,
 }
 
 void ColorChooserAura::OnColorChosen(SkColor color) {
-  if (tab_)
-    tab_->DidChooseColorInColorChooser(identifier(), color);
+  tab_->DidChooseColorInColorChooser(identifier(), color);
 }
 
 void ColorChooserAura::OnColorChooserDialogClosed() {
-  if (tab_)
-    tab_->DidEndColorChooser(identifier());
   view_ = NULL;
   widget_ = NULL;
+  tab_->DidEndColorChooser(identifier());
 }
 
 void ColorChooserAura::End() {
   if (widget_ && widget_->IsVisible()) {
+    view_->set_listener(NULL);
     widget_->Close();
-    tab_ = NULL;
     view_ = NULL;
     widget_ = NULL;
+    // DidEndColorChooser will invoke Browser::DidEndColorChooser, which deletes
+    // this. Take care of the call order.
+    tab_->DidEndColorChooser(identifier());
   }
 }
 

ui/views/color_chooser/color_chooser_view.cc

@@ -339,7 +339,8 @@ void ColorChooserView::OnColorChanged(SkColor color) {
 void ColorChooserView::OnHueChosen(SkScalar hue) {
   hsv_[0] = hue;
   SkColor color = SkHSVToColor(255, hsv_);
-  listener_->OnColorChosen(color);
+  if (listener_)
+    listener_->OnColorChosen(color);
   saturation_value_->OnHueChanged(hue);
   textfield_->SetText(GetColorText(color));
 }
@@ -349,7 +350,8 @@ void ColorChooserView::OnSaturationValueChosen(SkScalar saturation,
   hsv_[1] = saturation;
   hsv_[2] = value;
   SkColor color = SkHSVToColor(255, hsv_);
-  listener_->OnColorChosen(color);
+  if (listener_)
+    listener_->OnColorChosen(color);
   textfield_->SetText(GetColorText(color));
 }
 
@@ -375,7 +377,8 @@ void ColorChooserView::ContentsChanged(Textfield* sender,
   SkColor color = SK_ColorBLACK;
   if (GetColorFromText(new_contents, &color)) {
     SkColorToHSV(color, hsv_);
-    listener_->OnColorChosen(color);
+    if (listener_)
+      listener_->OnColorChosen(color);
     hue_->OnHueChanged(hsv_[0]);
     saturation_value_->OnHueChanged(hsv_[0]);
     saturation_value_->OnSaturationValueChanged(hsv_[1], hsv_[2]);

ui/views/color_chooser/color_chooser_view.h

@@ -41,6 +41,7 @@ class VIEWS_EXPORT ColorChooserView : public WidgetDelegateView,
   float hue() const { return hsv_[0]; }
   float saturation() const { return hsv_[1]; }
   float value() const { return hsv_[2]; }
+  void set_listener(ColorChooserListener* listener) { listener_ = listener; }
 
  private:
   class HueView;