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, a=sledru
authorArnaud Bienner <arnaud.bienner@gmail.com>
Thu, 13 Mar 2014 21:52:14 +0100
changeset 183422 869537b5d2877e19006f891928f50b477df00dbf
parent 183421 bdf9164956331f83378e37f512b14644e3043683
child 183423 361cac9505e5b3fe5fbb01f474c5a8d4114605be
push id3343
push userffxbld
push dateMon, 17 Mar 2014 21:55:32 +0000
treeherdermozilla-beta@2f7d3415f79f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersareinald, sledru
bugs975468
milestone29.0a2
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, a=sledru
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();
+}