Bug 1333459 - part2-2: EventStateManager should check if it needs to wait reply from remote content before handling access keys r=smaug

Currently, access key is handled in EventStateManager::PreHandleEvent() with eKeyPress event, i.e., before dispatching it into the DOM tree, if the access key is registered in EventStateManager.  So, the main process does not check if the preceding eKeyDown event is consumed in focused remote process.

When preceding eKeyDown event is consumed in the main process, eKeyPress event won't be dispatched by widget.  However, if remote process has focus, it's impossible widget to stop dispatching eKeyPress event because preceding eKeyDown event hasn't been handled in the focused remote process yet.  Therefore, main process needs to post eKeyPress event to check if preceding eKeyDown event was consumed.  When eKeyPress event is marked as "waiting reply from remote process", TabChild sends it back to the main process only when preceding eKeyDown event wasn't consumed.  So, only when eKeyPress event is back to the main process, main process should handle accesskey with it.

This patch makes EventStateManager::PreHandleEvent() check if a remote target has focus before handling accesskey.  If a remote process has accesskey and there is an accesskey matching with eKeyPress event, it marks the event as "waiting reply from remote content" and stop propagation in the process.

Finally, when eKeyPress event is sent back to TabParent, TabParent::RecvReplyKeyEvent() calls EventStateManager::HandleAccessKey() before dispatching the reply event into the DOM tree.

MozReview-Commit-ID: KsOkakaIVzb

--HG--
extra : rebase_source : 7e0c6966a1bde085e34d45bca4b0166b9fc2f3f1
This commit is contained in:
Masayuki Nakano 2017-07-22 10:50:41 +09:00
Родитель 908b7f2e51
Коммит 44d5a33919
9 изменённых файлов: 142 добавлений и 44 удалений

Просмотреть файл

@ -763,11 +763,34 @@ EventStateManager::PreHandleEvent(nsPresContext* aPresContext,
WidgetKeyboardEvent* keyEvent = aEvent->AsKeyboardEvent();
if (keyEvent->ModifiersMatchWithAccessKey(AccessKeyType::eChrome) ||
keyEvent->ModifiersMatchWithAccessKey(AccessKeyType::eContent)) {
AutoTArray<uint32_t, 10> accessCharCodes;
keyEvent->GetAccessKeyCandidates(accessCharCodes);
// If the eKeyPress event will be sent to a remote process, this
// process needs to wait reply from the remote process for checking if
// preceding eKeyDown event is consumed. If preceding eKeyDown event
// is consumed in the remote process, TabChild won't send the event
// back to this process. So, only when this process receives a reply
// eKeyPress event in TabParent, we should handle accesskey in this
// process.
if (IsRemoteTarget(GetFocusedContent())) {
// However, if there is no accesskey target for the key combination,
// we don't need to wait reply from the remote process. Otherwise,
// Mark the event as waiting reply from remote process and stop
// propagation in this process.
if (CheckIfEventMatchesAccessKey(keyEvent, aPresContext)) {
keyEvent->StopPropagation();
keyEvent->MarkAsWaitingReplyFromRemoteProcess();
}
}
// If the event target is in this process, we can handle accesskey now
// since if preceding eKeyDown event was consumed, eKeyPress event
// won't be dispatched by widget. So, coming eKeyPress event means
// that the preceding eKeyDown event wasn't consumed in this case.
else {
AutoTArray<uint32_t, 10> accessCharCodes;
keyEvent->GetAccessKeyCandidates(accessCharCodes);
if (HandleAccessKey(keyEvent, aPresContext, accessCharCodes)) {
*aStatus = nsEventStatus_eConsumeNoDefault;
if (HandleAccessKey(keyEvent, aPresContext, accessCharCodes)) {
*aStatus = nsEventStatus_eConsumeNoDefault;
}
}
}
}
@ -1102,7 +1125,13 @@ HandleAccessKeyInRemoteChild(TabParent* aTabParent, void* aArg)
bool active;
aTabParent->GetDocShellIsActive(&active);
if (active) {
accessKeyInfo->event->mAccessKeyForwardedToChild = true;
// Even if there is no target for the accesskey in this process,
// the event may match with a content accesskey. If so, the keyboard
// event should be handled with reply event for preventing double action.
// (e.g., Alt+Shift+F on Windows may focus a content in remote and open
// "File" menu.)
accessKeyInfo->event->StopPropagation();
accessKeyInfo->event->MarkAsWaitingReplyFromRemoteProcess();
aTabParent->HandleAccessKey(*accessKeyInfo->event,
accessKeyInfo->charCodes);
return true;
@ -1202,16 +1231,22 @@ EventStateManager::WalkESMTreeToHandleAccessKey(
if (aExecute &&
aEvent->ModifiersMatchWithAccessKey(AccessKeyType::eContent) &&
mDocument && mDocument->GetWindow()) {
// If the focus is currently on a node with a TabParent, the key event will
// get forwarded to the child process and HandleAccessKey called from there.
// If the focus is currently on a node with a TabParent, the key event
// should've gotten forwarded to the child process and HandleAccessKey
// called from there.
if (TabParent::GetFrom(GetFocusedContent())) {
// If access key may be only in remote contents, this method won't handle
// access key synchronously. In this case, only reply event should reach
// here.
MOZ_ASSERT(aEvent->IsHandledInRemoteProcess() ||
!aEvent->IsWaitingReplyFromRemoteProcess());
}
// If focus is somewhere else, then we need to check the remote children.
nsFocusManager* fm = nsFocusManager::GetFocusManager();
nsIContent* focusedContent = fm ? fm->GetFocusedContent() : nullptr;
if (TabParent::GetFrom(focusedContent)) {
// A remote child process is focused. The key event should get sent to
// the child process.
aEvent->mAccessKeyForwardedToChild = true;
} else {
// However, if the event has already been handled in a remote process,
// then, focus is moved from the remote process after posting the event.
// In such case, we shouldn't retry to handle access keys in remote
// processes.
else if (!aEvent->IsHandledInRemoteProcess()) {
AccessKeyInfo accessKeyInfo(aEvent, aAccessCharCodes);
nsContentUtils::CallOnAllRemoteChildren(mDocument->GetWindow(),
HandleAccessKeyInRemoteChild,

Просмотреть файл

@ -1599,7 +1599,7 @@ TabChild::RecvRealMouseButtonEvent(const WidgetMouseEvent& aEvent,
localEvent.mWidget = mPuppetWidget;
APZCCallbackHelper::ApplyCallbackTransform(localEvent, aGuid,
mPuppetWidget->GetDefaultScale());
APZCCallbackHelper::DispatchWidgetEvent(localEvent);
DispatchWidgetEventViaAPZ(localEvent);
if (aInputBlockId && aEvent.mFlags.mHandledByAPZ) {
mAPZEventState->ProcessMouseEvent(aEvent, aGuid, aInputBlockId);
@ -1648,6 +1648,13 @@ TabChild::MaybeCoalesceWheelEvent(const WidgetWheelEvent& aEvent,
return false;
}
nsEventStatus
TabChild::DispatchWidgetEventViaAPZ(WidgetGUIEvent& aEvent)
{
aEvent.ResetWaitingReplyFromRemoteProcessState();
return APZCCallbackHelper::DispatchWidgetEvent(aEvent);
}
void
TabChild::MaybeDispatchCoalescedWheelEvent()
{
@ -1678,7 +1685,7 @@ TabChild::DispatchWheelEvent(const WidgetWheelEvent& aEvent,
localEvent.mWidget = mPuppetWidget;
APZCCallbackHelper::ApplyCallbackTransform(localEvent, aGuid,
mPuppetWidget->GetDefaultScale());
APZCCallbackHelper::DispatchWidgetEvent(localEvent);
DispatchWidgetEventViaAPZ(localEvent);
if (localEvent.mCanTriggerSwipe) {
SendRespondStartSwipeEvent(aInputBlockId, localEvent.TriggersSwipe());
@ -1745,7 +1752,7 @@ TabChild::RecvRealTouchEvent(const WidgetTouchEvent& aEvent,
}
// Dispatch event to content (potentially a long-running operation)
nsEventStatus status = APZCCallbackHelper::DispatchWidgetEvent(localEvent);
nsEventStatus status = DispatchWidgetEventViaAPZ(localEvent);
if (!AsyncPanZoomEnabled()) {
// We shouldn't have any e10s platforms that have touch events enabled
@ -1805,7 +1812,7 @@ TabChild::RecvRealDragEvent(const WidgetDragEvent& aEvent,
}
}
APZCCallbackHelper::DispatchWidgetEvent(localEvent);
DispatchWidgetEventViaAPZ(localEvent);
return IPC_OK();
}
@ -1814,7 +1821,7 @@ TabChild::RecvPluginEvent(const WidgetPluginEvent& aEvent)
{
WidgetPluginEvent localEvent(aEvent);
localEvent.mWidget = mPuppetWidget;
nsEventStatus status = APZCCallbackHelper::DispatchWidgetEvent(localEvent);
nsEventStatus status = DispatchWidgetEventViaAPZ(localEvent);
if (status != nsEventStatus_eConsumeNoDefault) {
// If not consumed, we should call default action
SendDefaultProcOfPluginEvent(aEvent);
@ -1916,7 +1923,7 @@ TabChild::RecvRealKeyEvent(const WidgetKeyboardEvent& aEvent)
WidgetKeyboardEvent localEvent(aEvent);
localEvent.mWidget = mPuppetWidget;
localEvent.mUniqueId = aEvent.mUniqueId;
nsEventStatus status = APZCCallbackHelper::DispatchWidgetEvent(localEvent);
nsEventStatus status = DispatchWidgetEventViaAPZ(localEvent);
// Update the end time of the possible repeated event so that we can skip
// some incoming events in case event handling took long time.
@ -1931,16 +1938,21 @@ TabChild::RecvRealKeyEvent(const WidgetKeyboardEvent& aEvent)
}
// If a response is desired from the content process, resend the key event.
// If mAccessKeyForwardedToChild is set, then don't resend the key event yet
// as RecvHandleAccessKey will do this.
if (localEvent.WantReplyFromContentProcess()) {
if (aEvent.WantReplyFromContentProcess()) {
// If the event's default isn't prevented but the status is no default,
// That means that the event was consumed by EventStateManager or something
// which is not a usual event handler. In such case, prevent its default
// as a default handler. For example, when an eKeyPress event matches
// with a content accesskey, and it's executed, peventDefault() of the
// event won't be called but the status is set to "no default". Then,
// the event shouldn't be handled by nsMenuBarListener in the main process.
if (!localEvent.DefaultPrevented() &&
status == nsEventStatus_eConsumeNoDefault) {
localEvent.PreventDefault();
}
SendReplyKeyEvent(localEvent);
}
if (localEvent.mAccessKeyForwardedToChild) {
SendAccessKeyNotHandled(localEvent);
}
return IPC_OK();
}
@ -1962,7 +1974,7 @@ TabChild::RecvCompositionEvent(const WidgetCompositionEvent& aEvent)
{
WidgetCompositionEvent localEvent(aEvent);
localEvent.mWidget = mPuppetWidget;
APZCCallbackHelper::DispatchWidgetEvent(localEvent);
DispatchWidgetEventViaAPZ(localEvent);
Unused << SendOnEventNeedingAckHandled(aEvent.mMessage);
return IPC_OK();
}
@ -1972,7 +1984,7 @@ TabChild::RecvSelectionEvent(const WidgetSelectionEvent& aEvent)
{
WidgetSelectionEvent localEvent(aEvent);
localEvent.mWidget = mPuppetWidget;
APZCCallbackHelper::DispatchWidgetEvent(localEvent);
DispatchWidgetEventViaAPZ(localEvent);
Unused << SendOnEventNeedingAckHandled(aEvent.mMessage);
return IPC_OK();
}

Просмотреть файл

@ -802,6 +802,11 @@ private:
void MaybeDispatchCoalescedWheelEvent();
/**
* Dispatch aEvent on aEvent.mWidget.
*/
nsEventStatus DispatchWidgetEventViaAPZ(WidgetGUIEvent& aEvent);
void DispatchWheelEvent(const WidgetWheelEvent& aEvent,
const ScrollableLayerGuid& aGuid,
const uint64_t& aInputBlockId);

Просмотреть файл

@ -2046,7 +2046,23 @@ TabParent::RecvReplyKeyEvent(const WidgetKeyboardEvent& aEvent)
AutoHandlingUserInputStatePusher userInpStatePusher(localEvent.IsTrusted(),
&localEvent, doc);
EventDispatcher::Dispatch(mFrameElement, presContext, &localEvent);
nsEventStatus status = nsEventStatus_eIgnore;
// Handle access key in this process before dispatching reply event because
// ESM handles it before dispatching the event to the DOM tree.
if (localEvent.mMessage == eKeyPress &&
(localEvent.ModifiersMatchWithAccessKey(AccessKeyType::eChrome) ||
localEvent.ModifiersMatchWithAccessKey(AccessKeyType::eContent))) {
RefPtr<EventStateManager> esm = presContext->EventStateManager();
AutoTArray<uint32_t, 10> accessCharCodes;
localEvent.GetAccessKeyCandidates(accessCharCodes);
if (esm->HandleAccessKey(&localEvent, presContext, accessCharCodes)) {
status = nsEventStatus_eConsumeNoDefault;
}
}
EventDispatcher::Dispatch(mFrameElement, presContext, &localEvent, nullptr,
&status);
if (!localEvent.DefaultPrevented() &&
!localEvent.mFlags.mIsSynthesizedForTests) {
@ -2065,10 +2081,16 @@ TabParent::RecvAccessKeyNotHandled(const WidgetKeyboardEvent& aEvent)
{
NS_ENSURE_TRUE(mFrameElement, IPC_OK());
// This is called only when this process had focus and HandleAccessKey
// message was posted to all remote process and each remote process didn't
// execute any content access keys.
// XXX If there were two or more remote processes, this may be called
// twice or more for a keyboard event, that must be a bug. But how to
// detect if received event has already been handled?
WidgetKeyboardEvent localEvent(aEvent);
localEvent.MarkAsHandledInRemoteProcess();
localEvent.mMessage = eAccessKeyNotFound;
localEvent.mAccessKeyForwardedToChild = false;
// Here we convert the WidgetEvent that we received to an nsIDOMEvent
// to be able to dispatch it to the <browser> element as the target element.

Просмотреть файл

@ -8161,7 +8161,11 @@ PresShell::HandleEventInternal(WidgetEvent* aEvent,
if (aEvent->mClass == eKeyboardEventClass) {
nsContentUtils::SetIsHandlingKeyBoardEvent(true);
}
if (aEvent->IsAllowedToDispatchDOMEvent()) {
// If EventStateManager or something wants reply from remote process,
// PresShell shouldn't dispatch the event into the DOM tree because they
// don't have a chance to stop propagation in the system event group.
if (aEvent->IsAllowedToDispatchDOMEvent() &&
!aEvent->IsWaitingReplyFromRemoteProcess()) {
MOZ_ASSERT(nsContentUtils::IsSafeToRunScript(),
"Somebody changed aEvent to cause a DOM event!");
nsPresShellEventCB eventCB(this);

Просмотреть файл

@ -271,8 +271,7 @@ nsMenuBarListener::KeyPress(nsIDOMEvent* aKeyEvent)
// the mozaccesskeynotfound event before handling accesskeys.
WidgetKeyboardEvent* nativeKeyEvent =
aKeyEvent->WidgetEventPtr()->AsKeyboardEvent();
if (!nativeKeyEvent ||
(nativeKeyEvent && nativeKeyEvent->mAccessKeyForwardedToChild)) {
if (!nativeKeyEvent) {
return NS_OK;
}
@ -301,6 +300,9 @@ nsMenuBarListener::KeyPress(nsIDOMEvent* aKeyEvent)
// so, we'll know the menu got activated.
nsMenuFrame* result = mMenuBarFrame->FindMenuWithShortcut(keyEvent);
if (result) {
// TODO: If the event target is a remote process and not stopped
// sending the event to it, we need to mark the event as
// waiting reply from remote process and stop handling the event.
mMenuBarFrame->SetActiveByKeyboard();
mMenuBarFrame->SetActive(true);
result->OpenMenu(true);

Просмотреть файл

@ -252,6 +252,20 @@ public:
mNoRemoteProcessDispatch = false;
mWantReplyFromContentProcess = true;
}
/**
* Reset "waiting reply from remote process" state. This is useful when
* you dispatch a copy of an event coming from different process.
*/
inline void ResetWaitingReplyFromRemoteProcessState()
{
if (IsWaitingReplyFromRemoteProcess()) {
// FYI: mWantReplyFromContentProcess is also used for indicating
// "handled in remote process" state. Therefore, only when
// IsWaitingReplyFromRemoteProcess() returns true, this should
// reset the flag.
mWantReplyFromContentProcess = false;
}
}
/**
* Return true if the event handler should wait reply event. I.e., if this
* returns true, any event handler should do nothing with the event.
@ -301,6 +315,12 @@ public:
{
MOZ_ASSERT(!IsCrossProcessForwardingStopped());
mPostedToRemoteProcess = false;
// Ignore propagation state in the different process if it's marked as
// "waiting reply from remote process" because the process needs to
// stop propagation in the process until receiving a reply event.
if (IsWaitingReplyFromRemoteProcess()) {
mPropagationStopped = mImmediatePropagationStopped = false;
}
}
/**
* Return true if the event has been posted to a remote process.
@ -629,6 +649,14 @@ public:
{
mFlags.MarkAsWaitingReplyFromRemoteProcess();
}
/**
* Reset "waiting reply from remote process" state. This is useful when
* you dispatch a copy of an event coming from different process.
*/
inline void ResetWaitingReplyFromRemoteProcessState()
{
mFlags.ResetWaitingReplyFromRemoteProcessState();
}
/**
* Return true if the event handler should wait reply event. I.e., if this
* returns true, any event handler should do nothing with the event.

Просмотреть файл

@ -154,7 +154,6 @@ protected:
, mCharCode(0)
, mPseudoCharCode(0)
, mLocation(eKeyLocationStandard)
, mAccessKeyForwardedToChild(false)
, mUniqueId(0)
#ifdef XP_MACOSX
, mNativeModifierFlags(0)
@ -183,7 +182,6 @@ public:
, mCharCode(0)
, mPseudoCharCode(0)
, mLocation(eKeyLocationStandard)
, mAccessKeyForwardedToChild(false)
, mUniqueId(0)
#ifdef XP_MACOSX
, mNativeModifierFlags(0)
@ -279,11 +277,6 @@ public:
uint32_t mPseudoCharCode;
// One of eKeyLocation*
uint32_t mLocation;
// True if accesskey handling was forwarded to the child via
// TabParent::HandleAccessKey. In this case, parent process menu access key
// handling should be delayed until it is determined that there exists no
// overriding access key in the content process.
bool mAccessKeyForwardedToChild;
// Unique id associated with a keydown / keypress event. It's ok if this wraps
// over long periods.
uint32_t mUniqueId;
@ -509,7 +502,6 @@ public:
mAlternativeCharCodes = aEvent.mAlternativeCharCodes;
mIsRepeat = aEvent.mIsRepeat;
mIsComposing = aEvent.mIsComposing;
mAccessKeyForwardedToChild = aEvent.mAccessKeyForwardedToChild;
mKeyNameIndex = aEvent.mKeyNameIndex;
mCodeNameIndex = aEvent.mCodeNameIndex;
mKeyValue = aEvent.mKeyValue;

Просмотреть файл

@ -449,7 +449,6 @@ struct ParamTraits<mozilla::WidgetKeyboardEvent>
WriteParam(aMsg, aParam.mPseudoCharCode);
WriteParam(aMsg, aParam.mAlternativeCharCodes);
WriteParam(aMsg, aParam.mIsRepeat);
WriteParam(aMsg, aParam.mAccessKeyForwardedToChild);
WriteParam(aMsg, aParam.mLocation);
WriteParam(aMsg, aParam.mUniqueId);
WriteParam(aMsg, aParam.mIsSynthesizedByTIP);
@ -487,7 +486,6 @@ struct ParamTraits<mozilla::WidgetKeyboardEvent>
ReadParam(aMsg, aIter, &aResult->mPseudoCharCode) &&
ReadParam(aMsg, aIter, &aResult->mAlternativeCharCodes) &&
ReadParam(aMsg, aIter, &aResult->mIsRepeat) &&
ReadParam(aMsg, aIter, &aResult->mAccessKeyForwardedToChild) &&
ReadParam(aMsg, aIter, &aResult->mLocation) &&
ReadParam(aMsg, aIter, &aResult->mUniqueId) &&
ReadParam(aMsg, aIter, &aResult->mIsSynthesizedByTIP) &&