Bug 1255627 Don't call methods of TSFTextStore::sEnabledTextStore without independent strong reference to it r?m_kato
TSFTextStore::sEnabledTextStore is a static variable to grab a reference to focused TextStore instance. So, this may be changed by accidentally during a call of instance methods of TSFTextStore. Then, focused TextStore may be destroyed during running a method and crash when it accesses a member variable.
For avoiding this crash, static methods which call a method of sEnabledTextStore should create an independent RefPtr to it before calling the method.
MozReview-Commit-ID: 51Sor1LdABr
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -4521,66 +4521,66 @@ TSFTextStore::OnFocusChange(bool aGotFoc
// static
bool
TSFTextStore::CreateAndSetFocus(nsWindowBase* aFocusedWidget,
const InputContext& aContext)
{
// TSF might do something which causes that we need to access static methods
// of TSFTextStore. At that time, sEnabledTextStore may be necessary.
// So, we should set sEnabledTextStore directly.
- sEnabledTextStore = new TSFTextStore();
- if (NS_WARN_IF(!sEnabledTextStore->Init(aFocusedWidget))) {
+ RefPtr<TSFTextStore> textStore = new TSFTextStore();
+ sEnabledTextStore = textStore;
+ if (NS_WARN_IF(!textStore->Init(aFocusedWidget))) {
MOZ_LOG(sTextStoreLog, LogLevel::Error,
("TSF: TSFTextStore::CreateAndSetFocus() FAILED due to "
"TSFTextStore::Init() failure"));
return false;
}
- if (NS_WARN_IF(!sEnabledTextStore->mDocumentMgr)) {
+ if (NS_WARN_IF(!textStore->mDocumentMgr)) {
MOZ_LOG(sTextStoreLog, LogLevel::Error,
("TSF: TSFTextStore::CreateAndSetFocus() FAILED due to "
"invalid TSFTextStore::mDocumentMgr"));
return false;
}
if (aContext.mIMEState.mEnabled == IMEState::PASSWORD) {
- MarkContextAsKeyboardDisabled(sEnabledTextStore->mContext);
+ MarkContextAsKeyboardDisabled(textStore->mContext);
RefPtr<ITfContext> topContext;
- sEnabledTextStore->mDocumentMgr->GetTop(getter_AddRefs(topContext));
- if (topContext && topContext != sEnabledTextStore->mContext) {
+ textStore->mDocumentMgr->GetTop(getter_AddRefs(topContext));
+ if (topContext && topContext != textStore->mContext) {
MarkContextAsKeyboardDisabled(topContext);
}
}
- HRESULT hr = sThreadMgr->SetFocus(sEnabledTextStore->mDocumentMgr);
+ HRESULT hr = sThreadMgr->SetFocus(textStore->mDocumentMgr);
if (NS_WARN_IF(FAILED(hr))) {
MOZ_LOG(sTextStoreLog, LogLevel::Error,
("TSF: TSFTextStore::CreateAndSetFocus() FAILED due to "
"ITfTheadMgr::SetFocus() failure"));
return false;
}
// Use AssociateFocus() for ensuring that any native focus event
// never steal focus from our documentMgr.
RefPtr<ITfDocumentMgr> prevFocusedDocumentMgr;
hr = sThreadMgr->AssociateFocus(aFocusedWidget->GetWindowHandle(),
- sEnabledTextStore->mDocumentMgr,
+ textStore->mDocumentMgr,
getter_AddRefs(prevFocusedDocumentMgr));
if (NS_WARN_IF(FAILED(hr))) {
MOZ_LOG(sTextStoreLog, LogLevel::Error,
("TSF: TSFTextStore::CreateAndSetFocus() FAILED due to "
"ITfTheadMgr::AssociateFocus() failure"));
return false;
}
- sEnabledTextStore->SetInputScope(aContext.mHTMLInputType,
- aContext.mHTMLInputInputmode);
-
- if (sEnabledTextStore->mSink) {
+ textStore->SetInputScope(aContext.mHTMLInputType,
+ aContext.mHTMLInputInputmode);
+
+ if (textStore->mSink) {
MOZ_LOG(sTextStoreLog, LogLevel::Info,
("TSF: TSFTextStore::CreateAndSetFocus(), calling "
"ITextStoreACPSink::OnLayoutChange(TS_LC_CREATE) for 0x%p...",
- sEnabledTextStore.get()));
- sEnabledTextStore->mSink->OnLayoutChange(TS_LC_CREATE,
- TEXTSTORE_DEFAULT_VIEW);
+ textStore.get()));
+ textStore->mSink->OnLayoutChange(TS_LC_CREATE, TEXTSTORE_DEFAULT_VIEW);
}
return true;
}
// static
nsIMEUpdatePreference
TSFTextStore::GetIMEUpdatePreference()
{
@@ -5299,18 +5299,19 @@ TSFTextStore::SetInputContext(nsWindowBa
aWidget, GetIMEEnabledName(aContext.mIMEState.mEnabled),
GetFocusChangeName(aAction.mFocusChange), sEnabledTextStore.get(),
GetBoolName(ThinksHavingFocus())));
NS_ENSURE_TRUE_VOID(IsInTSFMode());
if (aAction.mFocusChange != InputContextAction::FOCUS_NOT_CHANGED) {
if (sEnabledTextStore) {
- sEnabledTextStore->SetInputScope(aContext.mHTMLInputType,
- aContext.mHTMLInputInputmode);
+ RefPtr<TSFTextStore> textStore = sEnabledTextStore;
+ textStore->SetInputScope(aContext.mHTMLInputType,
+ aContext.mHTMLInputInputmode);
}
return;
}
// If focus isn't actually changed but the enabled state is changed,
// emulate the focus move.
if (!ThinksHavingFocus() && aContext.mIMEState.IsEditable()) {
OnFocusChange(true, aWidget, aContext);
@@ -5641,18 +5642,19 @@ TSFTextStore::ProcessMessage(nsWindowBas
// 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) {
+ TSFTextStore* maybeTextStore = reinterpret_cast<TSFTextStore*>(aWParam);
+ if (maybeTextStore == sEnabledTextStore) {
+ RefPtr<TSFTextStore> textStore(maybeTextStore);
textStore->NotifyTSFOfLayoutChangeAgain();
}
break;
}
}
}
// static
--- a/widget/windows/TSFTextStore.h
+++ b/widget/windows/TSFTextStore.h
@@ -111,61 +111,78 @@ public:
static void SetIMEOpenState(bool);
static bool GetIMEOpenState(void);
static void CommitComposition(bool aDiscard)
{
NS_ASSERTION(IsInTSFMode(), "Not in TSF mode, shouldn't be called");
- if (sEnabledTextStore) {
- sEnabledTextStore->CommitCompositionInternal(aDiscard);
+ if (!sEnabledTextStore) {
+ return;
}
+ RefPtr<TSFTextStore> textStore = sEnabledTextStore;
+ textStore->CommitCompositionInternal(aDiscard);
}
static void SetInputContext(nsWindowBase* aWidget,
const InputContext& aContext,
const InputContextAction& aAction);
static nsresult OnFocusChange(bool aGotFocus,
nsWindowBase* aFocusedWidget,
const InputContext& aContext);
static nsresult OnTextChange(const IMENotification& aIMENotification)
{
NS_ASSERTION(IsInTSFMode(), "Not in TSF mode, shouldn't be called");
- return sEnabledTextStore ?
- sEnabledTextStore->OnTextChangeInternal(aIMENotification) : NS_OK;
+ if (!sEnabledTextStore) {
+ return NS_OK;
+ }
+ RefPtr<TSFTextStore> textStore = sEnabledTextStore;
+ return textStore->OnTextChangeInternal(aIMENotification);
}
static nsresult OnSelectionChange(const IMENotification& aIMENotification)
{
NS_ASSERTION(IsInTSFMode(), "Not in TSF mode, shouldn't be called");
- return sEnabledTextStore ?
- sEnabledTextStore->OnSelectionChangeInternal(aIMENotification) : NS_OK;
+ if (!sEnabledTextStore) {
+ return NS_OK;
+ }
+ RefPtr<TSFTextStore> textStore = sEnabledTextStore;
+ return textStore->OnSelectionChangeInternal(aIMENotification);
}
static nsresult OnLayoutChange()
{
NS_ASSERTION(IsInTSFMode(), "Not in TSF mode, shouldn't be called");
- return sEnabledTextStore ?
- sEnabledTextStore->OnLayoutChangeInternal() : NS_OK;
+ if (!sEnabledTextStore) {
+ return NS_OK;
+ }
+ RefPtr<TSFTextStore> textStore = sEnabledTextStore;
+ return textStore->OnLayoutChangeInternal();
}
static nsresult OnUpdateComposition()
{
NS_ASSERTION(IsInTSFMode(), "Not in TSF mode, shouldn't be called");
- return sEnabledTextStore ?
- sEnabledTextStore->OnUpdateCompositionInternal() : NS_OK;
+ if (!sEnabledTextStore) {
+ return NS_OK;
+ }
+ RefPtr<TSFTextStore> textStore = sEnabledTextStore;
+ return textStore->OnUpdateCompositionInternal();
}
static nsresult OnMouseButtonEvent(const IMENotification& aIMENotification)
{
NS_ASSERTION(IsInTSFMode(), "Not in TSF mode, shouldn't be called");
- return sEnabledTextStore ?
- sEnabledTextStore->OnMouseButtonEventInternal(aIMENotification) : NS_OK;
+ if (!sEnabledTextStore) {
+ return NS_OK;
+ }
+ RefPtr<TSFTextStore> textStore = sEnabledTextStore;
+ return textStore->OnMouseButtonEventInternal(aIMENotification);
}
static nsIMEUpdatePreference GetIMEUpdatePreference();
// Returns the address of the pointer so that the TSF automatic test can
// replace the system object with a custom implementation for testing.
// XXX TSF doesn't work now. Should we remove it?
static void* GetNativeData(uint32_t aDataType)