From 3b7a3c96c41f54630d0cab945aedf5275bbb2918 Mon Sep 17 00:00:00 2001 From: "Carsten \"Tomcat\" Book" Date: Tue, 29 Sep 2015 13:58:35 +0200 Subject: [PATCH] Backed out 2 changesets (bug 1205983) for memory leaks on a CLOSED TREE Backed out changeset f2c49c0ab84f (bug 1205983) Backed out changeset a81630dba992 (bug 1205983) --- editor/composer/nsEditorSpellCheck.cpp | 7 + editor/composer/test/chrome.ini | 1 - editor/composer/test/test_bug1205983.html | 122 ------------------ editor/composer/test/test_bug697981.html | 5 - editor/libeditor/nsEditor.cpp | 71 ++++++++++ editor/libeditor/nsEditor.h | 8 ++ editor/libeditor/nsEditorEventListener.cpp | 4 + editor/nsIEditorSpellCheck.idl | 8 +- editor/txtsvc/nsISpellChecker.h | 6 + .../spellcheck/hunspell/glue/mozHunspell.cpp | 21 +-- .../idl/mozISpellCheckingEngine.idl | 2 + .../spellcheck/src/mozInlineSpellChecker.cpp | 6 +- extensions/spellcheck/src/mozSpellChecker.cpp | 25 ++++ extensions/spellcheck/src/mozSpellChecker.h | 1 + .../chrome/test_add_remove_dictionaries.xul | 12 +- 15 files changed, 150 insertions(+), 149 deletions(-) delete mode 100644 editor/composer/test/test_bug1205983.html diff --git a/editor/composer/nsEditorSpellCheck.cpp b/editor/composer/nsEditorSpellCheck.cpp index f20a66f9e538..c9292b9796bf 100644 --- a/editor/composer/nsEditorSpellCheck.cpp +++ b/editor/composer/nsEditorSpellCheck.cpp @@ -636,6 +636,13 @@ nsEditorSpellCheck::SetCurrentDictionary(const nsAString& aDictionary) return mSpellChecker->SetCurrentDictionary(aDictionary); } +NS_IMETHODIMP +nsEditorSpellCheck::CheckCurrentDictionary() +{ + mSpellChecker->CheckCurrentDictionary(); + return NS_OK; +} + NS_IMETHODIMP nsEditorSpellCheck::UninitSpellChecker() { diff --git a/editor/composer/test/chrome.ini b/editor/composer/test/chrome.ini index cbf01740a71a..6ae8cae9038e 100644 --- a/editor/composer/test/chrome.ini +++ b/editor/composer/test/chrome.ini @@ -9,4 +9,3 @@ skip-if = buildapp == 'b2g' || os == 'android' [test_bug717433.html] [test_bug1204147.html] [test_bug1200533.html] -[test_bug1205983.html] diff --git a/editor/composer/test/test_bug1205983.html b/editor/composer/test/test_bug1205983.html deleted file mode 100644 index 58f0f7a1857c..000000000000 --- a/editor/composer/test/test_bug1205983.html +++ /dev/null @@ -1,122 +0,0 @@ - - - - - Test for Bug 1205983 - - - - -Mozilla Bug 1205983 -

- - -
German heute ist ein guter Tag
- - -
-
-
- - diff --git a/editor/composer/test/test_bug697981.html b/editor/composer/test/test_bug697981.html index 96cc5942498f..ba9778e0a8ee 100644 --- a/editor/composer/test/test_bug697981.html +++ b/editor/composer/test/test_bug697981.html @@ -98,11 +98,6 @@ function enFocus() { // Remove the fake de_DE dictionary again. hunspell.removeDirectory(de_DE); - // Focus again, so the spelling gets updated, but before we need to kill the focus handler. - elem_de.onfocus = null; - elem_de.blur(); - elem_de.focus(); - // After removal, the de_DE editor should refresh the spelling with en-US. onSpellCheck(elem_de, function () { spellchecker = inlineSpellChecker.spellChecker; diff --git a/editor/libeditor/nsEditor.cpp b/editor/libeditor/nsEditor.cpp index d4557baf5453..bbf97f051416 100644 --- a/editor/libeditor/nsEditor.cpp +++ b/editor/libeditor/nsEditor.cpp @@ -24,6 +24,7 @@ #include "PlaceholderTxn.h" // for PlaceholderTxn #include "SplitNodeTxn.h" // for SplitNodeTxn #include "mozFlushType.h" // for mozFlushType::Flush_Frames +#include "mozISpellCheckingEngine.h" #include "mozInlineSpellChecker.h" // for mozInlineSpellChecker #include "mozilla/CheckedInt.h" // for CheckedInt #include "mozilla/IMEStateManager.h" // for IMEStateManager @@ -78,6 +79,7 @@ #include "nsIInlineSpellChecker.h" // for nsIInlineSpellChecker, etc #include "nsNameSpaceManager.h" // for kNameSpaceID_None, etc #include "nsINode.h" // for nsINode, etc +#include "nsIObserverService.h" // for nsIObserverService #include "nsIPlaintextEditor.h" // for nsIPlaintextEditor, etc #include "nsIPresShell.h" // for nsIPresShell #include "nsISelectionController.h" // for nsISelectionController, etc @@ -146,6 +148,7 @@ nsEditor::nsEditor() , mDispatchInputEvent(true) , mIsInEditAction(false) , mHidingCaret(false) +, mObservingDictionaryUpdates(false) { } @@ -201,6 +204,7 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsEditor) NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference) NS_INTERFACE_MAP_ENTRY(nsIEditorIMESupport) NS_INTERFACE_MAP_ENTRY(nsIEditor) + NS_INTERFACE_MAP_ENTRY(nsIObserver) NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIEditor) NS_INTERFACE_MAP_END @@ -300,6 +304,13 @@ nsEditor::PostCreate() // update the UI with our state NotifyDocumentListeners(eDocumentCreated); NotifyDocumentListeners(eDocumentStateChanged); + + nsCOMPtr obs = mozilla::services::GetObserverService(); + if (obs) { + obs->AddObserver(this, + SPELLCHECK_DICTIONARY_REMOVE_NOTIFICATION, + false); + } } // update nsTextStateManager and caret if we have focus @@ -437,6 +448,14 @@ nsEditor::PreDestroy(bool aDestroyingFrames) IMEStateManager::OnEditorDestroying(this); + nsCOMPtr obs = mozilla::services::GetObserverService(); + if (obs) { + obs->RemoveObserver(this, + SPELLCHECK_DICTIONARY_UPDATE_NOTIFICATION); + obs->RemoveObserver(this, + SPELLCHECK_DICTIONARY_REMOVE_NOTIFICATION); + } + // Let spellchecker clean up its observers etc. It is important not to // actually free the spellchecker here, since the spellchecker could have // caused flush notifications, which could have gotten here if a textbox @@ -1295,6 +1314,35 @@ NS_IMETHODIMP nsEditor::GetInlineSpellChecker(bool autoCreate, return NS_OK; } +NS_IMETHODIMP nsEditor::Observe(nsISupports* aSubj, const char *aTopic, + const char16_t *aData) +{ + NS_ASSERTION(!strcmp(aTopic, + SPELLCHECK_DICTIONARY_UPDATE_NOTIFICATION) || + !strcmp(aTopic, + SPELLCHECK_DICTIONARY_REMOVE_NOTIFICATION), + "Unexpected observer topic"); + + // When mozInlineSpellChecker::CanEnableInlineSpellChecking changes + SyncRealTimeSpell(); + + // When nsIEditorSpellCheck::GetCurrentDictionary changes + if (mInlineSpellChecker) { + // Do the right thing in the spellchecker, if the dictionary is no longer + // available. This will not set a new dictionary. + nsCOMPtr editorSpellCheck; + mInlineSpellChecker->GetSpellChecker(getter_AddRefs(editorSpellCheck)); + if (editorSpellCheck) { + editorSpellCheck->CheckCurrentDictionary(); + } + + // update the inline spell checker to reflect the new current dictionary + mInlineSpellChecker->SpellCheckRange(nullptr); // causes recheck + } + + return NS_OK; +} + NS_IMETHODIMP nsEditor::SyncRealTimeSpell() { bool enable = GetDesiredSpellCheckState(); @@ -5158,6 +5206,29 @@ nsEditor::OnFocus(nsIDOMEventTarget* aFocusEventTarget) } } +void +nsEditor::StartWatchingDictionaryChanges() +{ + if (!mObservingDictionaryUpdates) { + nsCOMPtr obs = mozilla::services::GetObserverService(); + if (obs) { + obs->AddObserver(this, SPELLCHECK_DICTIONARY_UPDATE_NOTIFICATION, false); + } + mObservingDictionaryUpdates = true; + } +} + +void +nsEditor::StopWatchingDictionaryChanges() +{ + // Removing an observer that wasn't added doesn't cause any harm. + nsCOMPtr obs = mozilla::services::GetObserverService(); + if (obs) { + obs->RemoveObserver(this, SPELLCHECK_DICTIONARY_UPDATE_NOTIFICATION); + } + mObservingDictionaryUpdates = false; +} + NS_IMETHODIMP nsEditor::GetSuppressDispatchingInputEvent(bool *aSuppressed) { diff --git a/editor/libeditor/nsEditor.h b/editor/libeditor/nsEditor.h index 4873838fca30..31d2416cd995 100644 --- a/editor/libeditor/nsEditor.h +++ b/editor/libeditor/nsEditor.h @@ -140,6 +140,7 @@ inline bool operator!(const EditAction& aOp) class nsEditor : public nsIEditor, public nsIEditorIMESupport, public nsSupportsWeakReference, + public nsIObserver, public nsIPhonetic { public: @@ -187,6 +188,9 @@ public: /* ------------ nsIEditorIMESupport methods -------------- */ NS_DECL_NSIEDITORIMESUPPORT + /* ------------ nsIObserver methods -------------- */ + NS_DECL_NSIOBSERVER + // nsIPhonetic NS_DECL_NSIPHONETIC @@ -245,6 +249,9 @@ public: void SwitchTextDirectionTo(uint32_t aDirection); + void StartWatchingDictionaryChanges(); + void StopWatchingDictionaryChanges(); + protected: nsresult DetermineCurrentDirection(); void FireInputEvent(); @@ -887,6 +894,7 @@ protected: bool mDispatchInputEvent; bool mIsInEditAction; // true while the instance is handling an edit action bool mHidingCaret; // whether caret is hidden forcibly. + bool mObservingDictionaryUpdates; // whether the editor is observing dictionary changes. friend bool NSCanUnload(nsISupports* serviceMgr); friend class nsAutoTxnsConserveSelection; diff --git a/editor/libeditor/nsEditorEventListener.cpp b/editor/libeditor/nsEditorEventListener.cpp index e3010bff30d4..86e107404de7 100644 --- a/editor/libeditor/nsEditorEventListener.cpp +++ b/editor/libeditor/nsEditorEventListener.cpp @@ -1114,6 +1114,8 @@ nsEditorEventListener::Focus(nsIDOMEvent* aEvent) } } + mEditor->StartWatchingDictionaryChanges(); + mEditor->OnFocus(target); nsCOMPtr ps = GetPresShell(); @@ -1130,6 +1132,8 @@ nsEditorEventListener::Blur(nsIDOMEvent* aEvent) { NS_ENSURE_TRUE(aEvent, NS_OK); + mEditor->StopWatchingDictionaryChanges(); + // check if something else is focused. If another element is focused, then // we should not change the selection. nsIFocusManager* fm = nsFocusManager::GetFocusManager(); diff --git a/editor/nsIEditorSpellCheck.idl b/editor/nsIEditorSpellCheck.idl index adf4a0a03444..5a8d1be95ea7 100644 --- a/editor/nsIEditorSpellCheck.idl +++ b/editor/nsIEditorSpellCheck.idl @@ -9,10 +9,16 @@ interface nsIEditor; interface nsITextServicesFilter; interface nsIEditorSpellCheckCallback; -[scriptable, uuid(a171c25f-e4a8-4d08-adef-b797e6377bdc)] +[scriptable, uuid(dd32ef3b-a7d8-43d1-9617-5f2dddbe29eb)] interface nsIEditorSpellCheck : nsISupports { + /** + * Call this on any change in installed dictionaries to ensure that the spell + * checker is not using a current dictionary which is no longer available. + */ + void checkCurrentDictionary(); + /** * Returns true if we can enable spellchecking. If there are no available * dictionaries, this will return false. diff --git a/editor/txtsvc/nsISpellChecker.h b/editor/txtsvc/nsISpellChecker.h index cafc725bec93..c63f57091274 100644 --- a/editor/txtsvc/nsISpellChecker.h +++ b/editor/txtsvc/nsISpellChecker.h @@ -114,6 +114,12 @@ public: * empty string, spellchecker will be disabled. */ NS_IMETHOD SetCurrentDictionary(const nsAString &aDictionary) = 0; + + /** + * Call this on any change in installed dictionaries to ensure that the spell + * checker is not using a current dictionary which is no longer available. + */ + NS_IMETHOD CheckCurrentDictionary() = 0; }; NS_DEFINE_STATIC_IID_ACCESSOR(nsISpellChecker, NS_ISPELLCHECKER_IID) diff --git a/extensions/spellcheck/hunspell/glue/mozHunspell.cpp b/extensions/spellcheck/hunspell/glue/mozHunspell.cpp index ddf3a58b3b7c..3b66cac1324d 100644 --- a/extensions/spellcheck/hunspell/glue/mozHunspell.cpp +++ b/extensions/spellcheck/hunspell/glue/mozHunspell.cpp @@ -160,6 +160,12 @@ NS_IMETHODIMP mozHunspell::SetDictionary(const char16_t *aDictionary) mDecoder = nullptr; mEncoder = nullptr; + nsCOMPtr obs = mozilla::services::GetObserverService(); + if (obs) { + obs->NotifyObservers(nullptr, + SPELLCHECK_DICTIONARY_UPDATE_NOTIFICATION, + nullptr); + } return NS_OK; } @@ -220,6 +226,13 @@ NS_IMETHODIMP mozHunspell::SetDictionary(const char16_t *aDictionary) else mLanguage = Substring(mDictionary, 0, pos); + nsCOMPtr obs = mozilla::services::GetObserverService(); + if (obs) { + obs->NotifyObservers(nullptr, + SPELLCHECK_DICTIONARY_UPDATE_NOTIFICATION, + nullptr); + } + return NS_OK; } @@ -591,19 +604,11 @@ NS_IMETHODIMP mozHunspell::RemoveDirectory(nsIFile *aDir) { mDynamicDirectories.RemoveObject(aDir); LoadDictionaryList(true); - -#ifdef MOZ_THUNDERBIRD - /* - * This notification is needed for Thunderbird. Thunderbird derives the dictionary - * from the document's "lang" attribute. If a dictionary is removed, - * we need to change the "lang" attribute. - */ nsCOMPtr obs = mozilla::services::GetObserverService(); if (obs) { obs->NotifyObservers(nullptr, SPELLCHECK_DICTIONARY_REMOVE_NOTIFICATION, nullptr); } -#endif return NS_OK; } diff --git a/extensions/spellcheck/idl/mozISpellCheckingEngine.idl b/extensions/spellcheck/idl/mozISpellCheckingEngine.idl index c5f7db4210a5..c124bb8bca85 100644 --- a/extensions/spellcheck/idl/mozISpellCheckingEngine.idl +++ b/extensions/spellcheck/idl/mozISpellCheckingEngine.idl @@ -102,6 +102,8 @@ interface mozISpellCheckingEngine : nsISupports { #define DICTIONARY_SEARCH_DIRECTORY "DictD" #define DICTIONARY_SEARCH_DIRECTORY_LIST "DictDL" +#define SPELLCHECK_DICTIONARY_UPDATE_NOTIFICATION \ + "spellcheck-dictionary-update" #define SPELLCHECK_DICTIONARY_REMOVE_NOTIFICATION \ "spellcheck-dictionary-remove" %} diff --git a/extensions/spellcheck/src/mozInlineSpellChecker.cpp b/extensions/spellcheck/src/mozInlineSpellChecker.cpp index a9fcb3041050..89c0546d326d 100644 --- a/extensions/spellcheck/src/mozInlineSpellChecker.cpp +++ b/extensions/spellcheck/src/mozInlineSpellChecker.cpp @@ -2021,8 +2021,10 @@ nsresult mozInlineSpellChecker::CurrentDictionaryUpdated() currentDictionary.Truncate(); } - nsresult rv = SpellCheckRange(nullptr); - NS_ENSURE_SUCCESS(rv, rv); + if (!mPreviousDictionary.Equals(currentDictionary)) { + nsresult rv = SpellCheckRange(nullptr); + NS_ENSURE_SUCCESS(rv, rv); + } return NS_OK; } diff --git a/extensions/spellcheck/src/mozSpellChecker.cpp b/extensions/spellcheck/src/mozSpellChecker.cpp index c695efb863ab..adc34f2898ac 100644 --- a/extensions/spellcheck/src/mozSpellChecker.cpp +++ b/extensions/spellcheck/src/mozSpellChecker.cpp @@ -428,6 +428,31 @@ mozSpellChecker::SetCurrentDictionary(const nsAString &aDictionary) return NS_ERROR_NOT_AVAILABLE; } +NS_IMETHODIMP +mozSpellChecker::CheckCurrentDictionary() +{ + // If the current dictionary has been uninstalled, we need to stop using it. + // This happens when there is a current engine, but that engine has no + // current dictionary. + + if (!mSpellCheckingEngine) { + // We didn't have a current dictionary + return NS_OK; + } + + nsXPIDLString dictname; + mSpellCheckingEngine->GetDictionary(getter_Copies(dictname)); + + if (!dictname.IsEmpty()) { + // We still have a current dictionary + return NS_OK; + } + + // We had a current dictionary, but it has gone, so we cannot use it anymore. + mSpellCheckingEngine = nullptr; + return NS_OK; +} + nsresult mozSpellChecker::SetupDoc(int32_t *outBlockOffset) { diff --git a/extensions/spellcheck/src/mozSpellChecker.h b/extensions/spellcheck/src/mozSpellChecker.h index 883dee38d76e..d60b5ac26d21 100644 --- a/extensions/spellcheck/src/mozSpellChecker.h +++ b/extensions/spellcheck/src/mozSpellChecker.h @@ -48,6 +48,7 @@ public: NS_IMETHOD GetDictionaryList(nsTArray *aDictionaryList) override; NS_IMETHOD GetCurrentDictionary(nsAString &aDictionary) override; NS_IMETHOD SetCurrentDictionary(const nsAString &aDictionary) override; + NS_IMETHOD CheckCurrentDictionary() override; void DeleteRemoteEngine() { mEngine = nullptr; diff --git a/extensions/spellcheck/tests/chrome/test_add_remove_dictionaries.xul b/extensions/spellcheck/tests/chrome/test_add_remove_dictionaries.xul index 7375c744409f..7fed54acf024 100644 --- a/extensions/spellcheck/tests/chrome/test_add_remove_dictionaries.xul +++ b/extensions/spellcheck/tests/chrome/test_add_remove_dictionaries.xul @@ -82,25 +82,17 @@ function RunTest() { // select map dictionary setCurrentDictionary(editor, "maputf"); - // Focus again, so the spelling gets updated. - textbox.blur(); - textbox.focus(); - onSpellCheck(textbox, function () { // test that map dictionary is in use - is(getMisspelledWords(editor), "created" + "imply" + "tomorrow" + "qwertyu", "map misspellings (1)"); + is(getMisspelledWords(editor), "created" + "imply" + "tomorrow" + "qwertyu", "map misspellings"); is(getCurrentDictionary(editor), "maputf", "current dictionary"); // uninstall map dictionary hunspell.removeDirectory(map); - // Focus again, so the spelling gets updated. - textbox.blur(); - textbox.focus(); - onSpellCheck(textbox, function () { // test that map dictionary is not in use - isnot(getMisspelledWords(editor), "created" + "imply" + "tomorrow" + "qwertyu", "map misspellings (2)"); + isnot(getMisspelledWords(editor), "created" + "imply" + "tomorrow" + "qwertyu", "map misspellings"); isnot(getCurrentDictionary(editor), "maputf", "current dictionary"); // test that base dictionary is available and map dictionary is unavailable