diff --git a/patches/common/chromium/.patches b/patches/common/chromium/.patches index 5228b559cf..f324126af5 100644 --- a/patches/common/chromium/.patches +++ b/patches/common/chromium/.patches @@ -60,7 +60,6 @@ content_browser_main_loop.patch dump_syms.patch command-ismediakey.patch tts.patch -color_chooser.patch printing.patch verbose_generate_breakpad_symbols.patch cross_site_document_resource_handler.patch @@ -71,3 +70,5 @@ disable_time_ticks_dcheck.patch autofill_size_calculation.patch revert_build_swiftshader_for_arm32.patch fix_backup_includes_for_ptrace_get_thread_area_on_arm_arm64.patch +color_chooser_mac.patch +color_chooser_win.patch diff --git a/patches/common/chromium/color_chooser.patch b/patches/common/chromium/color_chooser.patch deleted file mode 100644 index aff91e9fd1..0000000000 --- a/patches/common/chromium/color_chooser.patch +++ /dev/null @@ -1,23 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Heilig Benedek -Date: Thu, 18 Oct 2018 17:08:13 -0700 -Subject: color_chooser.patch - -Disables a DCHECK that crashes the ColorChooser on Windows, -that DCHECK most likely is an artifact that remained in chromium from a -long time ago (last update of that part of the code was around 2012-2013, -and this is purely UI, I don't think they have automated tests for it). - -diff --git a/chrome/browser/ui/views/color_chooser_win.cc b/chrome/browser/ui/views/color_chooser_win.cc -index 434842ba0f81206a43fb26874930bf7309782890..93b4152003eaea05f0e16cd049687fbcbc672fb0 100644 ---- a/chrome/browser/ui/views/color_chooser_win.cc -+++ b/chrome/browser/ui/views/color_chooser_win.cc -@@ -91,7 +91,7 @@ void ColorChooserWin::OnColorChooserDialogClosed() { - color_chooser_dialog_->ListenerDestroyed(); - color_chooser_dialog_ = NULL; - } -- DCHECK(current_color_chooser_ == this); -+ // DCHECK(current_color_chooser_ == this); - current_color_chooser_ = NULL; - if (web_contents_) - web_contents_->DidEndColorChooser(); diff --git a/patches/common/chromium/color_chooser_mac.patch b/patches/common/chromium/color_chooser_mac.patch new file mode 100644 index 0000000000..9607c03010 --- /dev/null +++ b/patches/common/chromium/color_chooser_mac.patch @@ -0,0 +1,186 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benedek Heilig +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 + Ben Fiola + Ben Karel + Ben Noordhuis ++Benedek Heilig + Benjamin Dupont + Benjamin Jemlich + Bernard Cafarelli +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 { ++@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 @@ void ColorChooserMac::DidChooseColorInColorPanel(SkColor color) { + 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 @@ void ColorChooserMac::SetSelectedColor(SkColor color) { + 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 ColorChooserMac::SetSelectedColor(SkColor color) { + // 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 @@ class ColorPanelCocoaTest : public CocoaTest { + } + }; + +-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 @@ TEST_F(ColorPanelCocoaTest, ClearTargetOnEnd) { + + // 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); + } + diff --git a/patches/common/chromium/color_chooser_win.patch b/patches/common/chromium/color_chooser_win.patch new file mode 100644 index 0000000000..20fe0df1b9 --- /dev/null +++ b/patches/common/chromium/color_chooser_win.patch @@ -0,0 +1,92 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benedek Heilig +Date: Thu, 21 Feb 2019 17:16:33 +0000 +Subject: fix some DCHECKs on Windows when using color chooser dialog + +Backports an upstream fix I made to the color chooser dialog on +Windows. This can be removed once that fix lands in a chromium version we +use. Below is the original commit message: + +Fix DCHECKs being triggered while using color chooser dialog on Windows + +This fixes two DHCECKs being triggered on Windows when using the +dialog in a debug build. The main source of these triggered DCHECKs was +that when the user closes the dialog, OnColorChooserDialog is called, +which calls WebContentsImpl::DidEndColorChooser, which calls End on the +dialog, which calls OnColorChooserDialog again. This also happened on +macos. The other DCHECK was the receiver->HasAtLeastOneRef() one, +because ColorChooserDialog had a PostTask in it's constructor. + +Change-Id: I45ec32083a5850e006034073bc29d7ec4bb63189 + +diff --git a/chrome/browser/ui/views/color_chooser_dialog.cc b/chrome/browser/ui/views/color_chooser_dialog.cc +index c26be855120fcef78ce995d09fb3965f796746a9..1f568a2657250c5f21ab01da88950eedc6d4e177 100644 +--- a/chrome/browser/ui/views/color_chooser_dialog.cc ++++ b/chrome/browser/ui/views/color_chooser_dialog.cc +@@ -24,12 +24,14 @@ using content::BrowserThread; + // static + COLORREF ColorChooserDialog::g_custom_colors[16]; + +-ColorChooserDialog::ColorChooserDialog(views::ColorChooserListener* listener, +- SkColor initial_color, +- gfx::NativeWindow owning_window) ++ColorChooserDialog::ColorChooserDialog(views::ColorChooserListener* listener) + : listener_(listener) { + DCHECK(listener_); + CopyCustomColors(g_custom_colors, custom_colors_); ++} ++ ++void ColorChooserDialog::Open(SkColor initial_color, ++ gfx::NativeWindow owning_window) { + HWND owning_hwnd = views::HWNDForNativeWindow(owning_window); + + std::unique_ptr run_state = BeginRun(owning_hwnd); +diff --git a/chrome/browser/ui/views/color_chooser_dialog.h b/chrome/browser/ui/views/color_chooser_dialog.h +index 838e22dcd1a707714a098320d5bf8174fa60a2ea..b2558ce2a226ccde0f20de9712bf33051b21799c 100644 +--- a/chrome/browser/ui/views/color_chooser_dialog.h ++++ b/chrome/browser/ui/views/color_chooser_dialog.h +@@ -23,9 +23,9 @@ class ColorChooserDialog + public ui::BaseShellDialog, + public ui::BaseShellDialogImpl { + public: +- ColorChooserDialog(views::ColorChooserListener* listener, +- SkColor initial_color, +- gfx::NativeWindow owning_window); ++ explicit ColorChooserDialog(views::ColorChooserListener* listener); ++ ++ void Open(SkColor initial_color, gfx::NativeWindow owning_window); + + // BaseShellDialog: + bool IsRunning(gfx::NativeWindow owning_window) const override; +diff --git a/chrome/browser/ui/views/color_chooser_win.cc b/chrome/browser/ui/views/color_chooser_win.cc +index 434842ba0f81206a43fb26874930bf7309782890..ffc58eec78dc36e21c2c04aa0b0cd9097541d2f0 100644 +--- a/chrome/browser/ui/views/color_chooser_win.cc ++++ b/chrome/browser/ui/views/color_chooser_win.cc +@@ -61,9 +61,8 @@ ColorChooserWin::ColorChooserWin(content::WebContents* web_contents, + ->GetWidget() + ->GetView() + ->GetNativeView()); +- color_chooser_dialog_ = new ColorChooserDialog(this, +- initial_color, +- owning_window); ++ color_chooser_dialog_ = new ColorChooserDialog(this); ++ color_chooser_dialog_->Open(initial_color, owning_window); + } + + ColorChooserWin::~ColorChooserWin() { +@@ -90,11 +89,11 @@ void ColorChooserWin::OnColorChooserDialogClosed() { + if (color_chooser_dialog_.get()) { + color_chooser_dialog_->ListenerDestroyed(); + color_chooser_dialog_ = NULL; ++ DCHECK(current_color_chooser_ == this); ++ current_color_chooser_ = NULL; ++ if (web_contents_) ++ web_contents_->DidEndColorChooser(); + } +- DCHECK(current_color_chooser_ == this); +- current_color_chooser_ = NULL; +- if (web_contents_) +- web_contents_->DidEndColorChooser(); + } + + namespace chrome {