Closed Bug 1430982 Opened 2 years ago Closed 2 years ago

Make EditorBase not treat nsTextServicesDocument for mInlineSpellChecker as nsIEditActionListener

Categories

(Core :: DOM: Editor, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 1 open bug)

Details

Attachments

(10 files)

59 bytes, text/x-review-board-request
m_kato
: review+
Details
59 bytes, text/x-review-board-request
m_kato
: review+
Details
59 bytes, text/x-review-board-request
m_kato
: review+
Details
59 bytes, text/x-review-board-request
m_kato
: review+
Details
59 bytes, text/x-review-board-request
m_kato
: review+
Details
59 bytes, text/x-review-board-request
m_kato
: review+
Details
59 bytes, text/x-review-board-request
m_kato
: review+
Details
59 bytes, text/x-review-board-request
m_kato
: review+
Details
59 bytes, text/x-review-board-request
m_kato
: review+
Details
59 bytes, text/x-review-board-request
m_kato
: review+
Details
No description provided.
Hmm. nsEditorSpellChecker can be created by JS. Then, editor may be associated with 2 or more nsTextServicesDocument. However, this is really rare case. So, we should optimize performance for the case that there is only one nsTextServicesDocument is associated.
Summary: Make nsTextServicesDocument not derived from nsIEditActionListener → Make EditorBase treat nsTextServicesDocument for mInlineSpellChecker as nsIEditActionListener
Summary: Make EditorBase treat nsTextServicesDocument for mInlineSpellChecker as nsIEditActionListener → Make EditorBase not treat nsTextServicesDocument for mInlineSpellChecker as nsIEditActionListener
FYI:

nsTextServicesDocument is owned by mozSpellChecker, mozSpellChecker is owned by nsEditorSpellCheck, nsEditorSpellCheck is owned by mozInlineSpellChecker, mozInlineSpellChecker is owned by EditorBase in most cases.

Some exceptions are, some classes can be created from chrome JS. Actually, composer UI of comm-central creates nsEditorSpellCheck (although I'm not sure it's alive): https://searchfox.org/comm-central/source/editor/ui/dialogs/content/EdSpellCheck.js#26

Current goal of this bug is, making nsTextServicesDocument for EditorBase::mInlineSpellChecker is not added to the edit action listener array. Instead, called its methods directly.
And note that there is a bug of releasing above class instances when EditorBase becomes disabling spell check. When it becomes disabled, mozInlineSpellChecker releases nsEditorSpellCheck. However, if its refcount doesn't become 0, it keeps objects but if tried to clear them from mozInlineSpellChecker, that causes new crashes caused by nullptr access in some automated tests. I don't want to struggle with it in this bug.  Therefore, coming patches hack EditorBase::AddEditActionListener(). I think that this won't make damage to initializing performance because it's not called so many times.
Comment on attachment 8943794 [details]
Bug 1430982 - part 1: Make nsTextServicesDocument store editor with RefPtr<TextEditor> rather than nsWeakPtr

https://reviewboard.mozilla.org/r/214078/#review219930
Attachment #8943794 - Flags: review?(m_kato) → review+
Comment on attachment 8943795 [details]
Bug 1430982 - part 2: Rename nsTextServicesDocument to mozilla::TextServicesDocument and expose its header

https://reviewboard.mozilla.org/r/214080/#review219932
Attachment #8943795 - Flags: review?(m_kato) → review+
Comment on attachment 8943796 [details]
Bug 1430982 - part 3: Make mozSpellChecker and nsEditorSpellChecker treat TextServicesDocument directly rather than nsITextServicesDocument

https://reviewboard.mozilla.org/r/214082/#review219936
Attachment #8943796 - Flags: review?(m_kato) → review+
Comment on attachment 8943797 [details]
Bug 1430982 - part 4: Get rid of nsITextServicesDocument because it used by nobody

https://reviewboard.mozilla.org/r/214084/#review219938
Attachment #8943797 - Flags: review?(m_kato) → review+
Comment on attachment 8943798 [details]
Bug 1430982 - part 5: Make nsEditorSpellCheck store mozSpellChecker directly rather than nsISpellChecker

https://reviewboard.mozilla.org/r/214086/#review219940
Attachment #8943798 - Flags: review?(m_kato) → review+
Comment on attachment 8943799 [details]
Bug 1430982 - part 6: Rename nsEditorSpellCheck to mozilla::EditorSpellCheck and expose its header

https://reviewboard.mozilla.org/r/214088/#review219946
Attachment #8943799 - Flags: review?(m_kato) → review+
Comment on attachment 8943800 [details]
Bug 1430982 - part 7: Make mozInlineSpellChecker store EditorSpellCheck directly rather than nsIEditorSpellCheck

https://reviewboard.mozilla.org/r/214090/#review219948
Attachment #8943800 - Flags: review?(m_kato) → review+
Comment on attachment 8943801 [details]
Bug 1430982 - part 8: Make EditorBase store inline spell checker as mozInlineSpellChecker rather than nsIInlineSpellChecker

https://reviewboard.mozilla.org/r/214092/#review219950
Attachment #8943801 - Flags: review?(m_kato) → review+
Comment on attachment 8943803 [details]
Bug 1430982 - part 10: Make EditorBase store TextServicesDocument for its mInlineSpellChecker for avoiding to run for loops to call nsIEditActionListener methods

https://reviewboard.mozilla.org/r/214096/#review219966
Attachment #8943803 - Flags: review?(m_kato) → review+
Comment on attachment 8943802 [details]
Bug 1430982 - part 9: Create accessors for each member of mozSpellChecker, EditorSpellCheck, mozInlineSpellChecker

https://reviewboard.mozilla.org/r/214094/#review219968
Attachment #8943802 - Flags: review?(m_kato) → review+
Duplicate of this bug: 1378060
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/44be45cf31b4
part 1: Make nsTextServicesDocument store editor with RefPtr<TextEditor> rather than nsWeakPtr r=m_kato
https://hg.mozilla.org/integration/autoland/rev/1e9838519ff1
part 2: Rename nsTextServicesDocument to mozilla::TextServicesDocument and expose its header r=m_kato
https://hg.mozilla.org/integration/autoland/rev/05408b857941
part 3: Make mozSpellChecker and nsEditorSpellChecker treat TextServicesDocument directly rather than nsITextServicesDocument r=m_kato
https://hg.mozilla.org/integration/autoland/rev/0901d6f8322f
part 4: Get rid of nsITextServicesDocument because it used by nobody r=m_kato
https://hg.mozilla.org/integration/autoland/rev/2e02cfe32f94
part 5: Make nsEditorSpellCheck store mozSpellChecker directly rather than nsISpellChecker r=m_kato
https://hg.mozilla.org/integration/autoland/rev/2398b267741b
part 6: Rename nsEditorSpellCheck to mozilla::EditorSpellCheck and expose its header r=m_kato
https://hg.mozilla.org/integration/autoland/rev/c27c6e2b0494
part 7: Make mozInlineSpellChecker store EditorSpellCheck directly rather than nsIEditorSpellCheck r=m_kato
https://hg.mozilla.org/integration/autoland/rev/d0270ad1df87
part 8: Make EditorBase store inline spell checker as mozInlineSpellChecker rather than nsIInlineSpellChecker r=m_kato
https://hg.mozilla.org/integration/autoland/rev/4ef85ad5bf42
part 9: Create accessors for each member of mozSpellChecker, EditorSpellCheck, mozInlineSpellChecker r=m_kato
https://hg.mozilla.org/integration/autoland/rev/ff3e5fd4c725
part 10: Make EditorBase store TextServicesDocument for its mInlineSpellChecker for avoiding to run for loops to call nsIEditActionListener methods r=m_kato
You need to log in before you can comment on or make changes to this bug.