Bug 643786. Don't fire DOMNodeRemoved when removing the editor created <br>. r=ehsan
authorJonas Sicking <jonas@sicking.cc>
Mon, 09 May 2011 12:33:04 -0700
changeset 69461 d70ce672b2c323c03de60e8b341325c8b8f6a6b0
parent 69460 1447c689162d0240056beb6885024ba7d4fa4dbf
child 69462 33992b8ef80ef8ce48db4149b687fb58c421960f
push id76
push userbzbarsky@mozilla.com
push dateTue, 05 Jul 2011 17:00:57 +0000
treeherdermozilla-beta@d3a2732c35f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs643786
milestone6.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 643786. Don't fire DOMNodeRemoved when removing the editor created <br>. r=ehsan
content/base/public/nsContentUtils.h
content/base/src/nsContentUtils.cpp
editor/libeditor/html/crashtests/crashtests.list
editor/libeditor/html/nsHTMLEditRules.cpp
--- a/content/base/public/nsContentUtils.h
+++ b/content/base/public/nsContentUtils.h
@@ -125,16 +125,17 @@ class nsIXTFService;
 #endif
 #ifdef IBMBIDI
 class nsIBidiKeyboard;
 #endif
 class nsIMIMEHeaderParam;
 class nsIObserver;
 class nsPresContext;
 class nsIChannel;
+class nsAutoScriptBlockerSuppressNodeRemoved;
 struct nsIntMargin;
 class nsPIDOMWindow;
 class nsIDocumentLoaderFactory;
 
 #ifndef have_PrefChangedFunc_typedef
 typedef int (*PR_CALLBACK PrefChangedFunc)(const char *, void *);
 #define have_PrefChangedFunc_typedef
 #endif
@@ -179,16 +180,17 @@ struct nsShortcutCandidate {
   {
   }
   PRUint32 mCharCode;
   PRBool   mIgnoreShift;
 };
 
 class nsContentUtils
 {
+  friend class nsAutoScriptBlockerSuppressNodeRemoved;
   typedef mozilla::dom::Element Element;
 
 public:
   static nsresult Init();
 
   /**
    * Get a JSContext from the document's scope object.
    */
@@ -1908,16 +1910,19 @@ private:
 
 #ifdef IBMBIDI
   static nsIBidiKeyboard* sBidiKeyboard;
 #endif
 
   static PRBool sInitialized;
   static PRUint32 sScriptBlockerCount;
   static PRUint32 sRemovableScriptBlockerCount;
+#ifdef DEBUG
+  static PRUint32 sDOMNodeRemovedSuppressCount;
+#endif
   static nsCOMArray<nsIRunnable>* sBlockedScriptRunners;
   static PRUint32 sRunnersCountAtFirstBlocker;
   static PRUint32 sScriptBlockerCountWhereRunnersPrevented;
 
   static nsIInterfaceRequestor* sSameOriginChecker;
 
   static PRBool sIsHandlingKeyBoardEvent;
   static PRBool sAllowXULXBL_for_file;
@@ -1999,16 +2004,31 @@ public:
 
 private:
   PRUint32 mNestingLevel;
   nsCOMPtr<nsIDocument> mDocument;
   nsCOMPtr<nsIDocumentObserver> mObserver;
   MOZILLA_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
+class NS_STACK_CLASS nsAutoScriptBlockerSuppressNodeRemoved :
+                          public nsAutoScriptBlocker {
+public:
+  nsAutoScriptBlockerSuppressNodeRemoved() {
+#ifdef DEBUG
+    ++nsContentUtils::sDOMNodeRemovedSuppressCount;
+#endif
+  }
+  ~nsAutoScriptBlockerSuppressNodeRemoved() {
+#ifdef DEBUG
+    --nsContentUtils::sDOMNodeRemovedSuppressCount;
+#endif
+  }
+};
+
 #define NS_INTERFACE_MAP_ENTRY_TEAROFF(_interface, _allocator)                \
   if (aIID.Equals(NS_GET_IID(_interface))) {                                  \
     foundInterface = static_cast<_interface *>(_allocator);                   \
     if (!foundInterface) {                                                    \
       *aInstancePtr = nsnull;                                                 \
       return NS_ERROR_OUT_OF_MEMORY;                                          \
     }                                                                         \
   } else
--- a/content/base/src/nsContentUtils.cpp
+++ b/content/base/src/nsContentUtils.cpp
@@ -249,16 +249,19 @@ nsTArray<nsISupports**> *nsContentUtils:
 nsIScriptRuntime *nsContentUtils::sScriptRuntimes[NS_STID_ARRAY_UBOUND];
 PRInt32 nsContentUtils::sScriptRootCount[NS_STID_ARRAY_UBOUND];
 PRUint32 nsContentUtils::sJSGCThingRootCount;
 #ifdef IBMBIDI
 nsIBidiKeyboard *nsContentUtils::sBidiKeyboard = nsnull;
 #endif
 PRUint32 nsContentUtils::sScriptBlockerCount = 0;
 PRUint32 nsContentUtils::sRemovableScriptBlockerCount = 0;
+#ifdef DEBUG
+PRUint32 nsContentUtils::sDOMNodeRemovedSuppressCount = 0;
+#endif
 nsCOMArray<nsIRunnable>* nsContentUtils::sBlockedScriptRunners = nsnull;
 PRUint32 nsContentUtils::sRunnersCountAtFirstBlocker = 0;
 PRUint32 nsContentUtils::sScriptBlockerCountWhereRunnersPrevented = 0;
 nsIInterfaceRequestor* nsContentUtils::sSameOriginChecker = nsnull;
 
 PRBool nsContentUtils::sIsHandlingKeyBoardEvent = PR_FALSE;
 PRBool nsContentUtils::sAllowXULXBL_for_file = PR_FALSE;
 
@@ -3724,25 +3727,38 @@ nsContentUtils::HasMutationListeners(nsI
 void
 nsContentUtils::MaybeFireNodeRemoved(nsINode* aChild, nsINode* aParent,
                                      nsIDocument* aOwnerDoc)
 {
   NS_PRECONDITION(aChild, "Missing child");
   NS_PRECONDITION(aChild->GetNodeParent() == aParent, "Wrong parent");
   NS_PRECONDITION(aChild->GetOwnerDoc() == aOwnerDoc, "Wrong owner-doc");
 
+  // This checks that IsSafeToRunScript is true since we don't want to fire
+  // events when that is false. We can't rely on nsEventDispatcher to assert
+  // this in this situation since most of the time there are no mutation
+  // event listeners, in which case we won't even attempt to dispatch events.
+  // However this also allows for two exceptions. First off, we don't assert
+  // if the mutation happens to native anonymous content since we never fire
+  // mutation events on such content anyway.
+  // Second, we don't assert if sDOMNodeRemovedSuppressCount is true since
+  // that is a know case when we'd normally fire a mutation event, but can't
+  // make that safe and so we suppress it at this time. Ideally this should
+  // go away eventually.
   NS_ASSERTION(aChild->IsNodeOfType(nsINode::eCONTENT) &&
                static_cast<nsIContent*>(aChild)->
                  IsInNativeAnonymousSubtree() ||
-               IsSafeToRunScript(),
+               IsSafeToRunScript() ||
+               sDOMNodeRemovedSuppressCount,
                "Want to fire DOMNodeRemoved event, but it's not safe");
 
   // Having an explicit check here since it's an easy mistake to fall into,
   // and there might be existing code with problems. We'd rather be safe
-  // than fire DOMNodeRemoved in all corner cases.
+  // than fire DOMNodeRemoved in all corner cases. We also rely on it for
+  // nsAutoScriptBlockerSuppressNodeRemoved.
   if (!IsSafeToRunScript()) {
     return;
   }
 
   if (HasMutationListeners(aChild,
         NS_EVENT_BITS_MUTATION_NODEREMOVED, aParent)) {
     nsMutationEvent mutation(PR_TRUE, NS_MUTATION_NODEREMOVED);
     mutation.mRelatedNode = do_QueryInterface(aParent);
--- a/editor/libeditor/html/crashtests/crashtests.list
+++ b/editor/libeditor/html/crashtests/crashtests.list
@@ -16,8 +16,9 @@ load 499844-1.html
 load 503709-1.xhtml
 load 513375-1.xhtml
 load 535632-1.xhtml
 load 574558-1.xhtml
 load 582138-1.xhtml
 load 612565-1.html
 asserts(6) load 615015-1.html # Bug 439258
 load 615450-1.html
+load 643786-1.html
--- a/editor/libeditor/html/nsHTMLEditRules.cpp
+++ b/editor/libeditor/html/nsHTMLEditRules.cpp
@@ -9197,16 +9197,19 @@ nsHTMLEditRules::DocumentModified()
 
 void
 nsHTMLEditRules::DocumentModifiedWorker()
 {
   if (!mHTMLEditor) {
     return;
   }
 
+  // DeleteNode below may cause a flush, which could destroy the editor
+  nsAutoScriptBlockerSuppressNodeRemoved scriptBlocker;
+
   nsCOMPtr<nsIHTMLEditor> kungFuDeathGrip(mHTMLEditor);
   nsCOMPtr<nsISelection> selection;
   nsresult res = mHTMLEditor->GetSelection(getter_AddRefs(selection));
   NS_ENSURE_SUCCESS(res, );
 
   // Delete our bogus node, if we have one, since the document might not be
   // empty any more.
   if (mBogusNode) {