Bug 944737 - Allow only one color picker at a time. r=dholbert,jmathies a=bajaj
authorArnaud Bienner <arnaud.bienner@gmail.com>
Sat, 11 Jan 2014 18:46:20 +0100
changeset 176012 89acdc983be0812765d6da26463cb2fef7cb7727
parent 176011 12221143c88b6b512af00952a296f22e1a3b636e
child 176013 d06ab902cccac1b8878d5e568a297b371497e07a
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, jmathies, bajaj
bugs944737
milestone28.0a2
Bug 944737 - Allow only one color picker at a time. r=dholbert,jmathies a=bajaj
widget/windows/nsColorPicker.cpp
widget/windows/nsColorPicker.h
--- a/widget/windows/nsColorPicker.cpp
+++ b/widget/windows/nsColorPicker.cpp
@@ -3,16 +3,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/. */
 
 #include "nsColorPicker.h"
 
 #include <shlwapi.h>
 
+#include "mozilla/AutoRestore.h"
 #include "nsIWidget.h"
 #include "WidgetUtils.h"
 
 using namespace mozilla::widget;
 
 namespace
 {
 // Manages NS_NATIVE_TMP_WINDOW child windows. NS_NATIVE_TMP_WINDOWs are
@@ -84,40 +85,56 @@ BGRIntToRGBString(DWORD color, nsAString
 
   aResult.AssignLiteral("#");
   aResult.Append(ToHexString(r));
   aResult.Append(ToHexString(g));
   aResult.Append(ToHexString(b));
 }
 } // anonymous namespace
 
-AsyncColorChooser::AsyncColorChooser(DWORD aInitialColor, nsIWidget* aParentWidget, nsIColorPickerShownCallback* aCallback)
+AsyncColorChooser::AsyncColorChooser(const nsAString& aInitialColor,
+                                     nsIWidget* aParentWidget,
+                                     nsIColorPickerShownCallback* aCallback)
   : mInitialColor(aInitialColor)
   , mParentWidget(aParentWidget)
   , mCallback(aCallback)
 {
 }
 
 NS_IMETHODIMP
 AsyncColorChooser::Run()
 {
-  CHOOSECOLOR options;
-  static COLORREF customColors[16] = {0} ;
+  static COLORREF sCustomColors[16] = {0} ;
+
+  MOZ_ASSERT(NS_IsMainThread(),
+      "Color pickers can only be opened from main thread currently");
 
-  AutoDestroyTmpWindow adtw((HWND) (mParentWidget.get() ?
-    mParentWidget->GetNativeData(NS_NATIVE_TMP_WINDOW) : nullptr));
+  static bool sColorPickerOpen = false;
+  // Allow only one color picker to be opened at a time, to workaround bug 944737
+  if (!sColorPickerOpen) {
+    mozilla::AutoRestore<bool> autoRestoreColorPickerOpen(sColorPickerOpen);
+    sColorPickerOpen = true;
+
+    AutoDestroyTmpWindow adtw((HWND) (mParentWidget.get() ?
+      mParentWidget->GetNativeData(NS_NATIVE_TMP_WINDOW) : nullptr));
 
-  options.lStructSize   = sizeof(options);
-  options.hwndOwner     = adtw.get();
-  options.Flags         = CC_RGBINIT | CC_FULLOPEN;
-  options.rgbResult     = mInitialColor;
-  options.lpCustColors  = customColors;
+    CHOOSECOLOR options;
+    options.lStructSize   = sizeof(options);
+    options.hwndOwner     = adtw.get();
+    options.Flags         = CC_RGBINIT | CC_FULLOPEN;
+    options.rgbResult     = ColorStringToRGB(mInitialColor);
+    options.lpCustColors  = sCustomColors;
 
-  if (ChooseColor(&options)) {
-    BGRIntToRGBString(options.rgbResult, mColor);
+    if (ChooseColor(&options)) {
+      BGRIntToRGBString(options.rgbResult, mColor);
+    }
+  } else {
+    NS_WARNING("Currently, it's not possible to open more than one color "
+               "picker at a time");
+    mColor = mInitialColor;
   }
 
   if (mCallback) {
     mCallback->Done(mColor);
   }
 
   return NS_OK;
 }
@@ -131,22 +148,24 @@ nsColorPicker::nsColorPicker()
 
 nsColorPicker::~nsColorPicker()
 {
 }
 
 NS_IMPL_ISUPPORTS1(nsColorPicker, nsIColorPicker)
 
 NS_IMETHODIMP
-nsColorPicker::Init(nsIDOMWindow* parent, const nsAString& title, const nsAString& aInitialColor)
+nsColorPicker::Init(nsIDOMWindow* parent,
+                    const nsAString& title,
+                    const nsAString& aInitialColor)
 {
   NS_PRECONDITION(parent,
       "Null parent passed to colorpicker, no color picker for you!");
   mParentWidget =  WidgetUtils::DOMWindowToWidget(parent);
-  mInitialColor = ColorStringToRGB(aInitialColor);
+  mInitialColor = aInitialColor;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsColorPicker::Open(nsIColorPickerShownCallback* aCallback)
 {
   NS_ENSURE_ARG(aCallback);
   nsCOMPtr<nsIRunnable> event = new AsyncColorChooser(mInitialColor, mParentWidget, aCallback);
--- a/widget/windows/nsColorPicker.h
+++ b/widget/windows/nsColorPicker.h
@@ -16,36 +16,39 @@
 #include "nsThreadUtils.h"
 
 class nsIWidget;
 
 class AsyncColorChooser :
   public nsRunnable
 {
 public:
-  AsyncColorChooser(DWORD aInitialColor, nsIWidget* aParentWidget, nsIColorPickerShownCallback* aCallback);
+  AsyncColorChooser(const nsAString& aInitialColor,
+                    nsIWidget* aParentWidget,
+                    nsIColorPickerShownCallback* aCallback);
   NS_IMETHOD Run() MOZ_OVERRIDE;
 
 private:
-  DWORD mInitialColor;
+  nsString mInitialColor;
   nsCOMPtr<nsIWidget> mParentWidget;
   nsCOMPtr<nsIColorPickerShownCallback> mCallback;
   nsString mColor;
 };
 
 class nsColorPicker :
   public nsIColorPicker
 {
 public:
   nsColorPicker();
   virtual ~nsColorPicker();
 
   NS_DECL_ISUPPORTS
 
-  NS_IMETHOD Init(nsIDOMWindow* parent, const nsAString& title, const nsAString& aInitialColor);
+  NS_IMETHOD Init(nsIDOMWindow* parent, const nsAString& title,
+                  const nsAString& aInitialColor);
   NS_IMETHOD Open(nsIColorPickerShownCallback* aCallback);
 
 protected:
-  DWORD mInitialColor;
+  nsString mInitialColor;
   nsCOMPtr<nsIWidget> mParentWidget;
 };
 
 #endif // nsColorPicker_h__