Currently, EditorBase::mSelConWeak, EditorBase::mPlaceHolderTxn and EditorBase::mDocWeak are nsWeakPtr. However, they require QI before use but that appears in some profiles. So, I think that we should cache it and use nsWeakPtr only for checking if the pointer is still available.

Ah, it might be able to use mfbt/WeakPtr.h here. https://searchfox.org/mozilla-central/source/mfbt/WeakPtr.h

Oh, but EditorBase doesn't treat document as nsDocument. It treats document as nsIDocument or nsIDOMDocument. So, WeakPtr<nsDocument> isn't available in EditorBase. Sigh.

The fix improves the performance of bug 1346723 about 3%.

Oh, but this improves the score of the testcase for bug 1370754 (after setting innerHTML) about 10%.

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #10) > Oh, but this improves the score of the testcase for bug 1370754 (after > setting innerHTML) about 10%. Oops, not 10%, but 1%.

bsmedberg: Could you review the XPCOM part of the patch?

Comment on attachment 8878454 [details] Bug 1372829 - part1: Make mozilla::PlaceholderTransaction inherit mozilla::SupportsWeakPtr instead of nsSupportsWeakReference https://reviewboard.mozilla.org/r/149788/#review154794

Comment on attachment 8878455 [details] Bug 1372829 - part2: mozilla::EditorBase should cache raw pointer of nsISelectionController and nsIDocument with nsWeakPtr https://reviewboard.mozilla.org/r/149790/#review154796 ::: editor/libeditor/EditorBase.h:237 (Diff revision 1) > > public: > NS_DECL_CYCLE_COLLECTING_ISUPPORTS > NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(EditorBase, nsIEditor) > > + bool IsInitialized() { return !!mDocumentWeak; } bool IsInitialized() const?

Comment on attachment 8878455 [details] Bug 1372829 - part2: mozilla::EditorBase should cache raw pointer of nsISelectionController and nsIDocument with nsWeakPtr https://reviewboard.mozilla.org/r/149790/#review154796 > bool IsInitialized() const? Sure!

Comment on attachment 8878455 [details] Bug 1372829 - part2: mozilla::EditorBase should cache raw pointer of nsISelectionController and nsIDocument with nsWeakPtr https://reviewboard.mozilla.org/r/149790/#review155226

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/3d35d8ad784b part1: Make mozilla::PlaceholderTransaction inherit mozilla::SupportsWeakPtr instead of nsSupportsWeakReference r=m_kato https://hg.mozilla.org/integration/autoland/rev/971e73db7835 part2: mozilla::EditorBase should cache raw pointer of nsISelectionController and nsIDocument with nsWeakPtr r=froydnj,m_kato

https://hg.mozilla.org/mozilla-central/rev/3d35d8ad784b https://hg.mozilla.org/mozilla-central/rev/971e73db7835

I'm now getting crashes like: #0 0x00007fffe9adb528 in mozilla::CachedWeakPtr<nsIDocument, nsISupports>::get() const (this=<optimized out>) at /home/smaug/mozilla/hg/vsync/ff_build_opt/dist/include/mozilla/EditorBase.h:198 #1 0x00007fffe9adb528 in mozilla::EditorBase::GetDocument() (this=0x7ffff6c00008) at /home/smaug/mozilla/hg/vsync/editor/libeditor/EditorBase.cpp:566 #2 0x00007fffe9adb528 in mozilla::EditorBase::GetPresShell() (this=<optimized out>) at /home/smaug/mozilla/hg/vsync/editor/libeditor/EditorBase.cpp:587 #3 0x00007fffe9adb528 in mozilla::EditorBase::GetSelectionController() (this=0x7ffff6c00008) at /home/smaug/mozilla/hg/vsync/editor/libeditor/EditorBase.cpp:642 #4 0x00007fffe9ad5cc5 in mozilla::EditorBase::GetSelection(mozilla::SelectionType, nsISelection**) (aSelectionType=<optimized out>, this=<optimized out>, aSelection=<optimized out>) at /home/smaug/mozilla/hg/vsync/editor/libeditor/EditorBase.cpp:668 #5 0x00007fffe9ad5cc5 in mozilla::EditorBase::GetSelection(mozilla::SelectionType) (this=0x7ffff6c00008, aSelectionType=mozilla::SelectionType::eNormal) at /home/smaug/mozilla/hg/vsync/editor/libeditor/EditorBase.cpp:679 #6 0x00007fffe9af03ce in mozilla::HTMLEditor::GetSelectionContainer() (this=0x7ffff6c00008) at /home/smaug/mozilla/hg/vsync/editor/libeditor/HTMLEditor.cpp:4849 when trying to call GetSelectionContainer

(In reply to Olli Pettay [:smaug] from comment #24) > I'm now getting crashes like: > #0 0x00007fffe9adb528 in mozilla::CachedWeakPtr<nsIDocument, > nsISupports>::get() const (this=<optimized out>) at > /home/smaug/mozilla/hg/vsync/ff_build_opt/dist/include/mozilla/EditorBase.h: > 198 Let me know the crash reason like nullptr access or heap-use-after-free?

Maybe it was gdb showing wrong data after all while I was writing a patch which still contained some other issues. I'll re-report if I see the issue again.

Okay. FYI: we have no report crashed in CachedWeakPtr yet.