Bug 1052343 part.6 Don't destroy nsTextStore instance during it's locking the document r=emk
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 03 Sep 2014 10:38:20 +0900
changeset 203160 b364c512840e980663ddc86f8c99eb63ca61a322
parent 203159 06ba93acf94e6ff7c7562ea2bf7d90588a975668
child 203161 a34dfd7c025d70c27e1ed02e63d9b8716d08d8a1
push id48592
push usermasayuki@d-toybox.com
push dateWed, 03 Sep 2014 01:38:24 +0000
treeherdermozilla-inbound@b364c512840e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemk
bugs1052343
milestone35.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 1052343 part.6 Don't destroy nsTextStore instance during it's locking the document r=emk
widget/windows/nsTextStore.cpp
widget/windows/nsTextStore.h
--- a/widget/windows/nsTextStore.cpp
+++ b/widget/windows/nsTextStore.cpp
@@ -1172,16 +1172,17 @@ nsTextStore::nsTextStore()
   , mEditCookie(0)
   , mSinkMask(0)
   , mLock(0)
   , mLockQueued(0)
   , mRequestedAttrValues(false)
   , mIsRecordingActionsWithoutLock(false)
   , mPendingOnSelectionChange(false)
   , mPendingOnLayoutChange(false)
+  , mPendingDestroy(false)
   , mNativeCaretIsCreated(false)
 {
   for (int32_t i = 0; i < NUM_OF_SUPPORTED_ATTRS; i++) {
     mRequestedAttrs[i] = false;
   }
 
   // We hope that 5 or more actions don't occur at once.
   mPendingActions.SetCapacity(5);
@@ -1253,18 +1254,25 @@ nsTextStore::Init(nsWindowBase* aWidget)
 
   return true;
 }
 
 bool
 nsTextStore::Destroy()
 {
   PR_LOG(sTextStoreLog, PR_LOG_ALWAYS,
-    ("TSF: 0x%p nsTextStore::Destroy(), mComposition.IsComposing()=%s",
-     this, GetBoolName(mComposition.IsComposing())));
+    ("TSF: 0x%p nsTextStore::Destroy(), mLock=%s, "
+     "mComposition.IsComposing()=%s",
+     this, GetLockFlagNameStr(mLock).get(),
+     GetBoolName(mComposition.IsComposing())));
+
+  if (mLock) {
+    mPendingDestroy = true;
+    return true;
+  }
 
   // If there is composition, TSF keeps the composition even after the text
   // store destroyed.  So, we should clear the composition here.
   if (mComposition.IsComposing()) {
     NS_WARNING("Composition is still alive at destroying the text store");
     CommitCompositionInternal(false);
   }
 
@@ -1425,16 +1433,19 @@ nsTextStore::RequestLock(DWORD dwLockFla
 
   if (!mLock) {
     // put on lock
     mLock = dwLockFlags & (~TS_LF_SYNC);
     PR_LOG(sTextStoreLog, PR_LOG_ALWAYS,
       ("TSF: 0x%p   Locking (%s) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>"
        ">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>",
        this, GetLockFlagNameStr(mLock).get()));
+    // Don't release this instance during this lock because this is called by
+    // TSF but they don't grab us during this call.
+    nsRefPtr<nsTextStore> kungFuDeathGrip(this);
     *phrSession = mSink->OnLockGranted(mLock);
     PR_LOG(sTextStoreLog, PR_LOG_ALWAYS,
       ("TSF: 0x%p   Unlocked (%s) <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<"
        "<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<",
        this, GetLockFlagNameStr(mLock).get()));
     DidLockGranted();
     while (mLockQueued) {
       mLock = mLockQueued;
@@ -1449,17 +1460,17 @@ nsTextStore::RequestLock(DWORD dwLockFla
          "<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<",
          this, GetLockFlagNameStr(mLock).get()));
       DidLockGranted();
     }
 
     // The document is now completely unlocked.
     mLock = 0;
 
-    if (mPendingOnLayoutChange) {
+    if (!mPendingDestroy && mPendingOnLayoutChange) {
       mPendingOnLayoutChange = false;
       if (mSink) {
         PR_LOG(sTextStoreLog, PR_LOG_ALWAYS,
                ("TSF: 0x%p   nsTextStore::RequestLock(), "
                 "calling ITextStoreACPSink::OnLayoutChange()...", this));
         mSink->OnLayoutChange(TS_LC_CHANGE, TEXTSTORE_DEFAULT_VIEW);
       }
       // The layout change caused by composition string change should cause
@@ -1473,26 +1484,30 @@ nsTextStore::RequestLock(DWORD dwLockFla
                  ("TSF: 0x%p   nsTextStore::RequestLock(), "
                   "calling ITfContextOwnerServices::OnLayoutChange()...",
                   this));
           service->OnLayoutChange();
         }
       }
     }
 
-    if (mPendingOnSelectionChange) {
+    if (!mPendingDestroy && mPendingOnSelectionChange) {
       mPendingOnSelectionChange = false;
       if (mSink) {
         PR_LOG(sTextStoreLog, PR_LOG_ALWAYS,
                ("TSF: 0x%p   nsTextStore::RequestLock(), "
                 "calling ITextStoreACPSink::OnSelectionChange()...", this));
         mSink->OnSelectionChange();
       }
     }
 
+    if (mPendingDestroy) {
+      Destroy();
+    }
+
     PR_LOG(sTextStoreLog, PR_LOG_ALWAYS,
       ("TSF: 0x%p   nsTextStore::RequestLock() succeeded: *phrSession=%s",
        this, GetTextStoreReturnValueName(*phrSession)));
     return S_OK;
   }
 
   // only time when reentrant lock is allowed is when caller holds a
   // read-only lock and is requesting an async write lock
--- a/widget/windows/nsTextStore.h
+++ b/widget/windows/nsTextStore.h
@@ -727,16 +727,19 @@ protected:
   // mSink->OnSelectionChange() after mLock becomes 0.
   bool                         mPendingOnSelectionChange;
   // If GetTextExt() or GetACPFromPoint() is called and the layout hasn't been
   // calculated yet, these methods return TS_E_NOLAYOUT.  Then, RequestLock()
   // will call mSink->OnLayoutChange() and
   // ITfContextOwnerServices::OnLayoutChange() after the layout is fixed and
   // the document is unlocked.
   bool                         mPendingOnLayoutChange;
+  // During the documet is locked, we shouldn't destroy the instance.
+  // If this is true, the instance will be destroyed after unlocked.
+  bool                         mPendingDestroy;
   // While there is native caret, this is true.  Otherwise, false.
   bool                         mNativeCaretIsCreated;
 
   // TSF thread manager object for the current application
   static ITfThreadMgr*  sTsfThreadMgr;
   // sMessagePump is QI'ed from sTsfThreadMgr
   static ITfMessagePump* sMessagePump;
   // sKeystrokeMgr is QI'ed from sTsfThreadMgr