Skip to content

Commit

Permalink
Bug 1377672 - part3: IMEStateManager::NotifyIME() should ignore notif…
Browse files Browse the repository at this point in the history 
…ications and requests which comes from unexpected process r=m_kato,smaug

IME should receive notifications and requests only from proper process.  E.g., IME shouldn't commit composition by a request which came from previous focused process.

This patch makes that IMEStateManager::NotifyIME() takes pointer to TabParent optionally.  If the request or notification came from remote process, it should be non-nullptr.  Then, this makes it ignore notifications and requests from unexpected process.

Note that this patch also touches some gfx headers because they use |ipc::| but compiler is confused at the ambiguousness between |mozilla::ipc::| and |mozilla::dom::ipc::|.

Finally, this patch changes the NS_ASSERTION in IMEHandler::OnDestroyWindow() to MOZ_ASSERT because the orange caused by the NS_ASSERTION was not realized since there was already an intermittent orange bug caused by different NS_ASSERTION.

MozReview-Commit-ID: 9CgKXQRJWmN
  • Loading branch information
@masayuki-nakano
masayuki-nakano committed Jul 5, 2017
1 parent 0609c7e commit 3caf001
Show file tree
Hide file tree
Showing 10 changed files with 154 additions and 122 deletions.
185 changes: 98 additions & 87 deletions dom/events/IMEStateManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,23 @@ GetIMEStateSetOpenName(IMEState::Open aOpen)
}
}

static bool
IsSameProcess(const TabParent* aTabParent1, const TabParent* aTabParent2)
{
if (aTabParent1 == aTabParent2) {
return true;
}
if (!aTabParent1 != !aTabParent2) {
return false;
}
return aTabParent1->Manager() == aTabParent2->Manager();
}

StaticRefPtr<nsIContent> IMEStateManager::sContent;
StaticRefPtr<nsPresContext> IMEStateManager::sPresContext;
nsIWidget* IMEStateManager::sWidget = nullptr;
nsIWidget* IMEStateManager::sFocusedIMEWidget = nullptr;
StaticRefPtr<TabParent> IMEStateManager::sFocusedIMETabParent;
nsIWidget* IMEStateManager::sActiveInputContextWidget = nullptr;
StaticRefPtr<TabParent> IMEStateManager::sActiveTabParent;
StaticRefPtr<IMEContentObserver> IMEStateManager::sActiveIMEContentObserver;
Expand All @@ -149,7 +162,6 @@ bool IMEStateManager::sInstalledMenuKeyboardListener = false;
bool IMEStateManager::sIsGettingNewIMEState = false;
bool IMEStateManager::sCheckForIMEUnawareWebApps = false;
bool IMEStateManager::sInputModeSupported = false;
bool IMEStateManager::sRemoteHasFocus = false;

// static
void
Expand Down Expand Up @@ -226,7 +238,7 @@ IMEStateManager::StopIMEStateManagement()
// the rights to change input context.

if (sTextCompositions && sPresContext) {
NotifyIME(REQUEST_TO_COMMIT_COMPOSITION, sPresContext);
NotifyIME(REQUEST_TO_COMMIT_COMPOSITION, sPresContext, sActiveTabParent);
}
sActiveInputContextWidget = nullptr;
sPresContext = nullptr;
Expand Down Expand Up @@ -444,7 +456,7 @@ IMEStateManager::OnChangeFocusInternal(nsPresContext* aPresContext,
// If we're deactivating, we shouldn't commit composition forcibly because
// the user may want to continue the composition.
if (aPresContext) {
NotifyIME(REQUEST_TO_COMMIT_COMPOSITION, oldWidget);
NotifyIME(REQUEST_TO_COMMIT_COMPOSITION, oldWidget, sFocusedIMETabParent);
}
}

Expand Down Expand Up @@ -478,11 +490,7 @@ IMEStateManager::OnChangeFocusInternal(nsPresContext* aPresContext,
return NS_OK;
}

nsIContentParent* currentContentParent =
sActiveTabParent ? sActiveTabParent->Manager() : nullptr;
nsIContentParent* newContentParent =
newTabParent ? newTabParent->Manager() : nullptr;
if (sActiveTabParent && currentContentParent != newContentParent) {
if (sActiveTabParent && !IsSameProcess(sActiveTabParent, newTabParent)) {
MOZ_LOG(sISMLog, LogLevel::Debug,
(" OnChangeFocusInternal(), notifying previous "
"focused child process of parent process or another child process "
Expand Down Expand Up @@ -565,7 +573,8 @@ IMEStateManager::OnChangeFocusInternal(nsPresContext* aPresContext,
// Even if focus isn't changing actually, we should commit current
// composition here since the IME state is changing.
if (sPresContext && oldWidget && !focusActuallyChanging) {
NotifyIME(REQUEST_TO_COMMIT_COMPOSITION, oldWidget);
NotifyIME(REQUEST_TO_COMMIT_COMPOSITION, oldWidget,
sFocusedIMETabParent);
}
} else if (aAction.mFocusChange == InputContextAction::FOCUS_NOT_CHANGED) {
// If aContent isn't null or aContent is null but editable, somebody gets
Expand Down Expand Up @@ -923,7 +932,7 @@ IMEStateManager::UpdateIMEState(const IMEState& aNewIMEState,

if (updateIMEState) {
// commit current composition before modifying IME state.
NotifyIME(REQUEST_TO_COMMIT_COMPOSITION, widget);
NotifyIME(REQUEST_TO_COMMIT_COMPOSITION, widget, sFocusedIMETabParent);
if (NS_WARN_IF(widget->Destroyed())) {
MOZ_LOG(sISMLog, LogLevel::Error,
(" UpdateIMEState(), widget has gone during committing composition"));
Expand Down Expand Up @@ -1443,25 +1452,29 @@ IMEStateManager::OnCompositionEventDiscarded(
nsresult
IMEStateManager::NotifyIME(IMEMessage aMessage,
nsIWidget* aWidget,
bool aOriginIsRemote)
TabParent* aTabParent)
{
return IMEStateManager::NotifyIME(IMENotification(aMessage), aWidget,
aOriginIsRemote);
aTabParent);
}

// static
nsresult
IMEStateManager::NotifyIME(const IMENotification& aNotification,
nsIWidget* aWidget,
bool aOriginIsRemote)
TabParent* aTabParent)
{
MOZ_LOG(sISMLog, LogLevel::Info,
("NotifyIME(aNotification={ mMessage=%s }, "
"aWidget=0x%p, aOriginIsRemote=%s), sFocusedIMEWidget=0x%p, "
"sRemoteHasFocus=%s",
"aWidget=0x%p, aTabParent=0x%p), sFocusedIMEWidget=0x%p, "
"sActiveTabParent=0x%p, sFocusedIMETabParent=0x%p, "
"IsSameProcess(aTabParent, sActiveTabParent)=%s, "
"IsSameProcess(aTabParent, sFocusedIMETabParent)=%s",
ToChar(aNotification.mMessage), aWidget,
GetBoolName(aOriginIsRemote), sFocusedIMEWidget,
GetBoolName(sRemoteHasFocus)));
aTabParent, sFocusedIMEWidget, sActiveTabParent.get(),
sFocusedIMETabParent.get(),
GetBoolName(IsSameProcess(aTabParent, sActiveTabParent)),
GetBoolName(IsSameProcess(aTabParent, sFocusedIMETabParent))));

if (NS_WARN_IF(!aWidget)) {
MOZ_LOG(sISMLog, LogLevel::Error,
Expand All @@ -1471,92 +1484,81 @@ IMEStateManager::NotifyIME(const IMENotification& aNotification,

switch (aNotification.mMessage) {
case NOTIFY_IME_OF_FOCUS: {
// If focus notification comes from a remote process which already lost
// focus, we shouldn't accept the focus notification. Then, following
// notifications from the process will be ignored.
if (NS_WARN_IF(!IsSameProcess(aTabParent, sActiveTabParent))) {
MOZ_ASSERT(aTabParent,
"Why was the input context initialized for a remote process but "
"does this process get IME focus?");
MOZ_LOG(sISMLog, LogLevel::Warning,
(" NotifyIME(), WARNING, the received focus notification is ignored "
"because input context was initialized for %s, perhaps, it came "
"from a busy remote process",
sActiveTabParent ? "another remote process" : "current process"));
return NS_OK;
}
// If there is pending blur notification for current focused IME,
// we should notify IME of blur by ourselves. Then, we should ignore
// following notifications coming from the process.
if (sFocusedIMEWidget) {
if (NS_WARN_IF(!sRemoteHasFocus && !aOriginIsRemote)) {
MOZ_LOG(sISMLog, LogLevel::Error,
(" NotifyIME(), although, this process is "
"getting IME focus but there was focused IME widget"));
} else {
MOZ_LOG(sISMLog, LogLevel::Info,
(" NotifyIME(), tries to notify IME of "
"blur first because remote process's blur notification hasn't "
"been received yet..."));
}
MOZ_ASSERT(sFocusedIMETabParent || aTabParent,
"This case shouldn't be caused by focus move in this process");
nsCOMPtr<nsIWidget> focusedIMEWidget(sFocusedIMEWidget);
sFocusedIMEWidget = nullptr;
sRemoteHasFocus = false;
sFocusedIMETabParent = nullptr;
focusedIMEWidget->NotifyIME(IMENotification(NOTIFY_IME_OF_BLUR));
}
sRemoteHasFocus = aOriginIsRemote;
sFocusedIMETabParent = aTabParent;
sFocusedIMEWidget = aWidget;
nsCOMPtr<nsIWidget> widget(aWidget);
return widget->NotifyIME(aNotification);
}
case NOTIFY_IME_OF_BLUR: {
if (!sRemoteHasFocus && aOriginIsRemote) {
MOZ_LOG(sISMLog, LogLevel::Info,
(" NotifyIME(), received blur notification "
"after another one has focus, nothing to do..."));
return NS_OK;
}
if (NS_WARN_IF(sRemoteHasFocus && !aOriginIsRemote)) {
MOZ_LOG(sISMLog, LogLevel::Error,
(" NotifyIME(), FAILED, received blur "
"notification from this process but the remote has focus"));
if (!IsSameProcess(aTabParent, sFocusedIMETabParent)) {
MOZ_LOG(sISMLog, LogLevel::Warning,
(" NotifyIME(), WARNING, the received blur notification is ignored "
"because it's not from current focused IME process"));
return NS_OK;
}
if (!sFocusedIMEWidget && aOriginIsRemote) {
MOZ_LOG(sISMLog, LogLevel::Info,
(" NotifyIME(), received blur notification "
"but the remote has already lost focus"));
return NS_OK;
}
if (NS_WARN_IF(!sFocusedIMEWidget)) {
if (!sFocusedIMEWidget) {
MOZ_LOG(sISMLog, LogLevel::Error,
(" NotifyIME(), FAILED, received blur "
"notification but there is no focused IME widget"));
(" NotifyIME(), WARNING, received blur notification but there is "
"no focused IME widget"));
return NS_OK;
}
if (NS_WARN_IF(sFocusedIMEWidget != aWidget)) {
MOZ_LOG(sISMLog, LogLevel::Error,
(" NotifyIME(), FAILED, received blur "
"notification but there is no focused IME widget"));
MOZ_LOG(sISMLog, LogLevel::Warning,
(" NotifyIME(), WARNING, the received blur notification is ignored "
"because it's not for current focused IME widget"));
return NS_OK;
}
nsCOMPtr<nsIWidget> focusedIMEWidget(sFocusedIMEWidget);
sFocusedIMEWidget = nullptr;
sRemoteHasFocus = false;
sFocusedIMETabParent = nullptr;
return focusedIMEWidget->NotifyIME(IMENotification(NOTIFY_IME_OF_BLUR));
}
case NOTIFY_IME_OF_SELECTION_CHANGE:
case NOTIFY_IME_OF_TEXT_CHANGE:
case NOTIFY_IME_OF_POSITION_CHANGE:
case NOTIFY_IME_OF_MOUSE_BUTTON_EVENT:
case NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED: {
if (!sRemoteHasFocus && aOriginIsRemote) {
MOZ_LOG(sISMLog, LogLevel::Info,
(" NotifyIME(), received content change "
"notification from the remote but it's already lost focus"));
return NS_OK;
}
if (NS_WARN_IF(sRemoteHasFocus && !aOriginIsRemote)) {
MOZ_LOG(sISMLog, LogLevel::Error,
(" NotifyIME(), FAILED, received content "
"change notification from this process but the remote has already "
"gotten focus"));
if (!IsSameProcess(aTabParent, sFocusedIMETabParent)) {
MOZ_LOG(sISMLog, LogLevel::Warning,
(" NotifyIME(), WARNING, the received content change notification "
"is ignored because it's not from current focused IME process"));
return NS_OK;
}
if (!sFocusedIMEWidget) {
MOZ_LOG(sISMLog, LogLevel::Info,
(" NotifyIME(), received content change "
"notification but there is no focused IME widget"));
MOZ_LOG(sISMLog, LogLevel::Warning,
(" NotifyIME(), WARNING, the received content change notification "
"is ignored because there is no focused IME widget"));
return NS_OK;
}
if (NS_WARN_IF(sFocusedIMEWidget != aWidget)) {
MOZ_LOG(sISMLog, LogLevel::Error,
(" NotifyIME(), FAILED, received content "
"change notification for IME which has already lost focus, so, "
"nothing to do..."));
MOZ_LOG(sISMLog, LogLevel::Warning,
(" NotifyIME(), WARNING, the received content change notification "
"is ignored because it's not for current focused IME widget"));
return NS_OK;
}
nsCOMPtr<nsIWidget> widget(aWidget);
Expand All @@ -1568,26 +1570,35 @@ IMEStateManager::NotifyIME(const IMENotification& aNotification,
break;
}

RefPtr<TextComposition> composition;
if (sTextCompositions) {
composition = sTextCompositions->GetCompositionFor(aWidget);
if (!sTextCompositions) {
MOZ_LOG(sISMLog, LogLevel::Info,
(" NotifyIME(), the request to IME is ignored because "
"there have been no compositions yet"));
return NS_OK;
}

bool isSynthesizedForTests =
composition && composition->IsSynthesizedForTests();
RefPtr<TextComposition> composition =
sTextCompositions->GetCompositionFor(aWidget);
if (!composition) {
MOZ_LOG(sISMLog, LogLevel::Info,
(" NotifyIME(), the request to IME is ignored because "
"there is no active composition"));
return NS_OK;
}

MOZ_LOG(sISMLog, LogLevel::Info,
(" NotifyIME(), composition=0x%p, "
"composition->IsSynthesizedForTests()=%s",
composition.get(), GetBoolName(isSynthesizedForTests)));
if (!IsSameProcess(aTabParent, composition->GetTabParent())) {
MOZ_LOG(sISMLog, LogLevel::Warning,
(" NotifyIME(), WARNING, the request to IME is ignored because "
"it does not come from the remote process which has the composition "
"on aWidget"));
return NS_OK;
}

switch (aNotification.mMessage) {
case REQUEST_TO_COMMIT_COMPOSITION:
return composition ?
composition->RequestToCommit(aWidget, false) : NS_OK;
return composition->RequestToCommit(aWidget, false);
case REQUEST_TO_CANCEL_COMPOSITION:
return composition ?
composition->RequestToCommit(aWidget, true) : NS_OK;
return composition->RequestToCommit(aWidget, true);
default:
MOZ_CRASH("Unsupported notification");
}
Expand All @@ -1600,11 +1611,11 @@ IMEStateManager::NotifyIME(const IMENotification& aNotification,
nsresult
IMEStateManager::NotifyIME(IMEMessage aMessage,
nsPresContext* aPresContext,
bool aOriginIsRemote)
TabParent* aTabParent)
{
MOZ_LOG(sISMLog, LogLevel::Info,
("NotifyIME(aMessage=%s, aPresContext=0x%p, aOriginIsRemote=%s)",
ToChar(aMessage), aPresContext, GetBoolName(aOriginIsRemote)));
("NotifyIME(aMessage=%s, aPresContext=0x%p, aTabParent=0x%p)",
ToChar(aMessage), aPresContext, aTabParent));

if (NS_WARN_IF(!CanHandleWith(aPresContext))) {
return NS_ERROR_INVALID_ARG;
Expand All @@ -1617,7 +1628,7 @@ IMEStateManager::NotifyIME(IMEMessage aMessage,
"nsPresContext"));
return NS_ERROR_NOT_AVAILABLE;
}
return NotifyIME(aMessage, widget, aOriginIsRemote);
return NotifyIME(aMessage, widget, aTabParent);
}

// static
Expand Down
14 changes: 6 additions & 8 deletions dom/events/IMEStateManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,13 @@ class IMEStateManager
*/
static nsresult NotifyIME(const IMENotification& aNotification,
nsIWidget* aWidget,
bool aOriginIsRemote = false);
TabParent* aTabParent = nullptr);
static nsresult NotifyIME(IMEMessage aMessage,
nsIWidget* aWidget,
bool aOriginIsRemote = false);
TabParent* aTabParent = nullptr);
static nsresult NotifyIME(IMEMessage aMessage,
nsPresContext* aPresContext,
bool aOriginIsRemote = false);
TabParent* aTabParent = nullptr);

static nsINode* GetRootEditableNode(nsPresContext* aPresContext,
nsIContent* aContent);
Expand Down Expand Up @@ -280,11 +280,10 @@ class IMEStateManager
// sPresContext has gone, we need to clean up some IME state on the widget
// if the widget is available.
static nsIWidget* sWidget;
// sFocusedIMEWidget is, the widget which was sent to "focus" notification
// from IMEContentObserver and not yet sent "blur" notification.
// So, if this is not nullptr, the widget needs to receive "blur"
// notification.
// sFocusedIMETabParent is the tab parent, which send "focus" notification to
// sFocusedIMEWidget (and didn't yet sent "blur" notification).
static nsIWidget* sFocusedIMEWidget;
static StaticRefPtr<TabParent> sFocusedIMETabParent;
// sActiveInputContextWidget is the last widget whose SetInputContext() is
// called. This is important to reduce sync IPC cost with parent process.
// If IMEStateManager set input context to different widget, PuppetWidget can
Expand All @@ -308,7 +307,6 @@ class IMEStateManager
static bool sIsGettingNewIMEState;
static bool sCheckForIMEUnawareWebApps;
static bool sInputModeSupported;
static bool sRemoteHasFocus;

class MOZ_STACK_CLASS GettingNewIMEStateBlocker final
{
Expand Down
2 changes: 1 addition & 1 deletion dom/events/TextComposition.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ nsresult
TextComposition::NotifyIME(IMEMessage aMessage)
{
NS_ENSURE_TRUE(mPresContext, NS_ERROR_NOT_AVAILABLE);
return IMEStateManager::NotifyIME(aMessage, mPresContext);
return IMEStateManager::NotifyIME(aMessage, mPresContext, mTabParent);
}

void
Expand Down
5 changes: 5 additions & 0 deletions dom/events/TextComposition.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ class TextComposition final
{
return mPresContext ? mPresContext->GetRootWidget() : nullptr;
}
// Returns the tab parent which has this composition in its remote process.
TabParent* GetTabParent() const
{
return mTabParent;
}
// Returns true if the composition is started with synthesized event which
// came from nsDOMWindowUtils.
bool IsSynthesizedForTests() const { return mIsSynthesizedForTests; }
Expand Down
Loading

0 comments on commit 3caf001

Please sign in to comment.