From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Benedek Heilig <benecene@gmail.com>
Date: Thu, 21 Feb 2019 17:04:30 +0000
Subject: fix a crash when using color chooser on macos

Backports an upstream fix I made to the color chooser dialog on macos.
This can be removed once that fix lands in a chromium version we use.
Below is the original commit message:

Fix crash when closing color chooser dialog on macos

For some reason, closing the color chooser dialog caused a crash on
macos. By using NSNotificationCenter instead of providing a delegate
the crash goes away. I got the idea for this from the comment about the
__target method already in the code.

Change-Id: Ide35a455ff12decc9dd83b30c80b584ab376242b

diff --git a/AUTHORS b/AUTHORS
index 374b1a177f30d0c4b415d5c9c0fc4e4673414d39..436be8a093831020453285f0c476dcf9bd42fe8d 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -114,6 +114,7 @@ Ben Coe <bencoe@gmail.com>
 Ben Fiola <benfiola@gmail.com>
 Ben Karel <eschew@gmail.com>
 Ben Noordhuis <ben@strongloop.com>
+Benedek Heilig <benecene@gmail.com>
 Benjamin Dupont <bedupont@cisco.com>
 Benjamin Jemlich <pcgod99@gmail.com>
 Bernard Cafarelli <voyageur@gentoo.org>
diff --git a/chrome/browser/ui/cocoa/color_chooser_mac.h b/chrome/browser/ui/cocoa/color_chooser_mac.h
index 511000dc7855938ceab31694149ddf9e2125e0e4..9dbcc9fe41786555f0fb16ac47c71ee8851c8dd5 100644
--- a/chrome/browser/ui/cocoa/color_chooser_mac.h
+++ b/chrome/browser/ui/cocoa/color_chooser_mac.h
@@ -15,7 +15,7 @@ class ColorChooserMac;
 
 // A Listener class to act as a event target for NSColorPanel and send
 // the results to the C++ class, ColorChooserMac.
-@interface ColorPanelCocoa : NSObject<NSWindowDelegate> {
+@interface ColorPanelCocoa : NSObject {
  @protected
   // We don't call DidChooseColor if the change wasn't caused by the user
   // interacting with the panel.
@@ -26,6 +26,8 @@ class ColorChooserMac;
 
 - (id)initWithChooser:(ColorChooserMac*)chooser;
 
+- (void)windowWillClose:(NSNotification*)notification;
+
 // Called from NSColorPanel.
 - (void)didChooseColor:(NSColorPanel*)panel;
 
@@ -45,7 +47,7 @@ class ColorChooserMac : public content::ColorChooser {
 
   // Called from ColorPanelCocoa.
   void DidChooseColorInColorPanel(SkColor color);
-  void DidCloseColorPabel();
+  void DidCloseColorPanel();
 
   // Set the color programmatically.
   void SetSelectedColor(SkColor color) override;
diff --git a/chrome/browser/ui/cocoa/color_chooser_mac.mm b/chrome/browser/ui/cocoa/color_chooser_mac.mm
index bf47154f0a880f1c6143065e5c6d060f1df73765..e7cdab7a23d96345cc6e8ec578a8616d132b7d51 100644
--- a/chrome/browser/ui/cocoa/color_chooser_mac.mm
+++ b/chrome/browser/ui/cocoa/color_chooser_mac.mm
@@ -39,16 +39,18 @@
     web_contents_->DidChooseColorInColorChooser(color);
 }
 
-void ColorChooserMac::DidCloseColorPabel() {
+void ColorChooserMac::DidCloseColorPanel() {
   End();
 }
 
 void ColorChooserMac::End() {
-  panel_.reset();
-  DCHECK(current_color_chooser_ == this);
-  current_color_chooser_ = NULL;
-  if (web_contents_)
+  if (panel_) {
+    panel_.reset();
+    DCHECK(current_color_chooser_ == this);
+    current_color_chooser_ = NULL;
+    if (web_contents_)
       web_contents_->DidEndColorChooser();
+  }
 }
 
 void ColorChooserMac::SetSelectedColor(SkColor color) {
@@ -67,9 +69,14 @@ - (id)initWithChooser:(ColorChooserMac*)chooser {
     chooser_ = chooser;
     NSColorPanel* panel = [NSColorPanel sharedColorPanel];
     [panel setShowsAlpha:NO];
-    [panel setDelegate:self];
     [panel setTarget:self];
     [panel setAction:@selector(didChooseColor:)];
+
+    [[NSNotificationCenter defaultCenter]
+        addObserver:self
+           selector:@selector(windowWillClose:)
+               name:NSWindowWillCloseNotification
+             object:panel];
   }
   return self;
 }
@@ -82,19 +89,21 @@ - (void)dealloc {
   // the ColorPanelCocoa is still the target.
   BOOL respondsToPrivateTargetMethod =
       [panel respondsToSelector:@selector(__target)];
-
-  if ([panel delegate] == self ||
-      (respondsToPrivateTargetMethod && [panel __target] == self)) {
-    [panel setDelegate:nil];
+  if (respondsToPrivateTargetMethod && [panel __target] == self) {
     [panel setTarget:nil];
     [panel setAction:nullptr];
   }
 
+  [[NSNotificationCenter defaultCenter]
+      removeObserver:self
+                name:NSWindowWillCloseNotification
+              object:panel];
+
   [super dealloc];
 }
 
 - (void)windowWillClose:(NSNotification*)notification {
-  chooser_->DidCloseColorPabel();
+  chooser_->DidCloseColorPanel();
   nonUserChange_ = NO;
 }
 
diff --git a/chrome/browser/ui/cocoa/color_panel_cocoa_unittest.mm b/chrome/browser/ui/cocoa/color_panel_cocoa_unittest.mm
index 83185ceb9bbd29bf38fce7f72c23f3a5b15ba6c8..823365fdfc0bceceeed6f5edc00b3462715a4751 100644
--- a/chrome/browser/ui/cocoa/color_panel_cocoa_unittest.mm
+++ b/chrome/browser/ui/cocoa/color_panel_cocoa_unittest.mm
@@ -28,28 +28,6 @@ void SetUp() override {
   }
 };
 
-TEST_F(ColorPanelCocoaTest, ClearTargetAndDelegateOnEnd) {
-  NSColorPanel* nscolor_panel = [NSColorPanel sharedColorPanel];
-  @autoreleasepool {
-    EXPECT_TRUE([nscolor_panel respondsToSelector:@selector(__target)]);
-
-    // Create a ColorPanelCocoa.
-    ColorChooserMac* color_chooser_mac =
-        ColorChooserMac::Open(nullptr, SK_ColorBLACK);
-
-    // Confirm the NSColorPanel's configuration by the ColorChooserMac's
-    // ColorPanelCocoa.
-    EXPECT_TRUE([nscolor_panel delegate]);
-    EXPECT_TRUE([nscolor_panel __target]);
-
-    // Release the ColorPanelCocoa and confirm it's no longer the NSColorPanel's
-    // target or delegate.
-    color_chooser_mac->End();
-  }
-  EXPECT_EQ([nscolor_panel delegate], nil);
-  EXPECT_EQ([nscolor_panel __target], nil);
-}
-
 TEST_F(ColorPanelCocoaTest, ClearTargetOnEnd) {
   NSColorPanel* nscolor_panel = [NSColorPanel sharedColorPanel];
   @autoreleasepool {
@@ -61,19 +39,12 @@ void SetUp() override {
 
     // Confirm the NSColorPanel's configuration by the ColorChooserMac's
     // ColorPanelCocoa.
-    EXPECT_TRUE([nscolor_panel delegate]);
     EXPECT_TRUE([nscolor_panel __target]);
 
-    // Clear the delegate and release the ColorPanelCocoa.
-    [nscolor_panel setDelegate:nil];
-
     // Release the ColorPanelCocoa.
     color_chooser_mac->End();
   }
-  // Confirm the ColorPanelCocoa is no longer the NSColorPanel's target or
-  // delegate. Previously the ColorPanelCocoa would not clear the target if
-  // the delegate had already been cleared.
-  EXPECT_EQ([nscolor_panel delegate], nil);
+  // Confirm the ColorPanelCocoa is no longer the NSColorPanel's target
   EXPECT_EQ([nscolor_panel __target], nil);
 }
 
