Bug 1234422 TSFTextStore should retry to notify TSF/TIP of layout change if synchronous calls of OnLayoutChange() don't cause retrieving layout information r=m_kato a=ritu
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 06 Jan 2016 17:01:02 -0800
changeset 304198 36699ba4cc35bf293eb97398ad1243a6a09a5fdb
parent 304197 5893c5df7b847c9060bb8e1e8573d533ad84f214
child 304199 faa80a5c54b0ef9f76fa14489d0173f04dbf0be6
push id5466
push userkwierso@gmail.com
push dateThu, 07 Jan 2016 01:04:27 +0000
treeherdermozilla-beta@faa80a5c54b0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, ritu
bugs1234422
milestone44.0
Bug 1234422 TSFTextStore should retry to notify TSF/TIP of layout change if synchronous calls of OnLayoutChange() don't cause retrieving layout information r=m_kato a=ritu
widget/windows/TSFTextStore.cpp
widget/windows/TSFTextStore.h
widget/windows/nsWindowDefs.h
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -1310,17 +1310,18 @@ TSFTextStore::TSFTextStore()
   : mEditCookie(0)
   , mSinkMask(0)
   , mLock(0)
   , mLockQueued(0)
   , mLockedContent(mComposition, mSelection)
   , mRequestedAttrValues(false)
   , mIsRecordingActionsWithoutLock(false)
   , mPendingOnSelectionChange(false)
-  , mPendingOnLayoutChange(false)
+  , mHasReturnedNoLayoutError(false)
+  , mWaitingQueryLayout(false)
   , mPendingDestroy(false)
   , mDeferClearingLockedContent(false)
   , mNativeCaretIsCreated(false)
   , mDeferNotifyingTSF(false)
   , mDestroyed(false)
 {
   for (int32_t i = 0; i < NUM_OF_SUPPORTED_ATTRS; i++) {
     mRequestedAttrs[i] = false;
@@ -1665,17 +1666,17 @@ TSFTextStore::DidLockGranted()
     CompleteLastActionIfStillIncomplete();
 
     FlushPendingActions();
   }
 
   // If the widget has gone, we don't need to notify anything.
   if (!mWidget || mWidget->Destroyed()) {
     mPendingOnSelectionChange = false;
-    mPendingOnLayoutChange = false;
+    mHasReturnedNoLayoutError = false;
   }
 }
 
 void
 TSFTextStore::DispatchEvent(WidgetGUIEvent& aEvent)
 {
   if (NS_WARN_IF(!mWidget) || NS_WARN_IF(mWidget->Destroyed())) {
     return;
@@ -1690,17 +1691,17 @@ TSFTextStore::DispatchEvent(WidgetGUIEve
 
 void
 TSFTextStore::FlushPendingActions()
 {
   if (!mWidget || mWidget->Destroyed()) {
     mPendingActions.Clear();
     mLockedContent.Clear();
     mPendingOnSelectionChange = false;
-    mPendingOnLayoutChange = false;
+    mHasReturnedNoLayoutError = false;
     return;
   }
 
   RefPtr<nsWindowBase> kungFuDeathGrip(mWidget);
   for (uint32_t i = 0; i < mPendingActions.Length(); i++) {
     PendingAction& action = mPendingActions[i];
     switch (action.mType) {
       case PendingAction::COMPOSITION_START: {
@@ -1896,21 +1897,21 @@ TSFTextStore::MaybeFlushPendingNotificat
 
   if (!mDeferClearingLockedContent && mLockedContent.IsInitialized()) {
     mLockedContent.Clear();
     MOZ_LOG(sTextStoreLog, LogLevel::Debug,
            ("TSF: 0x%p   TSFTextStore::MaybeFlushPendingNotifications(), "
             "mLockedContent is cleared", this));
   }
 
-  if (mPendingOnLayoutChange) {
+  if (mHasReturnedNoLayoutError) {
     MOZ_LOG(sTextStoreLog, LogLevel::Info,
            ("TSF: 0x%p   TSFTextStore::MaybeFlushPendingNotifications(), "
             "calling TSFTextStore::NotifyTSFOfLayoutChange()...", this));
-    NotifyTSFOfLayoutChange(true);
+    NotifyTSFOfLayoutChange();
   }
 
   if (mPendingOnSelectionChange) {
     MOZ_LOG(sTextStoreLog, LogLevel::Info,
            ("TSF: 0x%p   TSFTextStore::MaybeFlushPendingNotifications(), "
             "calling TSFTextStore::NotifyTSFOfSelectionChange()...", this));
     NotifyTSFOfSelectionChange();
   }
@@ -3334,20 +3335,21 @@ TSFTextStore::GetActiveView(TsViewCookie
 STDMETHODIMP
 TSFTextStore::GetACPFromPoint(TsViewCookie vcView,
                               const POINT* pt,
                               DWORD dwFlags,
                               LONG* pacp)
 {
   MOZ_LOG(sTextStoreLog, LogLevel::Info,
          ("TSF: 0x%p TSFTextStore::GetACPFromPoint(pvcView=%d, pt=%p (x=%d, "
-          "y=%d), dwFlags=%s, pacp=%p, mDeferNotifyingTSF=%s",
+          "y=%d), dwFlags=%s, pacp=%p, mDeferNotifyingTSF=%s, "
+          "mWaitingQueryLayout=%s",
           this, vcView, pt, pt ? pt->x : 0, pt ? pt->y : 0,
           GetACPFromPointFlagName(dwFlags).get(), pacp,
-          GetBoolName(mDeferNotifyingTSF)));
+          GetBoolName(mDeferNotifyingTSF), GetBoolName(mWaitingQueryLayout)));
 
   if (!IsReadLocked()) {
     MOZ_LOG(sTextStoreLog, LogLevel::Error,
            ("TSF: 0x%p   TSFTextStore::GetACPFromPoint() FAILED due to "
             "not locked (read)", this));
     return TS_E_NOLOCK;
   }
 
@@ -3367,21 +3369,23 @@ TSFTextStore::GetACPFromPoint(TsViewCook
 
   if (!pacp) {
     MOZ_LOG(sTextStoreLog, LogLevel::Error,
            ("TSF: 0x%p   TSFTextStore::GetACPFromPoint() FAILED due to "
             "null pacp", this));
     return E_INVALIDARG;
   }
 
+  mWaitingQueryLayout = false;
+
   if (mLockedContent.IsLayoutChanged()) {
     MOZ_LOG(sTextStoreLog, LogLevel::Error,
-           ("TSF: 0x%p   TSFTextStore::GetACPFromPoint() FAILED due to "
-            "layout not recomputed", this));
-    mPendingOnLayoutChange = true;
+           ("TSF: 0x%p   TSFTextStore::GetACPFromPoint() returned "
+            "TS_E_NOLAYOUT", this));
+    mHasReturnedNoLayoutError = true;
     return TS_E_NOLAYOUT;
   }
 
   nsIntPoint ourPt(pt->x, pt->y);
   // Convert to widget relative coordinates from screen's.
   ourPt -= mWidget->WidgetToScreenOffsetUntyped();
 
   // NOTE: Don't check if the point is in the widget since the point can be
@@ -3487,19 +3491,19 @@ TSFTextStore::GetTextExt(TsViewCookie vc
                          LONG acpStart,
                          LONG acpEnd,
                          RECT* prc,
                          BOOL* pfClipped)
 {
   MOZ_LOG(sTextStoreLog, LogLevel::Info,
          ("TSF: 0x%p TSFTextStore::GetTextExt(vcView=%ld, "
           "acpStart=%ld, acpEnd=%ld, prc=0x%p, pfClipped=0x%p), "
-          "mDeferNotifyingTSF=%s",
+          "mDeferNotifyingTSF=%s, mWaitingQueryLayout=%s",
           this, vcView, acpStart, acpEnd, prc, pfClipped,
-          GetBoolName(mDeferNotifyingTSF)));
+          GetBoolName(mDeferNotifyingTSF), GetBoolName(mWaitingQueryLayout)));
 
   if (!IsReadLocked()) {
     MOZ_LOG(sTextStoreLog, LogLevel::Error,
            ("TSF: 0x%p   TSFTextStore::GetTextExt() FAILED due to "
             "not locked (read)", this));
     return TS_E_NOLOCK;
   }
 
@@ -3519,16 +3523,18 @@ TSFTextStore::GetTextExt(TsViewCookie vc
 
   if (acpStart < 0 || acpEnd < acpStart) {
     MOZ_LOG(sTextStoreLog, LogLevel::Error,
            ("TSF: 0x%p   TSFTextStore::GetTextExt() FAILED due to "
             "invalid position", this));
     return TS_E_INVALIDPOS;
   }
 
+  mWaitingQueryLayout = false;
+
   // NOTE: TSF (at least on Win 8.1) doesn't return TS_E_NOLAYOUT to the
   // caller even if we return it.  It's converted to just E_FAIL.
   // However, this is fixed on Win 10.
 
   const TSFStaticSink* kSink = TSFStaticSink::GetInstance();
   if (mComposition.IsComposing() && mComposition.mStart < acpEnd &&
       mLockedContent.IsLayoutChangedAfter(acpEnd)) {
     const Selection& currentSel = CurrentSelection();
@@ -3606,19 +3612,19 @@ TSFTextStore::GetTextExt(TsViewCookie vc
       MOZ_LOG(sTextStoreLog, LogLevel::Debug,
              ("TSF: 0x%p   TSFTextStore::GetTextExt() hacked the offsets for "
               "TIP acpStart=%d, acpEnd=%d", this, acpStart, acpEnd));
     }
   }
 
   if (mLockedContent.IsLayoutChangedAfter(acpEnd)) {
     MOZ_LOG(sTextStoreLog, LogLevel::Error,
-           ("TSF: 0x%p   TSFTextStore::GetTextExt() FAILED due to "
-            "layout not recomputed at %d", this, acpEnd));
-    mPendingOnLayoutChange = true;
+           ("TSF: 0x%p   TSFTextStore::GetTextExt() returned TS_E_NOLAYOUT "
+            "(acpEnd=%d)", this, acpEnd));
+    mHasReturnedNoLayoutError = true;
     return TS_E_NOLAYOUT;
   }
 
   // use eQueryTextRect to get rect in system, screen coordinates
   WidgetQueryContentEvent event(true, eQueryTextRect, mWidget);
   mWidget->InitEvent(event);
   event.InitForQueryTextRect(acpStart, acpEnd - acpStart);
   DispatchEvent(event);
@@ -4763,72 +4769,138 @@ TSFTextStore::OnLayoutChangeInternal()
   nsresult rv = NS_OK;
 
   // We need to notify TSF of layout change even if the document is locked.
   // So, don't use MaybeFlushPendingNotifications() for flushing pending
   // layout change.
   MOZ_LOG(sTextStoreLog, LogLevel::Info,
          ("TSF: 0x%p   TSFTextStore::OnLayoutChangeInternal(), calling "
           "NotifyTSFOfLayoutChange()...", this));
-  if (NS_WARN_IF(!NotifyTSFOfLayoutChange(mPendingOnLayoutChange))) {
+  if (NS_WARN_IF(!NotifyTSFOfLayoutChange())) {
     rv = NS_ERROR_FAILURE;
   }
 
   MOZ_LOG(sTextStoreLog, LogLevel::Debug,
          ("TSF: 0x%p   TSFTextStore::OnLayoutChangeInternal(), calling "
           "MaybeFlushPendingNotifications()...", this));
   MaybeFlushPendingNotifications();
 
   return rv;
 }
 
 bool
-TSFTextStore::NotifyTSFOfLayoutChange(bool aFlush)
+TSFTextStore::NotifyTSFOfLayoutChange()
 {
-  mPendingOnLayoutChange = false;
+  bool returnedNoLayoutError = mHasReturnedNoLayoutError;
+
+  // If we returned TS_E_NOLAYOUT, TIP should query the computed layout again.
+  mWaitingQueryLayout = returnedNoLayoutError;
+
+  // For avoiding to call this method again at unlocking the document during
+  // calls of OnLayoutChange(), reset mHasReturnedNoLayoutError.
+  mHasReturnedNoLayoutError = false;
 
   // Now, layout has been computed.  We should notify mLockedContent for
   // making GetTextExt() and GetACPFromPoint() not return TS_E_NOLAYOUT.
   if (mLockedContent.IsInitialized()) {
     mLockedContent.OnLayoutChanged();
   }
 
   // Now, the caret position is different from ours.  Destroy the native caret
   // if there is.
   MaybeDestroyNativeCaret();
 
   // This method should return true if either way succeeds.
-  bool ret = false;
+  bool ret = true;
 
   if (mSink) {
     MOZ_LOG(sTextStoreLog, LogLevel::Info,
            ("TSF: 0x%p   TSFTextStore::NotifyTSFOfLayoutChange(), "
             "calling ITextStoreACPSink::OnLayoutChange()...",
             this));
     HRESULT hr = mSink->OnLayoutChange(TS_LC_CHANGE, TEXTSTORE_DEFAULT_VIEW);
+    MOZ_LOG(sTextStoreLog, LogLevel::Info,
+           ("TSF: 0x%p   TSFTextStore::NotifyTSFOfLayoutChange(), "
+            "called ITextStoreACPSink::OnLayoutChange()",
+            this));
     ret = SUCCEEDED(hr);
   }
 
   // The layout change caused by composition string change should cause
   // calling ITfContextOwnerServices::OnLayoutChange() too.
-  if (aFlush && mContext) {
+  if (returnedNoLayoutError && mContext) {
     RefPtr<ITfContextOwnerServices> service;
     mContext->QueryInterface(IID_ITfContextOwnerServices,
                              getter_AddRefs(service));
     if (service) {
       MOZ_LOG(sTextStoreLog, LogLevel::Info,
              ("TSF: 0x%p   TSFTextStore::NotifyTSFOfLayoutChange(), "
               "calling ITfContextOwnerServices::OnLayoutChange()...",
               this));
       HRESULT hr = service->OnLayoutChange();
-      ret = SUCCEEDED(hr);
+      ret = ret && SUCCEEDED(hr);
+      MOZ_LOG(sTextStoreLog, LogLevel::Info,
+             ("TSF: 0x%p   TSFTextStore::NotifyTSFOfLayoutChange(), "
+              "called ITfContextOwnerServices::OnLayoutChange()",
+              this));
     }
   }
 
-  return ret;
+  if (!mWidget || mWidget->Destroyed()) {
+    MOZ_LOG(sTextStoreLog, LogLevel::Info,
+           ("TSF: 0x%p   TSFTextStore::NotifyTSFOfLayoutChange(), "
+            "the widget is destroyed during calling OnLayoutChange()",
+            this));
+    return ret;
+  }
+
+  if (mDestroyed) {
+    MOZ_LOG(sTextStoreLog, LogLevel::Info,
+           ("TSF: 0x%p   TSFTextStore::NotifyTSFOfLayoutChange(), "
+            "the TSFTextStore instance is destroyed during calling "
+            "OnLayoutChange()",
+            this));
+    return ret;
+  }
+
+  if (!mWaitingQueryLayout) {
+    MOZ_LOG(sTextStoreLog, LogLevel::Info,
+           ("TSF: 0x%p   TSFTextStore::NotifyTSFOfLayoutChange(), "
+            "succeeded notifying TIP of our layout change",
+            this));
+    return ret;
+  }
+
+  // If TIP hasn't accessed our new layout information yet, TSF and/or TIP may
+  // have met some trouble during calls of OnLayoutChange().  It should be
+  // tried again later.
+  mHasReturnedNoLayoutError = returnedNoLayoutError;
+  ::PostMessage(mWidget->GetWindowHandle(),
+                MOZ_WM_NOTIY_TSF_OF_LAYOUT_CHANGE,
+                reinterpret_cast<WPARAM>(this), 0);
+
+  return true;
+}
+
+void
+TSFTextStore::NotifyTSFOfLayoutChangeAgain()
+{
+  // Before preforming this method, TIP has accessed our layout information,
+  // we don't need to notify TIP of layout change anymore.
+  if (!mWaitingQueryLayout) {
+    return;
+  }
+
+  MOZ_LOG(sTextStoreLog, LogLevel::Info,
+         ("TSF: 0x%p   TSFTextStore::NotifyTSFOfLayoutChangeAgain(), "
+          "calling NotifyTSFOfLayoutChange()...", this));
+  NotifyTSFOfLayoutChange();
+  MOZ_LOG(sTextStoreLog, LogLevel::Info,
+         ("TSF: 0x%p   TSFTextStore::NotifyTSFOfLayoutChangeAgain(), "
+          "called NotifyTSFOfLayoutChange()", this));
 }
 
 nsresult
 TSFTextStore::OnUpdateCompositionInternal()
 {
   MOZ_LOG(sTextStoreLog, LogLevel::Debug,
     ("TSF: 0x%p   TSFTextStore::OnUpdateCompositionInternal(), "
      "mDeferNotifyingTSF=%s",
@@ -5457,16 +5529,23 @@ TSFTextStore::ProcessMessage(nsWindowBas
     case WM_ENTERIDLE:
       // When an modal dialog such as a file picker is open, composition
       // should be committed because IME might be used on it.
       if (!IsComposingOn(aWindow)) {
         break;
       }
       CommitComposition(false);
       break;
+    case MOZ_WM_NOTIY_TSF_OF_LAYOUT_CHANGE: {
+      TSFTextStore* textStore = reinterpret_cast<TSFTextStore*>(aWParam);
+      if (textStore == sEnabledTextStore) {
+        textStore->NotifyTSFOfLayoutChangeAgain();
+      }
+      break;
+    }
   }
 }
 
 // static
 bool
 TSFTextStore::IsIMM_IME()
 {
   return TSFStaticSink::IsIMM_IME();
--- a/widget/windows/TSFTextStore.h
+++ b/widget/windows/TSFTextStore.h
@@ -291,17 +291,18 @@ protected:
   // MaybeFlushPendingNotifications() performs pending notifications to TSF.
   void     MaybeFlushPendingNotifications();
 
   nsresult OnLayoutChangeInternal();
   nsresult OnUpdateCompositionInternal();
 
   void     NotifyTSFOfTextChange(const TS_TEXTCHANGE& aTextChange);
   void     NotifyTSFOfSelectionChange();
-  bool     NotifyTSFOfLayoutChange(bool aFlush);
+  bool     NotifyTSFOfLayoutChange();
+  void     NotifyTSFOfLayoutChangeAgain();
 
   HRESULT  HandleRequestAttrs(DWORD aFlags,
                               ULONG aFilterCount,
                               const TS_ATTRID* aFilterAttrs);
   void     SetInputScope(const nsString& aHTMLInputType);
 
   // Creates native caret over our caret.  This method only works on desktop
   // application.  Otherwise, this does nothing.
@@ -823,21 +824,24 @@ protected:
   // because it may cause TSF request new lock.  This is a problem if the
   // selection change is caused by a call of On*Composition() without document
   // lock since RequestLock() tries to flush the pending actions again (which
   // are flushing).  Therefore, OnSelectionChangeInternal() sets this true
   // during recoding actions and then, RequestLock() will call
   // 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;
+  // calculated yet, these methods return TS_E_NOLAYOUT.  At that time,
+  // mHasReturnedNoLayoutError is set to true.
+  bool                         mHasReturnedNoLayoutError;
+  // Before calling ITextStoreACPSink::OnLayoutChange() and
+  // ITfContextOwnerServices::OnLayoutChange(), mWaitingQueryLayout is set to
+  // true.  This is set to  false when GetTextExt() or GetACPFromPoint() is
+  // called.
+  bool                         mWaitingQueryLayout;
   // During the documet is locked, we shouldn't destroy the instance.
   // If this is true, the instance will be destroyed after unlocked.
   bool                         mPendingDestroy;
   // If this is false, MaybeFlushPendingNotifications() will clear the
   // mLockedContent.
   bool                         mDeferClearingLockedContent;
   // While there is native caret, this is true.  Otherwise, false.
   bool                         mNativeCaretIsCreated;
--- a/widget/windows/nsWindowDefs.h
+++ b/widget/windows/nsWindowDefs.h
@@ -31,17 +31,19 @@
 #define MOZ_WM_MOUSEHWHEEL                (WM_APP+0x0311)
 #define MOZ_WM_VSCROLL                    (WM_APP+0x0312)
 #define MOZ_WM_HSCROLL                    (WM_APP+0x0313)
 #define MOZ_WM_MOUSEWHEEL_FIRST           MOZ_WM_MOUSEVWHEEL
 #define MOZ_WM_MOUSEWHEEL_LAST            MOZ_WM_HSCROLL
 // If a popup window is being activated, we try to reactivate the previous
 // window with this message.
 #define MOZ_WM_REACTIVATE                 (WM_APP+0x0314)
-
+// If TSFTextStore needs to notify TSF/TIP of layout change later, this
+// message is posted.
+#define MOZ_WM_NOTIY_TSF_OF_LAYOUT_CHANGE (WM_APP+0x0315)
 // Internal message for ensuring the file picker is visible on multi monitor
 // systems, and when the screen resolution changes.
 #define MOZ_WM_ENSUREVISIBLE              (WM_APP+0x374F)
 
 #ifndef SM_CXPADDEDBORDER
 #define SM_CXPADDEDBORDER                 92
 #endif