From 369e8fb38b3d649461ce5a13a00b494b18b55abe Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 30 Dec 2015 01:05:46 +0900 Subject: [PATCH] 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 --- widget/windows/TSFTextStore.cpp | 123 ++++++++++++++++++++++++++------ widget/windows/TSFTextStore.h | 16 +++-- widget/windows/nsWindowDefs.h | 4 +- 3 files changed, 114 insertions(+), 29 deletions(-) diff --git a/widget/windows/TSFTextStore.cpp b/widget/windows/TSFTextStore.cpp index 9f530861f4c..5bc143dff24 100644 --- a/widget/windows/TSFTextStore.cpp +++ b/widget/windows/TSFTextStore.cpp @@ -1315,7 +1315,8 @@ TSFTextStore::TSFTextStore() , mRequestedAttrValues(false) , mIsRecordingActionsWithoutLock(false) , mPendingOnSelectionChange(false) - , mPendingOnLayoutChange(false) + , mHasReturnedNoLayoutError(false) + , mWaitingQueryLayout(false) , mPendingDestroy(false) , mDeferClearingLockedContent(false) , mNativeCaretIsCreated(false) @@ -1670,7 +1671,7 @@ TSFTextStore::DidLockGranted() // If the widget has gone, we don't need to notify anything. if (!mWidget || mWidget->Destroyed()) { mPendingOnSelectionChange = false; - mPendingOnLayoutChange = false; + mHasReturnedNoLayoutError = false; } } @@ -1695,7 +1696,7 @@ TSFTextStore::FlushPendingActions() mPendingActions.Clear(); mLockedContent.Clear(); mPendingOnSelectionChange = false; - mPendingOnLayoutChange = false; + mHasReturnedNoLayoutError = false; return; } @@ -1901,11 +1902,11 @@ 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) { @@ -3339,10 +3340,11 @@ TSFTextStore::GetACPFromPoint(TsViewCookie vcView, { 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, @@ -3372,11 +3374,13 @@ TSFTextStore::GetACPFromPoint(TsViewCookie vcView, 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; } @@ -3492,9 +3496,9 @@ TSFTextStore::GetTextExt(TsViewCookie vcView, 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, @@ -3524,6 +3528,8 @@ TSFTextStore::GetTextExt(TsViewCookie vcView, 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. @@ -3611,9 +3617,9 @@ TSFTextStore::GetTextExt(TsViewCookie vcView, 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; } @@ -4768,7 +4774,7 @@ TSFTextStore::OnLayoutChangeInternal() 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; } @@ -4781,9 +4787,16 @@ TSFTextStore::OnLayoutChangeInternal() } 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. @@ -4796,7 +4809,7 @@ TSFTextStore::NotifyTSFOfLayoutChange(bool aFlush) MaybeDestroyNativeCaret(); // This method should return true if either way succeeds. - bool ret = false; + bool ret = true; if (mSink) { MOZ_LOG(sTextStoreLog, LogLevel::Info, @@ -4804,12 +4817,16 @@ TSFTextStore::NotifyTSFOfLayoutChange(bool aFlush) "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 service; mContext->QueryInterface(IID_ITfContextOwnerServices, getter_AddRefs(service)); @@ -4819,11 +4836,66 @@ TSFTextStore::NotifyTSFOfLayoutChange(bool aFlush) "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(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 @@ -5462,6 +5534,13 @@ TSFTextStore::ProcessMessage(nsWindowBase* aWindow, } CommitComposition(false); break; + case MOZ_WM_NOTIY_TSF_OF_LAYOUT_CHANGE: { + TSFTextStore* textStore = reinterpret_cast(aWParam); + if (textStore == sEnabledTextStore) { + textStore->NotifyTSFOfLayoutChangeAgain(); + } + break; + } } } diff --git a/widget/windows/TSFTextStore.h b/widget/windows/TSFTextStore.h index 816a6c1de57..b97b65c6221 100644 --- a/widget/windows/TSFTextStore.h +++ b/widget/windows/TSFTextStore.h @@ -296,7 +296,8 @@ protected: void NotifyTSFOfTextChange(const TS_TEXTCHANGE& aTextChange); void NotifyTSFOfSelectionChange(); - bool NotifyTSFOfLayoutChange(bool aFlush); + bool NotifyTSFOfLayoutChange(); + void NotifyTSFOfLayoutChangeAgain(); HRESULT HandleRequestAttrs(DWORD aFlags, ULONG aFilterCount, @@ -828,11 +829,14 @@ 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; + // 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; diff --git a/widget/windows/nsWindowDefs.h b/widget/windows/nsWindowDefs.h index c64588162f7..a4217fbc7d9 100644 --- a/widget/windows/nsWindowDefs.h +++ b/widget/windows/nsWindowDefs.h @@ -36,7 +36,9 @@ // 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)