Bug 946479 - nsIFilePickers/nnsIColorPicker cannot run in parallel for a single HTMLInputElement, r=bz
authorAndrea Marchesini <amarchesini@mozilla.com>
Sat, 07 Dec 2013 12:09:14 +0000
changeset 174076 ccba2b6b092d0f43f5004fc9a76a6de0bc72170f
parent 174075 6aca1511c1bbc07fd7f51ee2f95b5b6134f7b8c0
child 174077 0c0feb0c132a1523b08ff5d860a08ce7a4a4b887
push id3224
push userlsblakk@mozilla.com
push dateTue, 04 Feb 2014 01:06:49 +0000
treeherdermozilla-beta@60c04d0987f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs946479
milestone28.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 946479 - nsIFilePickers/nnsIColorPicker cannot run in parallel for a single HTMLInputElement, r=bz
content/html/content/src/HTMLInputElement.cpp
content/html/content/src/HTMLInputElement.h
--- a/content/html/content/src/HTMLInputElement.cpp
+++ b/content/html/content/src/HTMLInputElement.cpp
@@ -609,16 +609,18 @@ private:
   // We'd prefer this member to be bool, but we don't support Atomic<bool>.
   mozilla::Atomic<uint32_t> mCanceled;
 };
 
 
 NS_IMETHODIMP
 HTMLInputElement::nsFilePickerShownCallback::Done(int16_t aResult)
 {
+  mInput->PickerClosed();
+
   if (aResult == nsIFilePicker::returnCancel) {
     return NS_OK;
   }
 
   mInput->CancelDirectoryPickerScanIfRunning();
 
   int16_t mode;
   mFilePicker->GetMode(&mode);
@@ -791,16 +793,18 @@ nsColorPickerShownCallback::Done(const n
    * When Done() is called, we might be at the end of a serie of Update() calls
    * in which case mValueChanged is set to true and a change event will have to
    * be fired but we might also be in a one shot Done() call situation in which
    * case we should fire a change event iif the value actually changed.
    * UpdateInternal(bool) is taking care of that logic for us.
    */
   nsresult rv = NS_OK;
 
+  mInput->PickerClosed();
+
   if (!aColor.IsEmpty()) {
     UpdateInternal(aColor, false);
   }
 
   if (mValueChanged) {
     rv = nsContentUtils::DispatchTrustedEvent(mInput->OwnerDoc(),
                                               static_cast<nsIDOMHTMLInputElement*>(mInput.get()),
                                               NS_LITERAL_STRING("change"), true,
@@ -834,16 +838,21 @@ HTMLInputElement::IsPopupBlocked() const
   uint32_t permission;
   pm->TestPermission(OwnerDoc()->NodePrincipal(), &permission);
   return permission == nsIPopupWindowManager::DENY_POPUP;
 }
 
 nsresult
 HTMLInputElement::InitColorPicker()
 {
+  if (mPickerRunning) {
+    NS_WARNING("Just one nsIColorPicker is allowed");
+    return NS_ERROR_FAILURE;
+  }
+
   nsCOMPtr<nsIDocument> doc = OwnerDoc();
 
   nsCOMPtr<nsPIDOMWindow> win = doc->GetWindow();
   if (!win) {
     return NS_ERROR_FAILURE;
   }
 
   if (IsPopupBlocked()) {
@@ -864,22 +873,32 @@ HTMLInputElement::InitColorPicker()
   nsAutoString initialValue;
   GetValueInternal(initialValue);
   nsresult rv = colorPicker->Init(win, title, initialValue);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIColorPickerShownCallback> callback =
     new nsColorPickerShownCallback(this, colorPicker);
 
-  return colorPicker->Open(callback);
+  rv = colorPicker->Open(callback);
+  if (NS_SUCCEEDED(rv)) {
+    mPickerRunning = true;
+  }
+
+  return rv;
 }
 
 nsresult
 HTMLInputElement::InitFilePicker(FilePickerType aType)
 {
+  if (mPickerRunning) {
+    NS_WARNING("Just one nsIFilePicker is allowed");
+    return NS_ERROR_FAILURE;
+  }
+
   // Get parent nsPIDOMWindow object.
   nsCOMPtr<nsIDocument> doc = OwnerDoc();
 
   nsCOMPtr<nsPIDOMWindow> win = doc->GetWindow();
   if (!win) {
     return NS_ERROR_FAILURE;
   }
 
@@ -950,20 +969,26 @@ HTMLInputElement::InitFilePicker(FilePic
     if (oldFiles.Length() == 1) {
       nsAutoString leafName;
       oldFiles[0]->GetName(leafName);
       if (!leafName.IsEmpty()) {
         filePicker->SetDefaultString(leafName);
       }
     }
 
-    return filePicker->Open(callback);
+    rv = filePicker->Open(callback);
+    if (NS_SUCCEEDED(rv)) {
+      mPickerRunning = true;
+    }
+
+    return rv;
   }
 
   HTMLInputElement::gUploadLastDir->FetchDirectoryAndDisplayPicker(doc, filePicker, callback);
+  mPickerRunning = true;
   return NS_OK;
 }
 
 #define CPS_PREF_NAME NS_LITERAL_STRING("browser.upload.lastDir")
 
 NS_IMPL_ISUPPORTS2(UploadLastDir, nsIObserver, nsISupportsWeakReference)
 
 void
@@ -1089,16 +1114,18 @@ HTMLInputElement::HTMLInputElement(alrea
   , mIndeterminate(false)
   , mInhibitRestoration(aFromParser & FROM_PARSER_FRAGMENT)
   , mCanShowValidUI(true)
   , mCanShowInvalidUI(true)
   , mHasRange(false)
   , mIsDraggingRange(false)
   , mProgressTimerIsActive(false)
   , mNumberControlSpinnerIsSpinning(false)
+  , mNumberControlSpinnerSpinsUp(false)
+  , mPickerRunning(false)
 {
   // We are in a type=text so we now we currenty need a nsTextEditorState.
   mInputData.mState = new nsTextEditorState(this);
 
   if (!gUploadLastDir)
     HTMLInputElement::InitUploadLastDir();
 
   // Set up our default state.  By default we're enabled (since we're
@@ -7173,16 +7200,22 @@ HTMLInputElement::UpdateHasRange()
 
   Decimal maximum = GetMaximum();
   if (!maximum.isNaN()) {
     mHasRange = true;
     return;
   }
 }
 
+void
+HTMLInputElement::PickerClosed()
+{
+  mPickerRunning = false;
+}
+
 JSObject*
 HTMLInputElement::WrapNode(JSContext* aCx, JS::Handle<JSObject*> aScope)
 {
   return HTMLInputElementBinding::Wrap(aCx, aScope, this);
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/content/html/content/src/HTMLInputElement.h
+++ b/content/html/content/src/HTMLInputElement.h
@@ -198,16 +198,19 @@ public:
   const nsTArray<nsCOMPtr<nsIDOMFile> >& GetFilesInternal() const
   {
     return mFiles;
   }
 
   void SetFiles(const nsTArray<nsCOMPtr<nsIDOMFile> >& aFiles, bool aSetValueChanged);
   void SetFiles(nsIDOMFileList* aFiles, bool aSetValueChanged);
 
+  // Called when a nsIFilePicker or a nsIColorPicker terminate.
+  void PickerClosed();
+
   void SetCheckedChangedInternal(bool aCheckedChanged);
   bool GetCheckedChanged() const {
     return mCheckedChanged;
   }
   void AddedToRadioGroup();
   void WillRemoveFromRadioGroup();
 
  /**
@@ -1262,16 +1265,17 @@ protected:
   bool                     mInhibitRestoration  : 1;
   bool                     mCanShowValidUI      : 1;
   bool                     mCanShowInvalidUI    : 1;
   bool                     mHasRange            : 1;
   bool                     mIsDraggingRange     : 1;
   bool                     mProgressTimerIsActive : 1;
   bool                     mNumberControlSpinnerIsSpinning : 1;
   bool                     mNumberControlSpinnerSpinsUp : 1;
+  bool                     mPickerRunning : 1;
 
 private:
   static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
                                     nsRuleData* aData);
 
   /**
    * Returns true if this input's type will fire a DOM "change" event when it
    * loses focus if its value has changed since it gained focus.