From 8cf45849efac072928aa0aab74834b66442b5df6 Mon Sep 17 00:00:00 2001
From: Masayuki Nakano <masayuki@d-toybox.com>
Date: Mon, 6 Jun 2016 21:07:24 +0900
Subject: [PATCH] Bug 1278084 part.1 Don't release TSF objects during handling
 a key message r=m_kato

While TIP is handling a key message, TSFTextStore shouldn't release any TSF objects since it may cause hitting a bug of TIPs.  Actually, MS-IME for Japanese on Windows 10 crashes when TSFTextStore is destroyed during composition because probably it accesses some destroyed objects to request to commit composition or query contents.

MozReview-Commit-ID: 9CTjHhAvG04

--HG--
extra : rebase_source : c34041962927795fe0d288aed10a96cf064b6243
---
 widget/windows/TSFTextStore.cpp | 49 +++++++++++++++++++++++++++++----
 widget/windows/TSFTextStore.h   | 16 +++++++++++
 2 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/widget/windows/TSFTextStore.cpp b/widget/windows/TSFTextStore.cpp
index 65f1c8af9b6f..66ade4e57057 100644
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -1182,6 +1182,7 @@ TSFTextStore::TSFTextStore()
   , mSinkMask(0)
   , mLock(0)
   , mLockQueued(0)
+  , mHandlingKeyMessage(0)
   , mLockedContent(mComposition, mSelection)
   , mRequestedAttrValues(false)
   , mIsRecordingActionsWithoutLock(false)
@@ -1288,9 +1289,10 @@ TSFTextStore::Destroy()
 {
   MOZ_LOG(sTextStoreLog, LogLevel::Info,
     ("TSF: 0x%p TSFTextStore::Destroy(), mLock=%s, "
-     "mComposition.IsComposing()=%s",
+     "mComposition.IsComposing()=%s, mHandlingKeyMessage=%u",
      this, GetLockFlagNameStr(mLock).get(),
-     GetBoolName(mComposition.IsComposing())));
+     GetBoolName(mComposition.IsComposing()),
+     mHandlingKeyMessage));
 
   mDestroyed = true;
 
@@ -1319,6 +1321,27 @@ TSFTextStore::Destroy()
   mLockedContent.Clear();
   mSelection.MarkDirty();
 
+  // If this is called during handling a keydown or keyup message, we should
+  // put off to release TSF objects until it completely finishes since
+  // MS-IME for Japanese refers some objects without grabbing them.
+  if (!mHandlingKeyMessage) {
+    ReleaseTSFObjects();
+  }
+
+  MOZ_LOG(sTextStoreLog, LogLevel::Info,
+    ("TSF: 0x%p   TSFTextStore::Destroy() succeeded", this));
+
+  return true;
+}
+
+void
+TSFTextStore::ReleaseTSFObjects()
+{
+  MOZ_ASSERT(!mHandlingKeyMessage);
+
+  MOZ_LOG(sTextStoreLog, LogLevel::Info,
+    ("TSF: 0x%p TSFTextStore::ReleaseTSFObjects()", this));
+
   mContext = nullptr;
   if (mDocumentMgr) {
     mDocumentMgr->Pop(TF_POPF_ALL);
@@ -1330,14 +1353,14 @@ TSFTextStore::Destroy()
 
   if (!mMouseTrackers.IsEmpty()) {
     MOZ_LOG(sTextStoreLog, LogLevel::Debug,
-      ("TSF: 0x%p   TSFTextStore::Destroy(), removing a mouse tracker...",
+      ("TSF: 0x%p   TSFTextStore::ReleaseTSFObjects(), "
+       "removing a mouse tracker...",
        this));
     mMouseTrackers.Clear();
   }
 
-  MOZ_LOG(sTextStoreLog, LogLevel::Info,
-    ("TSF: 0x%p   TSFTextStore::Destroy() succeeded", this));
-  return true;
+  MOZ_LOG(sTextStoreLog, LogLevel::Debug,
+    ("TSF: 0x%p   TSFTextStore::ReleaseTSFObjects() completed", this));
 }
 
 STDMETHODIMP
@@ -5470,7 +5493,14 @@ TSFTextStore::ProcessRawKeyMessage(const MSG& aMsg)
     if (FAILED(hr) || !eaten) {
       return false;
     }
+    RefPtr<TSFTextStore> textStore(sEnabledTextStore);
+    if (textStore) {
+      textStore->OnStartToHandleKeyMessage();
+    }
     hr = sKeystrokeMgr->KeyDown(aMsg.wParam, aMsg.lParam, &eaten);
+    if (textStore) {
+      textStore->OnEndHandlingKeyMessage();
+    }
     return SUCCEEDED(hr) && eaten;
   }
   if (aMsg.message == WM_KEYUP) {
@@ -5479,7 +5509,14 @@ TSFTextStore::ProcessRawKeyMessage(const MSG& aMsg)
     if (FAILED(hr) || !eaten) {
       return false;
     }
+    RefPtr<TSFTextStore> textStore(sEnabledTextStore);
+    if (textStore) {
+      textStore->OnStartToHandleKeyMessage();
+    }
     hr = sKeystrokeMgr->KeyUp(aMsg.wParam, aMsg.lParam, &eaten);
+    if (textStore) {
+      textStore->OnEndHandlingKeyMessage();
+    }
     return SUCCEEDED(hr) && eaten;
   }
   return false;
diff --git a/widget/windows/TSFTextStore.h b/widget/windows/TSFTextStore.h
index b4d7f8545c30..f3ed35556003 100644
--- a/widget/windows/TSFTextStore.h
+++ b/widget/windows/TSFTextStore.h
@@ -248,6 +248,7 @@ protected:
 
   bool     Init(nsWindowBase* aWidget);
   bool     Destroy();
+  void     ReleaseTSFObjects();
 
   bool     IsReadLock(DWORD aLock) const
   {
@@ -348,6 +349,21 @@ protected:
   // 0 if no lock is queued, otherwise TS_LF_* indicating the queue lock
   DWORD                        mLockQueued;
 
+  uint32_t mHandlingKeyMessage;
+  void OnStartToHandleKeyMessage() { ++mHandlingKeyMessage; }
+  void OnEndHandlingKeyMessage()
+  {
+    MOZ_ASSERT(mHandlingKeyMessage);
+    if (--mHandlingKeyMessage) {
+      return;
+    }
+    // If TSFTextStore instance is destroyed during handling key message(s),
+    // release all TSF objects when all nested key messages have been handled.
+    if (mDestroyed) {
+      ReleaseTSFObjects();
+    }
+  }
+
   class Composition final
   {
   public: