Bug 726334, FormFillController should not have strong references to content objects, r=gavin
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Wed, 22 Feb 2012 14:59:39 +0200
changeset 87316 2d065305b7d25191eab42473e29b008c285e2a5e
parent 87315 373c710112e60a9761a6a8e569fe7b80831406a2
child 87317 a227041e66ad6b8fe37bb5212deb66e1ab1b4f07
push id22112
push useropettay@mozilla.com
push dateWed, 22 Feb 2012 13:16:00 +0000
treeherdermozilla-central@2d065305b7d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgavin
bugs726334
milestone13.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 726334, FormFillController should not have strong references to content objects, r=gavin
toolkit/components/satchel/nsFormFillController.cpp
toolkit/components/satchel/nsFormFillController.h
--- a/toolkit/components/satchel/nsFormFillController.cpp
+++ b/toolkit/components/satchel/nsFormFillController.cpp
@@ -76,33 +76,56 @@
 NS_IMPL_ISUPPORTS5(nsFormFillController,
                    nsIFormFillController,
                    nsIAutoCompleteInput,
                    nsIAutoCompleteSearch,
                    nsIDOMEventListener,
                    nsIMutationObserver)
 
 nsFormFillController::nsFormFillController() :
+  mFocusedInput(nsnull),
+  mFocusedInputNode(nsnull),
+  mListNode(nsnull),
   mTimeout(50),
   mMinResultsForPopup(1),
   mMaxRows(0),
   mDisableAutoComplete(false),
   mCompleteDefaultIndex(false),
   mCompleteSelectedIndex(false),
   mForceComplete(false),
   mSuppressOnInput(false)
 {
   mController = do_GetService("@mozilla.org/autocomplete/controller;1");
   mDocShells = do_CreateInstance("@mozilla.org/supports-array;1");
   mPopups = do_CreateInstance("@mozilla.org/supports-array;1");
   mPwmgrInputs.Init();
 }
 
+struct PwmgrInputsEnumData
+{
+  PwmgrInputsEnumData(nsIMutationObserver* aMutationObserver, nsIDocument* aDoc)
+  : mMutationObserver(aMutationObserver), mDoc(aDoc) {}
+
+  nsIMutationObserver* mMutationObserver;
+  nsCOMPtr<nsIDocument> mDoc;
+};
+
 nsFormFillController::~nsFormFillController()
 {
+  PwmgrInputsEnumData ed(this, nsnull);
+  mPwmgrInputs.Enumerate(RemoveForDocumentEnumerator, &ed);
+  if (mListNode) {
+    mListNode->RemoveMutationObserver(this);
+    mListNode = nsnull;
+  }
+  if (mFocusedInputNode) {
+    mFocusedInputNode->RemoveMutationObserver(this);
+    mFocusedInputNode = nsnull;
+    mFocusedInput = nsnull;
+  }
   // Remove ourselves as a focus listener from all cached docShells
   PRUint32 count;
   mDocShells->Count(&count);
   for (PRUint32 i = 0; i < count; ++i) {
     nsCOMPtr<nsIDocShell> docShell;
     mDocShells->GetElementAt(i, getter_AddRefs(docShell));
     nsCOMPtr<nsIDOMWindow> domWindow = GetWindowForDocShell(docShell);
     RemoveWindowListeners(domWindow);
@@ -114,45 +137,53 @@ nsFormFillController::~nsFormFillControl
 //
 
 void
 nsFormFillController::AttributeChanged(nsIDocument* aDocument,
                                        mozilla::dom::Element* aElement,
                                        PRInt32 aNameSpaceID,
                                        nsIAtom* aAttribute, PRInt32 aModType)
 {
-  RevalidateDataList();
+  if (mListNode && mListNode->Contains(aElement)) {
+    RevalidateDataList();
+  }
 }
 
 void
 nsFormFillController::ContentAppended(nsIDocument* aDocument,
                                       nsIContent* aContainer,
                                       nsIContent* aChild,
                                       PRInt32 aIndexInContainer)
 {
-  RevalidateDataList();
+  if (mListNode && mListNode->Contains(aContainer)) {
+    RevalidateDataList();
+  }
 }
 
 void
 nsFormFillController::ContentInserted(nsIDocument* aDocument,
                                       nsIContent* aContainer,
                                       nsIContent* aChild,
                                       PRInt32 aIndexInContainer)
 {
-  RevalidateDataList();
+  if (mListNode && mListNode->Contains(aContainer)) {
+    RevalidateDataList();
+  }
 }
 
 void
 nsFormFillController::ContentRemoved(nsIDocument* aDocument,
                                      nsIContent* aContainer,
                                      nsIContent* aChild,
                                      PRInt32 aIndexInContainer,
                                      nsIContent* aPreviousSibling)
 {
-  RevalidateDataList();
+  if (mListNode && mListNode->Contains(aContainer)) {
+    RevalidateDataList();
+  }
 }
 
 void
 nsFormFillController::CharacterDataWillChange(nsIDocument* aDocument,
                                               nsIContent* aContent,
                                               CharacterDataChangeInfo* aInfo)
 {
 }
@@ -175,16 +206,24 @@ nsFormFillController::AttributeWillChang
 void
 nsFormFillController::ParentChainChanged(nsIContent* aContent)
 {
 }
 
 void
 nsFormFillController::NodeWillBeDestroyed(const nsINode* aNode)
 {
+  mPwmgrInputs.Remove(aNode);
+  if (aNode == mListNode) {
+    mListNode = nsnull;
+    RevalidateDataList();
+  } else if (aNode == mFocusedInputNode) {
+    mFocusedInputNode = nsnull;
+    mFocusedInput = nsnull;
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////
 //// nsIFormFillController
 
 NS_IMETHODIMP
 nsFormFillController::AttachToBrowser(nsIDocShell *aDocShell, nsIAutoCompletePopup *aPopup)
 {
@@ -224,17 +263,20 @@ nsFormFillController::MarkAsLoginManager
 {
   /*
    * The Login Manager can supply autocomplete results for username fields,
    * when a user has multiple logins stored for a site. It uses this
    * interface to indicate that the form manager shouldn't handle the
    * autocomplete. The form manager also checks for this tag when saving
    * form history (so it doesn't save usernames).
    */
-  mPwmgrInputs.Put(aInput, 1);
+  nsCOMPtr<nsINode> node = do_QueryInterface(aInput);
+  NS_ENSURE_STATE(node);
+  mPwmgrInputs.Put(node, true);
+  node->AddMutationObserverUnlessExists(this);
 
   if (!mLoginManager)
     mLoginManager = do_GetService("@mozilla.org/login-manager;1");
 
   return NS_OK;
 }
 
 
@@ -559,18 +601,18 @@ NS_IMETHODIMP
 nsFormFillController::StartSearch(const nsAString &aSearchString, const nsAString &aSearchParam,
                                   nsIAutoCompleteResult *aPreviousResult, nsIAutoCompleteObserver *aListener)
 {
   nsresult rv;
   nsCOMPtr<nsIAutoCompleteResult> result;
 
   // If the login manager has indicated it's responsible for this field, let it
   // handle the autocomplete. Otherwise, handle with form history.
-  PRInt32 dummy;
-  if (mPwmgrInputs.Get(mFocusedInput, &dummy)) {
+  bool dummy;
+  if (mPwmgrInputs.Get(mFocusedInputNode, &dummy)) {
     // XXX aPreviousResult shouldn't ever be a historyResult type, since we're not letting
     // satchel manage the field?
     rv = mLoginManager->AutoCompleteSearch(aSearchString,
                                          aPreviousResult,
                                          mFocusedInput,
                                          getter_AddRefs(result));
   } else {
     nsCOMPtr<nsIAutoCompleteResult> formHistoryResult;
@@ -604,18 +646,25 @@ nsFormFillController::StartSearch(const 
                                                    mFocusedInput,
                                                    getter_AddRefs(result));
 
     if (mFocusedInput) {
       nsCOMPtr<nsIDOMHTMLElement> list;
       mFocusedInput->GetList(getter_AddRefs(list));
 
       nsCOMPtr<nsINode> node = do_QueryInterface(list);
-      if(node) {
-        node->AddMutationObserverUnlessExists(this);
+      if (mListNode != node) {
+        if (mListNode) {
+          mListNode->RemoveMutationObserver(this);
+          mListNode = nsnull;
+        }
+        if (node) {
+          node->AddMutationObserverUnlessExists(this);
+          mListNode = node;
+        }
       }
     }
   }
   NS_ENSURE_SUCCESS(rv, rv);
 
   aListener->OnSearchResult(this, result);
 
   return NS_OK;
@@ -642,16 +691,19 @@ public:
 private:
   nsCOMPtr<nsIAutoCompleteObserver> mObserver;
   nsCOMPtr<nsIAutoCompleteSearch> mSearch;
   nsCOMPtr<nsIAutoCompleteResult> mResult;
 };
 
 void nsFormFillController::RevalidateDataList()
 {
+  if (!mLastListener) {
+    return;
+  }
   nsresult rv;
   nsCOMPtr <nsIInputListAutoComplete> inputListAutoComplete =
     do_GetService("@mozilla.org/satchel/inputlist-autocomplete;1", &rv);
 
   nsCOMPtr<nsIAutoCompleteResult> result;
 
   rv = inputListAutoComplete->AutoCompleteSearch(mLastSearchResult,
                                                  mLastSearchString,
@@ -723,60 +775,61 @@ nsFormFillController::HandleEvent(nsIDOM
 
     if (mFocusedInput) {
       nsCOMPtr<nsIDOMDocument> inputDoc;
       mFocusedInput->GetOwnerDocument(getter_AddRefs(inputDoc));
       if (domDoc == inputDoc)
         StopControllingInput();
     }
 
-    mPwmgrInputs.Enumerate(RemoveForDOMDocumentEnumerator, domDoc);
+    nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);
+    PwmgrInputsEnumData ed(this, doc);
+    mPwmgrInputs.Enumerate(RemoveForDocumentEnumerator, &ed);
   }
 
   return NS_OK;
 }
 
 
 /* static */ PLDHashOperator
-nsFormFillController::RemoveForDOMDocumentEnumerator(nsISupports* aKey,
-                                                  PRInt32& aEntry,
+nsFormFillController::RemoveForDocumentEnumerator(const nsINode* aKey,
+                                                  bool& aEntry,
                                                   void* aUserData)
 {
-  nsIDOMDocument* domDoc = static_cast<nsIDOMDocument*>(aUserData);
-  nsCOMPtr<nsIDOMHTMLInputElement> element = do_QueryInterface(aKey);
-  nsCOMPtr<nsIDOMDocument> elementDoc;
-  element->GetOwnerDocument(getter_AddRefs(elementDoc));
-  if (elementDoc == domDoc)
+  PwmgrInputsEnumData* ed = static_cast<PwmgrInputsEnumData*>(aUserData);
+  if (aKey && (!ed->mDoc || aKey->OwnerDoc() == ed->mDoc)) {
+    const_cast<nsINode*>(aKey)->RemoveMutationObserver(ed->mMutationObserver);
     return PL_DHASH_REMOVE;
-
+  }
   return PL_DHASH_NEXT;
 }
 
 nsresult
 nsFormFillController::Focus(nsIDOMEvent* aEvent)
 {
   nsCOMPtr<nsIDOMEventTarget> target;
   aEvent->GetTarget(getter_AddRefs(target));
 
   nsCOMPtr<nsIDOMHTMLInputElement> input = do_QueryInterface(target);
-  if (!input)
+  nsCOMPtr<nsINode> inputNode = do_QueryInterface(input); 
+  if (!inputNode)
     return NS_OK;
 
   bool isReadOnly = false;
   input->GetReadOnly(&isReadOnly);
 
   bool autocomplete = nsContentUtils::IsAutocompleteEnabled(input);
 
   nsCOMPtr<nsIDOMHTMLElement> datalist;
   input->GetList(getter_AddRefs(datalist));
   bool hasList = datalist != nsnull;
 
-  PRInt32 dummy;
+  bool dummy;
   bool isPwmgrInput = false;
-  if (mPwmgrInputs.Get(input, &dummy))
+  if (mPwmgrInputs.Get(inputNode, &dummy))
       isPwmgrInput = true;
 
   nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(input);
   if (isPwmgrInput || (formControl &&
                        formControl->IsSingleLineTextControl(PR_TRUE) &&
                        (hasList || autocomplete) && !isReadOnly)) {
     StartControllingInput(input);
   }
@@ -944,17 +997,19 @@ nsFormFillController::RemoveWindowListen
 {
   if (!aWindow)
     return;
 
   StopControllingInput();
 
   nsCOMPtr<nsIDOMDocument> domDoc;
   aWindow->GetDocument(getter_AddRefs(domDoc));
-  mPwmgrInputs.Enumerate(RemoveForDOMDocumentEnumerator, domDoc);
+  nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);
+  PwmgrInputsEnumData ed(this, doc);
+  mPwmgrInputs.Enumerate(RemoveForDocumentEnumerator, &ed);
 
   nsCOMPtr<nsPIDOMWindow> privateDOMWindow(do_QueryInterface(aWindow));
   nsIDOMEventTarget* target = nsnull;
   if (privateDOMWindow)
     target = privateDOMWindow->GetChromeEventHandler();
 
   if (!target)
     return;
@@ -1003,47 +1058,62 @@ nsFormFillController::StartControllingIn
   nsCOMPtr<nsIDocShell> docShell = GetDocShellForInput(aInput);
   PRInt32 index = GetIndexOfDocShell(docShell);
   if (index < 0)
     return;
 
   // Cache the popup for the focused docShell
   mPopups->GetElementAt(index, getter_AddRefs(mFocusedPopup));
 
+  nsCOMPtr<nsINode> node = do_QueryInterface(aInput);
+  if (!node) {
+    return;
+  }
+
   AddKeyListener(aInput);
+  
+  node->AddMutationObserverUnlessExists(this);
+  mFocusedInputNode = node;
   mFocusedInput = aInput;
 
+  nsCOMPtr<nsIDOMHTMLElement> list;
+  mFocusedInput->GetList(getter_AddRefs(list));
+  nsCOMPtr<nsINode> listNode = do_QueryInterface(list);
+  if (listNode) {
+    listNode->AddMutationObserverUnlessExists(this);
+    mListNode = listNode;
+  }
+
   // Now we are the autocomplete controller's bitch
   mController->SetInput(this);
 }
 
 void
 nsFormFillController::StopControllingInput()
 {
   RemoveKeyListener();
 
-  if(mFocusedInput) {
-    nsCOMPtr<nsIDOMHTMLElement> list;
-    mFocusedInput->GetList(getter_AddRefs(list));
-
-    nsCOMPtr<nsINode> node = do_QueryInterface(list);
-    if (node) {
-      node->RemoveMutationObserver(this);
-    }
+  if (mListNode) {
+    mListNode->RemoveMutationObserver(this);
+    mListNode = nsnull;
   }
 
   // Reset the controller's input, but not if it has been switched
   // to another input already, which might happen if the user switches
   // focus by clicking another autocomplete textbox
   nsCOMPtr<nsIAutoCompleteInput> input;
   mController->GetInput(getter_AddRefs(input));
   if (input == this)
     mController->SetInput(nsnull);
 
-  mFocusedInput = nsnull;
+  if (mFocusedInputNode) {
+    mFocusedInputNode->RemoveMutationObserver(this);
+    mFocusedInputNode = nsnull;
+    mFocusedInput = nsnull;
+  }
   mFocusedPopup = nsnull;
 }
 
 nsIDocShell *
 nsFormFillController::GetDocShellForInput(nsIDOMHTMLInputElement *aInput)
 {
   nsCOMPtr<nsIDOMDocument> domDoc;
   aInput->GetOwnerDocument(getter_AddRefs(domDoc));
--- a/toolkit/components/satchel/nsFormFillController.h
+++ b/toolkit/components/satchel/nsFormFillController.h
@@ -56,16 +56,17 @@
 #include "nsIMutationObserver.h"
 
 // X.h defines KeyPress
 #ifdef KeyPress
 #undef KeyPress
 #endif
 
 class nsFormHistory;
+class nsINode;
 
 class nsFormFillController : public nsIFormFillController,
                              public nsIAutoCompleteInput,
                              public nsIAutoCompleteSearch,
                              public nsIDOMEventListener,
                              public nsIMutationObserver
 {
 public:
@@ -95,36 +96,38 @@ protected:
 
   void RevalidateDataList();
   bool RowMatch(nsFormHistory *aHistory, PRUint32 aIndex, const nsAString &aInputName, const nsAString &aInputValue);
 
   inline nsIDocShell *GetDocShellForInput(nsIDOMHTMLInputElement *aInput);
   inline nsIDOMWindow *GetWindowForDocShell(nsIDocShell *aDocShell);
   inline PRInt32 GetIndexOfDocShell(nsIDocShell *aDocShell);
 
-  static PLDHashOperator RemoveForDOMDocumentEnumerator(nsISupports* aKey,
-                                                        PRInt32& aEntry,
-                                                        void* aUserData);
+  static PLDHashOperator RemoveForDocumentEnumerator(const nsINode* aKey,
+                                                     bool& aEntry,
+                                                     void* aUserData);
   bool IsEventTrusted(nsIDOMEvent *aEvent);
   // members //////////////////////////////////////////
 
   nsCOMPtr<nsIAutoCompleteController> mController;
   nsCOMPtr<nsILoginManager> mLoginManager;
-  nsCOMPtr<nsIDOMHTMLInputElement> mFocusedInput;
+  nsIDOMHTMLInputElement* mFocusedInput;
+  nsINode* mFocusedInputNode;
+  nsINode* mListNode;
   nsCOMPtr<nsIAutoCompletePopup> mFocusedPopup;
 
   nsCOMPtr<nsISupportsArray> mDocShells;
   nsCOMPtr<nsISupportsArray> mPopups;
 
   //these are used to dynamically update the autocomplete
   nsCOMPtr<nsIAutoCompleteResult> mLastSearchResult;
   nsCOMPtr<nsIAutoCompleteObserver> mLastListener;
   nsString mLastSearchString;
 
-  nsDataHashtable<nsISupportsHashKey,PRInt32> mPwmgrInputs;
+  nsDataHashtable<nsPtrHashKey<const nsINode>, bool> mPwmgrInputs;
 
   PRUint32 mTimeout;
   PRUint32 mMinResultsForPopup;
   PRUint32 mMaxRows;
   bool mDisableAutoComplete;
   bool mCompleteDefaultIndex;
   bool mCompleteSelectedIndex;
   bool mForceComplete;