Bug 975468 - Allow only one NSColorPanelWrapper to be opened at a time, and retarget the existing one if user clicks on another input type color. r=areinald
authorArnaud Bienner <arnaud.bienner@gmail.com>
Thu, 13 Mar 2014 21:52:14 +0100
changeset 191888 11685d14b3b4655794c11864d6b6a1dfe153970c
parent 191887 77b90f9bc6dbe97c6147d95b4329eeb3449ef6bb
child 191889 52892649259ed84fef1955c352150959638124eb
push id474
push userasasaki@mozilla.com
push dateMon, 02 Jun 2014 21:01:02 +0000
treeherdermozilla-release@967f4cf1b31c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersareinald
bugs975468
milestone30.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 975468 - Allow only one NSColorPanelWrapper to be opened at a time, and retarget the existing one if user clicks on another input type color. r=areinald
widget/cocoa/nsColorPicker.h
widget/cocoa/nsColorPicker.mm
--- a/widget/cocoa/nsColorPicker.h
+++ b/widget/cocoa/nsColorPicker.h
@@ -22,21 +22,27 @@ public:
   NS_DECL_ISUPPORTS
 
   NS_IMETHOD Init(nsIDOMWindow* aParent, const nsAString& aTitle,
                   const nsAString& aInitialColor);
   NS_IMETHOD Open(nsIColorPickerShownCallback* aCallback);
 
   // For NSColorPanelWrapper.
   void Update(NSColor* aColor);
+  // Call this method if you are done with this input, but the color picker needs to
+  // stay open as it will be associated to another input
+  void DoneWithRetarget();
+  // Same as DoneWithRetarget + clean the static instance of sColorPanelWrapper,
+  // as it is not needed anymore for now
   void Done();
 
 private:
   static NSColor* GetNSColorFromHexString(const nsAString& aColor);
   static void GetHexStringFromNSColor(NSColor* aColor, nsAString& aResult);
 
+  static NSColorPanelWrapper* sColorPanelWrapper;
+
   nsString             mTitle;
   nsString             mColor;
-  NSColorPanelWrapper* mColorPanel;
   nsCOMPtr<nsIColorPickerShownCallback> mCallback;
 };
 
 #endif // nsColorPicker_h_
--- a/widget/cocoa/nsColorPicker.mm
+++ b/widget/cocoa/nsColorPicker.mm
@@ -2,16 +2,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #import <Cocoa/Cocoa.h>
 
 #include "nsColorPicker.h"
 #include "nsCocoaUtils.h"
+#include "nsThreadUtils.h"
 
 using namespace mozilla;
 
 static unsigned int
 HexStrToInt(NSString* str)
 {
   unsigned int result = 0;
 
@@ -32,16 +33,17 @@ HexStrToInt(NSString* str)
 
 @interface NSColorPanelWrapper : NSObject <NSWindowDelegate>
 {
   NSColorPanel*  mColorPanel;
   nsColorPicker* mColorPicker;
 }
 - (id)initWithPicker:(nsColorPicker*)aPicker;
 - (void)open:(NSColor*)aInitialColor title:(NSString*)aTitle;
+- (void)retarget:(nsColorPicker*)aPicker;
 - (void)colorChanged:(NSColorPanel*)aPanel;
 @end
 
 @implementation NSColorPanelWrapper
 - (id)initWithPicker:(nsColorPicker*)aPicker
 {
   mColorPicker = aPicker;
   mColorPanel = [NSColorPanel sharedColorPanel];
@@ -65,16 +67,22 @@ HexStrToInt(NSString* str)
   mColorPicker->Update([mColorPanel color]);
 }
 
 - (void)windowWillClose:(NSNotification*)aNotification
 {
   mColorPicker->Done();
 }
 
+- (void)retarget:(nsColorPicker*)aPicker
+{
+  mColorPicker->DoneWithRetarget();
+  mColorPicker = aPicker;
+}
+
 - (void)dealloc
 {
   if ([mColorPanel delegate] == self) {
     [mColorPanel setTarget:nil];
     [mColorPanel setAction:nil];
     [mColorPanel setDelegate:nil];
   }
 
@@ -82,25 +90,34 @@ HexStrToInt(NSString* str)
   mColorPicker = nullptr;
 
   [super dealloc];
 }
 @end
 
 NS_IMPL_ISUPPORTS1(nsColorPicker, nsIColorPicker)
 
+NSColorPanelWrapper* nsColorPicker::sColorPanelWrapper = nullptr;
+
 NS_IMETHODIMP
 nsColorPicker::Init(nsIDOMWindow* aParent, const nsAString& aTitle,
                     const nsAString& aInitialColor)
 {
+  MOZ_ASSERT(NS_IsMainThread(),
+      "Color pickers can only be opened from main thread currently");
   mTitle = aTitle;
   mColor = aInitialColor;
 
-  mColorPanel = [[NSColorPanelWrapper alloc] initWithPicker:this];
-
+  if (sColorPanelWrapper) {
+    // Update current wrapper to target the new input instead
+    [sColorPanelWrapper retarget:this];
+  } else {
+    // Create a brand new color panel wrapper
+    sColorPanelWrapper = [[NSColorPanelWrapper alloc] initWithPicker:this];
+  }
   return NS_OK;
 }
 
 /* static */ NSColor*
 nsColorPicker::GetNSColorFromHexString(const nsAString& aColor)
 {
   NSString* str = nsCocoaUtils::ToNSString(aColor);
 
@@ -125,33 +142,38 @@ nsColorPicker::GetHexStringFromNSColor(N
 }
 
 NS_IMETHODIMP
 nsColorPicker::Open(nsIColorPickerShownCallback* aCallback)
 {
   MOZ_ASSERT(aCallback);
   mCallback = aCallback;
 
-  [mColorPanel open:GetNSColorFromHexString(mColor)
+  [sColorPanelWrapper open:GetNSColorFromHexString(mColor)
               title:nsCocoaUtils::ToNSString(mTitle)];
 
   NS_ADDREF_THIS();
 
   return NS_OK;
 }
 
 void
 nsColorPicker::Update(NSColor* aColor)
 {
   GetHexStringFromNSColor(aColor, mColor);
   mCallback->Update(mColor);
 }
 
 void
-nsColorPicker::Done()
+nsColorPicker::DoneWithRetarget()
 {
   mCallback->Done(EmptyString());
   mCallback = nullptr;
-
-  [mColorPanel release];
-
   NS_RELEASE_THIS();
 }
+
+void
+nsColorPicker::Done()
+{
+  [sColorPanelWrapper release];
+  sColorPanelWrapper = nullptr;
+  DoneWithRetarget();
+}