Fix for 52808 -- standardized nsEditorShell creation and destruction across components, to fix editor observer notification timing. r=kin,ducarroz,jelwell, sr=hyatt.

editor/base/nsEditor.cpp

@@ -919,9 +919,6 @@ nsEditor::nsEditor()
 
 nsEditor::~nsEditor()
 {
-  // not sure if this needs to be called earlier.
-  NotifyDocumentListeners(eDocumentToBeDestroyed);
-
   delete mEditorObservers;   // no need to release observers; we didn't addref them
   mEditorObservers = 0;
   
@@ -1040,6 +1037,15 @@ nsEditor::PostCreate()
   return NS_OK;
 }
 
+
+NS_IMETHODIMP
+nsEditor::PreDestroy()
+{
+  // tell our listeners that the doc is going away
+  NotifyDocumentListeners(eDocumentToBeDestroyed);
+  return NS_OK;
+}
+
 NS_IMETHODIMP 
 nsEditor::GetDocument(nsIDOMDocument **aDoc)
 {

editor/base/nsEditor.h

@@ -215,6 +215,7 @@ public:
   /* ------------ nsIEditor methods -------------- */
   NS_IMETHOD Init(nsIDOMDocument *aDoc, nsIPresShell *aPresShell, nsIContent *aRoot, nsISelectionController *aSelCon, PRUint32 aFlags);
   NS_IMETHOD PostCreate();
+  NS_IMETHOD PreDestroy();
   NS_IMETHOD GetFlags(PRUint32 *aFlags) = 0;
   NS_IMETHOD SetFlags(PRUint32 aFlags) = 0;
   NS_IMETHOD GetDocument(nsIDOMDocument **aDoc);

editor/base/nsEditorShell.cpp

@@ -249,6 +249,7 @@ nsEditorShell::nsEditorShell()
 ,  mEditorController(nsnull)
 ,  mDocShell(nsnull)
 ,  mContentAreaDocShell(nsnull)
+,  mInitted(PR_FALSE)
 ,  mCloseWindowWhenLoaded(PR_FALSE)
 ,  mCantEditReason(eCantEditNoReason)
 ,  mEditorType(eUninitializedEditorType)
@@ -308,7 +309,11 @@ nsEditorShell::QueryInterface(REFNSIID aIID,void** aInstancePtr)
 
 NS_IMETHODIMP    
 nsEditorShell::Init()
-{  
+{
+  NS_ASSERTION(!mInitted, "Double init of nsEditorShell detected");
+  if (mInitted)
+    return NS_OK;
+  
   nsAutoString    editorType; editorType.AssignWithConversion("html");      // default to creating HTML editor
   mEditorTypeString = editorType;
   mEditorTypeString.ToLowerCase();
@@ -325,7 +330,8 @@ nsEditorShell::Init()
 
   // XXX: why are we returning NS_OK here rather than res?
   // is it ok to fail to get a string bundle?  if so, it should be documented.
-
+  mInitted = PR_TRUE;
+  
   return NS_OK;
 }
 
@@ -333,6 +339,13 @@ NS_IMETHODIMP
 nsEditorShell::Shutdown()
 {
   nsresult rv = NS_OK;
+  
+  nsCOMPtr<nsIEditor> editor(do_QueryInterface(mEditor));
+  if (editor)
+  {
+    editor->PreDestroy();
+  }
+  
   // Remove our document mouse event listener
   if (mMouseListenerP)
   {

editor/base/nsEditorShell.h

@@ -212,6 +212,7 @@ class nsEditorShell :   public nsIEditorShell,
     // if we are in a frameset, this assumption is false.
     nsIDocShell         *mContentAreaDocShell;  // weak reference
 
+    PRPackedBool        mInitted;
     PRPackedBool        mCloseWindowWhenLoaded; // error on load. Close window when loaded
     ECantEditReason     mCantEditReason;
     

editor/composer/src/nsEditorShell.cpp

@@ -249,6 +249,7 @@ nsEditorShell::nsEditorShell()
 ,  mEditorController(nsnull)
 ,  mDocShell(nsnull)
 ,  mContentAreaDocShell(nsnull)
+,  mInitted(PR_FALSE)
 ,  mCloseWindowWhenLoaded(PR_FALSE)
 ,  mCantEditReason(eCantEditNoReason)
 ,  mEditorType(eUninitializedEditorType)
@@ -308,7 +309,11 @@ nsEditorShell::QueryInterface(REFNSIID aIID,void** aInstancePtr)
 
 NS_IMETHODIMP    
 nsEditorShell::Init()
-{  
+{
+  NS_ASSERTION(!mInitted, "Double init of nsEditorShell detected");
+  if (mInitted)
+    return NS_OK;
+  
   nsAutoString    editorType; editorType.AssignWithConversion("html");      // default to creating HTML editor
   mEditorTypeString = editorType;
   mEditorTypeString.ToLowerCase();
@@ -325,7 +330,8 @@ nsEditorShell::Init()
 
   // XXX: why are we returning NS_OK here rather than res?
   // is it ok to fail to get a string bundle?  if so, it should be documented.
-
+  mInitted = PR_TRUE;
+  
   return NS_OK;
 }
 
@@ -333,6 +339,13 @@ NS_IMETHODIMP
 nsEditorShell::Shutdown()
 {
   nsresult rv = NS_OK;
+  
+  nsCOMPtr<nsIEditor> editor(do_QueryInterface(mEditor));
+  if (editor)
+  {
+    editor->PreDestroy();
+  }
+  
   // Remove our document mouse event listener
   if (mMouseListenerP)
   {

editor/composer/src/nsEditorShell.h

@@ -212,6 +212,7 @@ class nsEditorShell :   public nsIEditorShell,
     // if we are in a frameset, this assumption is false.
     nsIDocShell         *mContentAreaDocShell;  // weak reference
 
+    PRPackedBool        mInitted;
     PRPackedBool        mCloseWindowWhenLoaded; // error on load. Close window when loaded
     ECantEditReason     mCantEditReason;
     

editor/libeditor/base/nsEditor.cpp

@@ -919,9 +919,6 @@ nsEditor::nsEditor()
 
 nsEditor::~nsEditor()
 {
-  // not sure if this needs to be called earlier.
-  NotifyDocumentListeners(eDocumentToBeDestroyed);
-
   delete mEditorObservers;   // no need to release observers; we didn't addref them
   mEditorObservers = 0;
   
@@ -1040,6 +1037,15 @@ nsEditor::PostCreate()
   return NS_OK;
 }
 
+
+NS_IMETHODIMP
+nsEditor::PreDestroy()
+{
+  // tell our listeners that the doc is going away
+  NotifyDocumentListeners(eDocumentToBeDestroyed);
+  return NS_OK;
+}
+
 NS_IMETHODIMP 
 nsEditor::GetDocument(nsIDOMDocument **aDoc)
 {

editor/libeditor/base/nsEditor.h

@@ -215,6 +215,7 @@ public:
   /* ------------ nsIEditor methods -------------- */
   NS_IMETHOD Init(nsIDOMDocument *aDoc, nsIPresShell *aPresShell, nsIContent *aRoot, nsISelectionController *aSelCon, PRUint32 aFlags);
   NS_IMETHOD PostCreate();
+  NS_IMETHOD PreDestroy();
   NS_IMETHOD GetFlags(PRUint32 *aFlags) = 0;
   NS_IMETHOD SetFlags(PRUint32 aFlags) = 0;
   NS_IMETHOD GetDocument(nsIDOMDocument **aDoc);

editor/public/nsIEditor.h

@@ -85,6 +85,12 @@ public:
    */
   NS_IMETHOD PostCreate()=0;
 
+  /**
+   * PreDestroy is called before the editor goes away, and gives the editor a chance
+   * to tell its documentStateObservers that the document is going away.
+   */
+  NS_IMETHOD PreDestroy()=0;
+
   /** return the edit flags for this editor */
   NS_IMETHOD GetFlags(PRUint32 *aFlags)=0;
 

editor/ui/composer/content/editor.js

@@ -168,8 +168,6 @@ function EditorStartup(editorType, editorElement)
 
   // store the editor shell in the window, so that child windows can get to it.
   editorShell = editorElement.editorShell;        // this pattern exposes a JS/XBL bug that causes leaks
-  
-  editorShell.Init();
   editorShell.editorType = editorType;
 
   editorShell.webShellWindow = window;
@@ -284,8 +282,7 @@ function _EditorNotImplemented()
 
 function EditorShutdown()
 {
-  dump("In EditorShutdown..\n");
-  return editorShell.Shutdown();
+    // nothing to do. editorShell->Shutdown is called by the nsEditorBoxObject
 }
 
 function SafeSetAttribute(nodeID, attributeName, attributeValue)

layout/html/forms/src/nsGfxTextControlFrame2.cpp

@@ -1089,6 +1089,12 @@ nsGfxTextControlFrame2::~nsGfxTextControlFrame2()
 NS_IMETHODIMP
 nsGfxTextControlFrame2::Destroy(nsIPresContext* aPresContext)
 {
+  // notify the editor that we are going away
+  if (mEditor)
+  {
+    mEditor->PreDestroy();
+  }
+  
   // Clean up the controller
   nsCOMPtr<nsIControllers> controllers;
   nsCOMPtr<nsIDOMNSHTMLInputElement> inputElement = do_QueryInterface(mContent);
@@ -1143,15 +1149,6 @@ nsGfxTextControlFrame2::Destroy(nsIPresContext* aPresContext)
       erP->RemoveEventListenerByIID(NS_STATIC_CAST(nsIDOMFocusListener  *,mTextListener), NS_GET_IID(nsIDOMFocusListener));
       erP->RemoveEventListenerByIID(NS_STATIC_CAST(nsIDOMKeyListener*,mTextListener), NS_GET_IID(nsIDOMKeyListener));
     }
-    nsCOMPtr<nsIPresShell> shell;
-    nsresult rv = aPresContext->GetShell(getter_AddRefs(shell));
-    if (NS_FAILED(rv) || !shell)
-      return rv?rv:NS_ERROR_FAILURE;
-
-    if (mEditor)
-    {
-      mEditor->RemoveEditorObserver(mTextListener);
-    }
   }
   return nsBoxFrame::Destroy(aPresContext);
 }

layout/xul/base/src/nsEditorBoxObject.cpp

@@ -37,6 +37,7 @@ public:
   nsEditorBoxObject();
   virtual ~nsEditorBoxObject();
 //NS_PIBOXOBJECT interfaces
+  NS_IMETHOD Init(nsIContent* aContent, nsIPresShell* aPresShell);
   NS_IMETHOD SetDocument(nsIDocument* aDocument);
   
 protected:
@@ -44,32 +45,27 @@ protected:
 };
 
 /* Implementation file */
-NS_IMPL_ADDREF(nsEditorBoxObject)
-NS_IMPL_RELEASE(nsEditorBoxObject)
+NS_IMPL_ISUPPORTS_INHERITED(nsEditorBoxObject, nsBoxObject, nsIEditorBoxObject)
 
 
 NS_IMETHODIMP
 nsEditorBoxObject::SetDocument(nsIDocument* aDocument)
 {
+  nsresult rv;
+  
+  if (mEditorShell)
+  {
+    rv = mEditorShell->Shutdown();
+    NS_ASSERTION(NS_SUCCEEDED(rv), "Error from editorShell->Shutdown");
+  }
+  
+  // this should only be called with a null document, which indicates
+  // that we're being torn down.
+  NS_ASSERTION(aDocument == nsnull, "SetDocument called with non-null document");
   mEditorShell = nsnull;
   return nsBoxObject::SetDocument(aDocument);
 }
 
-
-NS_IMETHODIMP 
-nsEditorBoxObject::QueryInterface(REFNSIID iid, void** aResult)
-{
-  if (!aResult)
-    return NS_ERROR_NULL_POINTER;
-  
-  if (iid.Equals(NS_GET_IID(nsIEditorBoxObject))) {
-    *aResult = (nsIEditorBoxObject*)this;
-    NS_ADDREF(this);
-    return NS_OK;
-  }
-
-  return nsBoxObject::QueryInterface(iid, aResult);
-}
   
 nsEditorBoxObject::nsEditorBoxObject()
 {
@@ -81,12 +77,27 @@ nsEditorBoxObject::~nsEditorBoxObject()
   /* destructor code */
 }
 
-/* void openEditor (in boolean openFlag); */
+
+NS_IMETHODIMP nsEditorBoxObject::Init(nsIContent* aContent, nsIPresShell* aPresShell)
+{
+  nsresult rv = nsBoxObject::Init(aContent, aPresShell);
+  if (NS_FAILED(rv)) return rv;
+  
+  NS_ASSERTION(!mEditorShell, "Double init of nsEditorBoxObject");
+  
+  mEditorShell = do_CreateInstance("@mozilla.org/editor/editorshell;1");
+  if (!mEditorShell) return NS_ERROR_OUT_OF_MEMORY;
+
+  rv = mEditorShell->Init();
+  if (NS_FAILED(rv)) return rv;
+  
+  return NS_OK;
+}
+
+
 NS_IMETHODIMP nsEditorBoxObject::GetEditorShell(nsIEditorShell** aResult)
 {
-  if (!mEditorShell) {
-    mEditorShell = do_CreateInstance("@mozilla.org/editor/editorshell;1");
-  }
+  NS_ASSERTION(mEditorShell, "Editor box object not initted");
 
   *aResult = mEditorShell;
   NS_IF_ADDREF(*aResult);

mailnews/compose/resources/content/MsgComposeCommands.js

@@ -614,8 +614,13 @@ function ComposeStartup()
 		if (msgCompose)
 		{
 			//Creating a Editor Shell
-      var editorShell = Components.classes["@mozilla.org/editor/editorshell;1"].createInstance();
-      editorShell = editorShell.QueryInterface(Components.interfaces.nsIEditorShell);
+			var editorElement = document.getElementById("content-frame");
+			if (!editorElement)
+			{
+				dump("Failed to get editor element!\n");
+				return;
+			}
+			var editorShell = editorElement.editorShell;
 			if (!editorShell)
 			{
 				dump("Failed to create editorShell!\n");
@@ -624,7 +629,6 @@ function ComposeStartup()
 			
 			// save the editorShell in the window. The editor JS expects to find it there.
 			window.editorShell = editorShell;
-			window.editorShell.Init();
 			dump("Created editorShell\n");
 
 			// setEditorType MUST be call before setContentWindow
@@ -648,8 +652,8 @@ function ComposeStartup()
 			window.editorShell.webShellWindow = window;
 			window.editorShell.contentWindow = window._content;
 
-      // Do setup common to Message Composer and Web Composer
-      EditorSharedStartup();
+			// Do setup common to Message Composer and Web Composer
+			EditorSharedStartup();
 
 	    	var msgCompFields = msgCompose.compFields;
 	    	if (msgCompFields)
@@ -753,7 +757,6 @@ function ComposeUnload(calledFromExit)
 	msgCompose.UnregisterStateListener(stateListener);
 	if (msgCompose && msgComposeService)
 		msgComposeService.DisposeCompose(msgCompose, false);
-	//...and what's about the editor appcore, how can we release it?
 }
 
 function SetDocumentCharacterSet(aCharset)