Fix bad reference counting of atoms so we don't manipulate garbage atom pointers. b=422546 r=timeless sr=roc a=damon
authordbaron@dbaron.org
Thu, 13 Mar 2008 11:54:01 -0700
changeset 13021 3032133086b85de1b1a84ab0bc01da5318df8db6
parent 13020 0634e71ff5477c315a1a8a77fee026daad0425f8
child 13022 b509e75904a84a9b2200e38565340ab27a6d677c
push idunknown
push userunknown
push dateunknown
reviewerstimeless, roc, damon
bugs422546
milestone1.9b5pre
Fix bad reference counting of atoms so we don't manipulate garbage atom pointers. b=422546 r=timeless sr=roc a=damon
content/base/src/nsGkAtomList.h
editor/libeditor/base/Makefile.in
editor/libeditor/base/PlaceholderTxn.cpp
editor/libeditor/base/nsEditor.cpp
editor/libeditor/base/nsEditor.h
editor/libeditor/html/Makefile.in
editor/libeditor/html/nsHTMLEditor.cpp
editor/libeditor/text/Makefile.in
editor/libeditor/text/nsPlaintextEditor.cpp
--- a/content/base/src/nsGkAtomList.h
+++ b/content/base/src/nsGkAtomList.h
@@ -1517,8 +1517,13 @@ GK_ATOM(usedPaddingProperty, "UsedPaddin
 GK_ATOM(viewProperty, "ViewProperty")                      
 
 // Languages for lang-specific transforms
 GK_ATOM(Japanese, "ja")
 GK_ATOM(Chinese, "zh-CN")
 GK_ATOM(Taiwanese, "zh-TW")
 GK_ATOM(HongKongChinese, "zh-HK")
 GK_ATOM(Unicode, "x-unicode")
+
+// Names for editor transactions
+GK_ATOM(TypingTxnName, "Typing")
+GK_ATOM(IMETxnName, "IME")
+GK_ATOM(DeleteTxnName, "Deleting")
--- a/editor/libeditor/base/Makefile.in
+++ b/editor/libeditor/base/Makefile.in
@@ -99,8 +99,11 @@ CPPSRCS		+=                          \
 		TransactionFactory.cpp      \
 		$(NULL)
 
 # don't want the shared lib; force the creation of a static lib.
 FORCE_STATIC_LIB = 1
 
 include $(topsrcdir)/config/rules.mk
 
+INCLUDES	+= \
+		-I$(topsrcdir)/content/base/src \
+		$(NULL)
--- a/editor/libeditor/base/PlaceholderTxn.cpp
+++ b/editor/libeditor/base/PlaceholderTxn.cpp
@@ -34,16 +34,17 @@
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "PlaceholderTxn.h"
 #include "nsEditor.h"
 #include "IMETextTxn.h"
+#include "nsGkAtoms.h"
 
 PlaceholderTxn::PlaceholderTxn() :  EditAggregateTxn(), 
                                     mAbsorb(PR_TRUE), 
                                     mForwarding(nsnull),
                                     mIMETextTxn(nsnull),
                                     mCommitted(PR_FALSE),
                                     mStartSel(nsnull),
                                     mEndSel(),
@@ -165,19 +166,19 @@ NS_IMETHODIMP PlaceholderTxn::Merge(nsIT
     *aDidMerge = PR_TRUE;
 //  RememberEndingSelection();
 //  efficiency hack: no need to remember selection here, as we haven't yet 
 //  finished the initial batch and we know we will be told when the batch ends.
 //  we can remeber the selection then.
   }
   else
   { // merge typing or IME or deletion transactions if the selection matches
-    if (((mName.get() == nsEditor::gTypingTxnName) ||
-         (mName.get() == nsEditor::gIMETxnName)    ||
-         (mName.get() == nsEditor::gDeleteTxnName)) 
+    if (((mName.get() == nsGkAtoms::TypingTxnName) ||
+         (mName.get() == nsGkAtoms::IMETxnName)    ||
+         (mName.get() == nsGkAtoms::DeleteTxnName)) 
          && !mCommitted ) 
     {
       nsCOMPtr<nsIAbsorbingTransaction> plcTxn;// = do_QueryInterface(editTxn);
       // can't do_QueryInterface() above due to our broken transaction interfaces.
       // instead have to brute it below. ugh. 
       editTxn->QueryInterface(NS_GET_IID(nsIAbsorbingTransaction), getter_AddRefs(plcTxn));
       if (plcTxn)
       {
--- a/editor/libeditor/base/nsEditor.cpp
+++ b/editor/libeditor/base/nsEditor.cpp
@@ -126,20 +126,16 @@ static PRBool gNoisy = PR_FALSE;
 extern nsIParserService *sParserService;
 
 //---------------------------------------------------------------------------
 //
 // nsEditor: base editor class implementation
 //
 //---------------------------------------------------------------------------
 
-nsIAtom *nsEditor::gTypingTxnName;
-nsIAtom *nsEditor::gIMETxnName;
-nsIAtom *nsEditor::gDeleteTxnName;
-
 nsEditor::nsEditor()
 :  mModCount(0)
 ,  mPresShellWeak(nsnull)
 ,  mViewManager(nsnull)
 ,  mUpdateCount(0)
 ,  mSpellcheckCheckboxState(eTriUnset)
 ,  mPlaceHolderTxn(nsnull)
 ,  mPlaceHolderName(nsnull)
@@ -156,66 +152,21 @@ nsEditor::nsEditor()
 ,  mIsIMEComposing(PR_FALSE)
 ,  mShouldTxnSetSelection(PR_TRUE)
 ,  mDidPreDestroy(PR_FALSE)
 ,  mDocDirtyState(-1)
 ,  mDocWeak(nsnull)
 ,  mPhonetic(nsnull)
 {
   //initialize member variables here
-  if (!gTypingTxnName)
-    gTypingTxnName = NS_NewAtom("Typing");
-  else
-    NS_ADDREF(gTypingTxnName);
-  if (!gIMETxnName)
-    gIMETxnName = NS_NewAtom("IME");
-  else
-    NS_ADDREF(gIMETxnName);
-  if (!gDeleteTxnName)
-    gDeleteTxnName = NS_NewAtom("Deleting");
-  else
-    NS_ADDREF(gDeleteTxnName);
 }
 
 nsEditor::~nsEditor()
 {
-  /* first, delete the transaction manager if there is one.
-     this will release any remaining transactions.
-     this is important because transactions can hold onto the atoms (gTypingTxnName, ...)
-     and to make the optimization (holding refcounted statics) work correctly, 
-     the editor instance needs to hold the last refcount.
-     If you get this wrong, expect to deref a garbage gTypingTxnName pointer if you bring up a second editor.
-  */
-  if (mTxnMgr) { 
-    mTxnMgr = 0;
-  }
-  nsrefcnt refCount=0;
-  if (gTypingTxnName)  // we addref'd in the constructor
-  { // want to release it without nulling out the pointer.
-    refCount = gTypingTxnName->Release();
-    if (0==refCount) {
-      gTypingTxnName = nsnull; 
-    }
-  }
-
-  if (gIMETxnName)  // we addref'd in the constructor
-  { // want to release it without nulling out the pointer.
-    refCount = gIMETxnName->Release();
-    if (0==refCount) {
-      gIMETxnName = nsnull;
-    }
-  }
-
-  if (gDeleteTxnName)  // we addref'd in the constructor
-  { // want to release it without nulling out the pointer.
-    refCount = gDeleteTxnName->Release();
-    if (0==refCount) {
-      gDeleteTxnName = nsnull;
-    }
-  }
+  mTxnMgr = nsnull;
 
   delete mPhonetic;
  
   NS_IF_RELEASE(mViewManager);
 }
 
 NS_IMPL_ISUPPORTS5(nsEditor, nsIEditor, nsIEditorIMESupport,
                    nsISupportsWeakReference, nsIPhonetic, nsIMutationObserver)
--- a/editor/libeditor/base/nsEditor.h
+++ b/editor/libeditor/base/nsEditor.h
@@ -576,23 +576,16 @@ public:
                                     nsIDOMNode *aEndNode,
                                     PRInt32 aEndOffset);
 
   already_AddRefed<nsPIDOMEventTarget> GetPIDOMEventTarget();
 
   // Fast non-refcounting editor root element accessor
   nsIDOMElement *GetRoot();
 
-public:
-  // Argh!  These transaction names are used by PlaceholderTxn and
-  // nsPlaintextEditor.  They should be localized to those classes.
-  static nsIAtom *gTypingTxnName;
-  static nsIAtom *gIMETxnName;
-  static nsIAtom *gDeleteTxnName;
-
 protected:
 
   PRUint32        mModCount;		// number of modifications (for undo/redo stack)
   PRUint32        mFlags;		// behavior flags. See nsIPlaintextEditor.idl for the flags we use.
   
   nsWeakPtr       mPresShellWeak;   // weak reference to the nsIPresShell
   nsWeakPtr       mSelConWeak;   // weak reference to the nsISelectionController
   nsIViewManager *mViewManager;
--- a/editor/libeditor/html/Makefile.in
+++ b/editor/libeditor/html/Makefile.in
@@ -97,9 +97,11 @@ DEFINES += -DENABLE_EDITOR_API_LOG
 endif
 
 # don't want the shared lib; force the creation of a static lib.
 FORCE_STATIC_LIB = 1
 
 include $(topsrcdir)/config/rules.mk
 
 INCLUDES        += -I$(topsrcdir)/editor/libeditor/base \
-                   -I$(topsrcdir)/editor/libeditor/text
+                   -I$(topsrcdir)/editor/libeditor/text \
+                   -I$(topsrcdir)/content/base/src \
+                   $(NULL)
--- a/editor/libeditor/html/nsHTMLEditor.cpp
+++ b/editor/libeditor/html/nsHTMLEditor.cpp
@@ -120,16 +120,17 @@
 // Transactionas
 #include "nsStyleSheetTxns.h"
 
 // Misc
 #include "TextEditorTest.h"
 #include "nsEditorUtils.h"
 #include "nsWSRunObject.h"
 #include "nsHTMLObjectResizer.h"
+#include "nsGkAtoms.h"
 
 #include "nsIFrame.h"
 #include "nsIView.h"
 #include "nsIWidget.h"
 #include "nsIParserService.h"
 #include "nsIEventStateManager.h"
 
 // Some utilities to handle annoying overloading of "A" tag for link and named anchor
@@ -1340,17 +1341,17 @@ NS_IMETHODIMP nsHTMLEditor::HandleKeyPre
    purposes.  Can't use HandleKeyPress() (above) for that since it takes
    a nsIDOMKeyEvent* parameter.  So instead we pass enough info through
    to TypedText() to determine what action to take, but without passing
    an event.
    */
 NS_IMETHODIMP nsHTMLEditor::TypedText(const nsAString& aString,
                                       PRInt32 aAction)
 {
-  nsAutoPlaceHolderBatch batch(this, gTypingTxnName);
+  nsAutoPlaceHolderBatch batch(this, nsGkAtoms::TypingTxnName);
 
   switch (aAction)
   {
     case eTypedText:
     case eTypedBreak:
       {
         return nsPlaintextEditor::TypedText(aString, aAction);
       }
--- a/editor/libeditor/text/Makefile.in
+++ b/editor/libeditor/text/Makefile.in
@@ -76,10 +76,12 @@ CPPSRCS		=                           \
 		nsTextEditRulesBidi.cpp \
 		$(NULL)
 
 # don't want the shared lib; force the creation of a static lib.
 FORCE_STATIC_LIB = 1
 
 include $(topsrcdir)/config/rules.mk
 
-INCLUDES        += -I$(topsrcdir)/editor/libeditor/base
-
+INCLUDES	+= \
+		-I$(topsrcdir)/editor/libeditor/base \
+		-I$(topsrcdir)/content/base/src \
+		$(NULL)
--- a/editor/libeditor/text/nsPlaintextEditor.cpp
+++ b/editor/libeditor/text/nsPlaintextEditor.cpp
@@ -74,16 +74,17 @@
 #include "nsEditorUtils.h"  // nsAutoEditBatch, nsAutoRules
 #include "nsIPrefBranch.h"
 #include "nsIPrefService.h"
 #include "nsUnicharUtils.h"
 #include "nsContentCID.h"
 #include "nsAOLCiter.h"
 #include "nsInternetCiter.h"
 #include "nsEventDispatcher.h"
+#include "nsGkAtoms.h"
 
 // Drag & Drop, Clipboard
 #include "nsIClipboard.h"
 #include "nsITransferable.h"
 #include "nsCopySupport.h"
 
 // prototype for rules creation shortcut
 nsresult NS_NewTextEditRules(nsIEditRules** aInstancePtrResult);
@@ -412,17 +413,17 @@ NS_IMETHODIMP nsPlaintextEditor::HandleK
    purposes.  Can't use HandleKeyPress() (above) for that since it takes
    a nsIDOMKeyEvent* parameter.  So instead we pass enough info through
    to TypedText() to determine what action to take, but without passing
    an event.
    */
 NS_IMETHODIMP nsPlaintextEditor::TypedText(const nsAString& aString,
                                       PRInt32 aAction)
 {
-  nsAutoPlaceHolderBatch batch(this, gTypingTxnName);
+  nsAutoPlaceHolderBatch batch(this, nsGkAtoms::TypingTxnName);
 
   switch (aAction)
   {
     case eTypedText:
       {
         return InsertText(aString);
       }
     case eTypedBreak:
@@ -638,17 +639,17 @@ nsPlaintextEditor::GetTextSelectionOffse
 
 NS_IMETHODIMP nsPlaintextEditor::DeleteSelection(nsIEditor::EDirection aAction)
 {
   if (!mRules) { return NS_ERROR_NOT_INITIALIZED; }
 
   nsresult result;
 
   // delete placeholder txns merge.
-  nsAutoPlaceHolderBatch batch(this, gDeleteTxnName);
+  nsAutoPlaceHolderBatch batch(this, nsGkAtoms::DeleteTxnName);
   nsAutoRules beginRulesSniffing(this, kOpDeleteSelection, aAction);
 
   // pre-process
   nsCOMPtr<nsISelection> selection;
   result = GetSelection(getter_AddRefs(selection));
   if (NS_FAILED(result)) return result;
   if (!selection) return NS_ERROR_NULL_POINTER;
 
@@ -1707,17 +1708,17 @@ nsPlaintextEditor::SetCompositionString(
     }
 
     // XXX_kin: END HACK! HACK! HACK!
 
     // we need the nsAutoPlaceHolderBatch destructor called before hitting
     // GetCaretCoordinates so the states in Frame system sync with content
     // therefore, we put the nsAutoPlaceHolderBatch into a inner block
     {
-      nsAutoPlaceHolderBatch batch(this, gIMETxnName);
+      nsAutoPlaceHolderBatch batch(this, nsGkAtoms::IMETxnName);
 
       SetIsIMEComposing(); // We set mIsIMEComposing properly.
 
       result = InsertText(aCompositionString);
 
       mIMEBufferLength = aCompositionString.Length();
 
       if (caretP)