Bug 1662265 - Fix input events handling for sync XHR when both TaskController and e10s are enabled r=smaug

There are two issues in our current setup

1) Input events which are occurring in the same tab are going to be lost
because sync XHR. We have event handling suppression for synx XHR, so input
events are going to be discarded.

2) Input events that are happening in another tab (same process as the
synx XHR tab) are not going to be delayed. This is not correct since
sync XHR should block the Javascript execution.

This patches fixes the above cases for when both TaskController and e10s are
enabled by suspending the InputTaskManager during sync XHR, which
delays the input event handling and keeps the events around.

Differential Revision: https://phabricator.services.mozilla.com/D90780
This commit is contained in:
Sean Feng 2020-12-03 03:13:04 +00:00
parent e271c21bcc
commit f521450d39
23 changed files with 367 additions and 11 deletions

View File

@ -7,6 +7,7 @@
#include "mozilla/dom/BrowsingContextGroup.h"
#include "mozilla/ClearOnShutdown.h"
#include "mozilla/InputTaskManager.h"
#include "mozilla/dom/BrowsingContextBinding.h"
#include "mozilla/dom/BindingUtils.h"
#include "mozilla/dom/ContentChild.h"
@ -319,6 +320,36 @@ void BrowsingContextGroup::FlushPostMessageEvents() {
}
}
void BrowsingContextGroup::IncInputEventSuspensionLevel() {
MOZ_ASSERT(StaticPrefs::dom_input_events_canSuspendInBCG_enabled_AtStartup());
if (!mInputEventSuspensionLevel && !InputTaskManager::Get()->IsSuspended()) {
InputTaskManager::Get()->SetIsSuspended(true);
}
++mInputEventSuspensionLevel;
}
void BrowsingContextGroup::DecInputEventSuspensionLevel() {
MOZ_ASSERT(StaticPrefs::dom_input_events_canSuspendInBCG_enabled_AtStartup());
--mInputEventSuspensionLevel;
if (!mInputEventSuspensionLevel && InputTaskManager::Get()->IsSuspended()) {
InputTaskManager::Get()->SetIsSuspended(false);
}
}
void BrowsingContextGroup::UpdateInputTaskManagerIfNeeded() {
MOZ_ASSERT(StaticPrefs::dom_input_events_canSuspendInBCG_enabled_AtStartup());
InputTaskManager* inputManager = InputTaskManager::Get();
if (mInputEventSuspensionLevel && !inputManager->IsSuspended()) {
inputManager->SetIsSuspended(true);
return;
}
if (!mInputEventSuspensionLevel && inputManager->IsSuspended()) {
inputManager->SetIsSuspended(false);
}
}
/* static */
BrowsingContextGroup* BrowsingContextGroup::GetChromeGroup() {
MOZ_DIAGNOSTIC_ASSERT(XRE_IsParentProcess());

View File

@ -126,6 +126,11 @@ class BrowsingContextGroup final : public nsWrapperCache {
void FlushPostMessageEvents();
// Update the status of the InputTaskManager based on
// 1.Its suspension status
// 2.Which BCG sets the suspension status.
void UpdateInputTaskManagerIfNeeded();
static BrowsingContextGroup* GetChromeGroup();
void GetDocGroups(nsTArray<DocGroup*>& aDocGroups);
@ -149,6 +154,9 @@ class BrowsingContextGroup final : public nsWrapperCache {
static void GetAllGroups(nsTArray<RefPtr<BrowsingContextGroup>>& aGroups);
void IncInputEventSuspensionLevel();
void DecInputEventSuspensionLevel();
private:
friend class CanonicalBrowsingContext;
@ -206,8 +214,14 @@ class BrowsingContextGroup final : public nsWrapperCache {
RefPtr<mozilla::ThrottledEventQueue> mTimerEventQueue;
RefPtr<mozilla::ThrottledEventQueue> mWorkerEventQueue;
};
// A counter to keep track of the input event suspension level of this BCG
//
// We use BrowsingContextGroup to emulate process isolation in Fission, so
// documents within the same the same BCG will behave like they share
// the same input task queue.
uint32_t mInputEventSuspensionLevel = 0;
};
} // namespace dom
} // namespace mozilla

View File

@ -4836,6 +4836,18 @@ nsDocShell::SetIsActive(bool aIsActive) {
}
}
// When switching between tabs, there are always docShells that become
// active in the same process if they are same process tabs, so we just
// let the docShells that are becoming active to update the
// InputTaskManager if needed, because it's going to be duplicate works
// to ask both active and inactive docShells to do it.
//
// If the tabs are in different processes, we still don't need to call
// UpdateInputTaskManagerIfNeeded because the it's okay to keep
// the input events suspended for background tabs.
if (aIsActive && InputTaskManager::CanSuspendInputEvent()) {
mBrowsingContext->Group()->UpdateInputTaskManagerIfNeeded();
}
return NS_OK;
}

View File

@ -70,6 +70,7 @@
#include "mozilla/HTMLEditor.h"
#include "mozilla/HoldDropJSObjects.h"
#include "mozilla/IdentifierMapEntry.h"
#include "mozilla/InputTaskManager.h"
#include "mozilla/IntegerRange.h"
#include "mozilla/InternalMutationEvent.h"
#include "mozilla/Likely.h"
@ -15603,7 +15604,9 @@ static CallState MarkDocumentTreeToBeInSyncOperation(
return CallState::Continue;
}
nsAutoSyncOperation::nsAutoSyncOperation(Document* aDoc) {
nsAutoSyncOperation::nsAutoSyncOperation(Document* aDoc,
SyncOperationBehavior aSyncBehavior)
: mSyncBehavior(aSyncBehavior) {
mMicroTaskLevel = 0;
CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get();
if (ccjs) {
@ -15618,6 +15621,13 @@ nsAutoSyncOperation::nsAutoSyncOperation(Document* aDoc) {
}
}
}
mBrowsingContext = aDoc->GetBrowsingContext();
if (mBrowsingContext &&
mSyncBehavior == SyncOperationBehavior::eSuspendInput &&
InputTaskManager::CanSuspendInputEvent()) {
mBrowsingContext->Group()->IncInputEventSuspensionLevel();
}
}
}
@ -15632,6 +15642,12 @@ nsAutoSyncOperation::~nsAutoSyncOperation() {
if (ccjs) {
ccjs->SetMicroTaskLevel(mMicroTaskLevel);
}
if (mBrowsingContext &&
mSyncBehavior == SyncOperationBehavior::eSuspendInput &&
InputTaskManager::CanSuspendInputEvent()) {
mBrowsingContext->Group()->DecInputEventSuspensionLevel();
}
}
gfxUserFontSet* Document::GetUserFontSet() {

View File

@ -5243,14 +5243,19 @@ class MOZ_STACK_CLASS mozAutoSubtreeModified {
RefPtr<Document> mSubtreeOwner;
};
enum class SyncOperationBehavior { eSuspendInput, eAllowInput };
class MOZ_STACK_CLASS nsAutoSyncOperation {
public:
explicit nsAutoSyncOperation(Document* aDocument);
explicit nsAutoSyncOperation(Document* aDocument,
SyncOperationBehavior aSyncBehavior);
~nsAutoSyncOperation();
private:
nsTArray<RefPtr<Document>> mDocuments;
uint32_t mMicroTaskLevel;
const SyncOperationBehavior mSyncBehavior;
RefPtr<BrowsingContext> mBrowsingContext;
};
class MOZ_RAII AutoSetThrowOnDynamicMarkupInsertionCounter final {

View File

@ -28,6 +28,7 @@
#include "mozilla/PendingAnimationTracker.h"
#include "mozilla/ServoStyleSet.h"
#include "mozilla/SharedStyleSheetCache.h"
#include "mozilla/InputTaskManager.h"
#include "nsIObjectLoadingContent.h"
#include "nsIFrame.h"
#include "mozilla/layers/APZCCallbackHelper.h"
@ -1420,6 +1421,12 @@ nsDOMWindowUtils::GetIsMozAfterPaintPending(bool* aResult) {
return NS_OK;
}
NS_IMETHODIMP
nsDOMWindowUtils::GetIsInputTaskManagerSuspended(bool* aResult) {
*aResult = InputTaskManager::Get()->IsSuspended();
return NS_OK;
}
NS_IMETHODIMP
nsDOMWindowUtils::DisableNonTestMouseEvents(bool aDisable) {
nsCOMPtr<nsPIDOMWindowOuter> window = do_QueryReferent(mWindow);

View File

@ -4943,7 +4943,7 @@ bool nsGlobalWindowOuter::AlertOrConfirm(bool aAlert, const nsAString& aMessage,
}
bool result = false;
nsAutoSyncOperation sync(mDoc);
nsAutoSyncOperation sync(mDoc, SyncOperationBehavior::eSuspendInput);
if (ShouldPromptToBlockDialogs()) {
bool disallowDialog = false;
nsAutoString label;
@ -5043,7 +5043,7 @@ void nsGlobalWindowOuter::PromptOuter(const nsAString& aMessage,
nsContentUtils::eCOMMON_DIALOG_PROPERTIES, "ScriptDialogLabel", label);
}
nsAutoSyncOperation sync(mDoc);
nsAutoSyncOperation sync(mDoc, SyncOperationBehavior::eSuspendInput);
bool ok;
aError = prompt->Prompt(title.get(), fixedMessage.get(), &inoutValue,
label.IsVoid() ? nullptr : label.get(),
@ -5287,7 +5287,7 @@ Nullable<WindowProxyHolder> nsGlobalWindowOuter::Print(
return nullptr;
}
nsAutoSyncOperation sync(docToPrint);
nsAutoSyncOperation sync(docToPrint, SyncOperationBehavior::eAllowInput);
EnterModalState();
auto exitModal = MakeScopeExit([&] { LeaveModalState(); });

View File

@ -821,6 +821,11 @@ interface nsIDOMWindowUtils : nsISupports {
*/
readonly attribute boolean isMozAfterPaintPending;
/**
* Returns true if the InputTaskManager is suspended.
*/
readonly attribute boolean isInputTaskManagerSuspended;
/**
* Suppresses/unsuppresses user initiated event handling in window's document
* and subdocuments.

View File

@ -3032,7 +3032,8 @@ nsresult XMLHttpRequestMainThread::SendInternal(const BodyExtractorBase* aBody,
}
if (NS_SUCCEEDED(rv)) {
nsAutoSyncOperation sync(mSuspendedDoc);
nsAutoSyncOperation sync(mSuspendedDoc,
SyncOperationBehavior::eSuspendInput);
if (!SpinEventLoopUntil([&]() { return !mFlagSyncLooping; })) {
rv = NS_ERROR_UNEXPECTED;
}

View File

@ -0,0 +1,40 @@
<!DOCTYPE HTML>
<html>
<body>
<input>
<script>
const input = document.querySelector("input");
var count = 0;
input.addEventListener("keydown", function() {
++count;
});
input.addEventListener("keyup", function() {
++count;
if (count == 6) {
window.opener.receivedAllEvents = true;
window.close();
}
});
input.focus();
window.opener.startSlowXHR();
function triggerKeys() {
// Use loadChromeScript to run the script in the parent process
// so that we can dispatch key event in the parent to test
// InputTaskManager properly
SpecialPowers.loadChromeScript(() => {
const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
const { EventUtils } = ChromeUtils.import(
"resource://specialpowers/SpecialPowersEventUtils.jsm"
);
var win = Services.wm.getMostRecentBrowserWindow();
EventUtils.synthesizeKey("a", {}, win);
EventUtils.synthesizeKey("b", {}, win);
EventUtils.synthesizeKey("c", {}, win);
});
}
</script>
</body>
</html>

View File

@ -0,0 +1,14 @@
<!DOCTYPE HTML>
<html>
<body>
<script>
// Use setTimeout here to let opener's sync XHR to run
setTimeout(() => {
var utils = SpecialPowers.getDOMWindowUtils(window);
const channel = new BroadcastChannel('taskManagerStatus');
channel.postMessage(utils.isInputTaskManagerSuspended);
window.close();
}, 0);
</script>
</body>
</html>

View File

@ -0,0 +1,33 @@
<!DOCTYPE HTML>
<html>
<body>
<input />
<script>
const input = document.querySelector("input");
input.addEventListener("keydown", function() {
window.opener.receivedInput();
window.close();
});
function startSlowXHR() {
setTimeout(function() {
input.focus();
SpecialPowers.loadChromeScript(() => {
const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
const { EventUtils } = ChromeUtils.import(
"resource://specialpowers/SpecialPowersEventUtils.jsm"
);
var win = Services.wm.getMostRecentBrowserWindow();
EventUtils.synthesizeKey("a", {}, win);
});
var xhr = new XMLHttpRequest();
xhr.open("GET", "slow.sjs", false);
xhr.send(null);
window.opener.childXHRFinished = true;
}, 0);
}
</script>
</body>
</html>

View File

@ -108,6 +108,18 @@ support-files = test_XHR_timeout.js
[test_XHRSendData.html]
[test_sync_xhr_document_write_with_iframe.html]
skip-if = toolkit == "android" && debug && !is_fennec
[test_sync_xhr_event_handling.html]
support-files =
file_sync_xhr_event_handling_helper.html
fail-if = !e10s || release_or_beta
[test_sync_xhr_nested.html]
support-files =
file_sync_xhr_nested_helper.html
skip-if = !e10s || release_or_beta # Input event will be discarded during sync XHR, thus timeout
[test_sync_xhr_event_handling_switch_bcg.html]
support-files =
file_sync_xhr_event_handling_switch_bcg_helper.html
fail-if = !e10s || release_or_beta
[test_nestedSyncXHR.html]
[test_event_listener_leaks.html]
skip-if = (os == "win" && processor == "aarch64") #bug 1535784

View File

@ -0,0 +1,36 @@
<!DOCTYPE HTML>
<!-- vim: set shiftwidth=2 tabstop=2 autoindent cindent expandtab: -->
<html>
<head>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<script src="/tests/SimpleTest/EventUtils.js"></script>
<link rel="stylesheet" href="/tests/SimpleTest/test.css">
</head>
<body >
<script type="text/javascript">
SimpleTest.waitForExplicitFinish();
var receivedAllEvents = false;
var subTab = null;
function startSlowXHR() {
setTimeout(() => {
var xhr = new XMLHttpRequest();
xhr.open("GET", "slow.sjs", false);
subTab.triggerKeys();
xhr.send(null);
ok(!receivedAllEvents, "Input Event should be blocked during sync XHR");
window.requestIdleCallback(() => {
ok(receivedAllEvents, "Input Event should be processed after synx XHR");
SimpleTest.finish();
});
}, 0);
}
function runTest() {
subTab = window.open("file_sync_xhr_event_handling_helper.html");
}
runTest();
</script>
</body>
</html>

View File

@ -0,0 +1,36 @@
<!DOCTYPE HTML>
<!-- vim: set shiftwidth=2 tabstop=2 autoindent cindent expandtab: -->
<html>
<head>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<script src="/tests/SimpleTest/EventUtils.js"></script>
<link rel="stylesheet" href="/tests/SimpleTest/test.css">
</head>
<body >
<script type="text/javascript">
SimpleTest.waitForExplicitFinish();
let receivedMessage = [];
const channel = new BroadcastChannel('taskManagerStatus');
channel.onmessage = function(message) {
receivedMessage.push(message.data);
}
let hasTestedSameBCG = false;
function startSlowXHR(aWindowFeature) {
var xhr = new XMLHttpRequest();
window.open("file_sync_xhr_event_handling_switch_bcg_helper.html", "", aWindowFeature);
xhr.open("GET", "slow.sjs", false);
xhr.send(null);
if (hasTestedSameBCG) {
SimpleTest.isDeeply(receivedMessage, [true, false], "The first message should be true because the helper document was opened in the same BCG, and the second message should be false because it was opened in a different BCG");
SimpleTest.finish();
} else {
hasTestedSameBCG = true;
startSlowXHR("noopener");
}
}
startSlowXHR("");
</script>
</body>
</html>

View File

@ -0,0 +1,44 @@
<!DOCTYPE HTML>
<!-- vim: set shiftwidth=2 tabstop=2 autoindent cindent expandtab: -->
<html>
<head>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<script src="/tests/SimpleTest/EventUtils.js"></script>
<link rel="stylesheet" href="/tests/SimpleTest/test.css">
</head>
<body >
<script type="text/javascript">
SimpleTest.waitForExplicitFinish();
var childXHRFinished = false;
var xhrFinished = false;
var subTab = null;
function receivedInput() {
ok(xhrFinished, "Input event should be handled after the sync xhr");
SimpleTest.finish();
}
function startSlowXHR() {
var xhr = new XMLHttpRequest();
xhr.open("GET", "slow.sjs", false);
subTab.startSlowXHR();
xhr.send(null);
// Above xhr.send(null) should spin up an event loop to process the inner XHR first
ok(childXHRFinished, "Child's XHR should be finished first");
xhrFinished = true;
}
async function runTest() {
subTab = window.open("file_sync_xhr_nested_helper.html");
await new Promise((r) => {
subTab.addEventListener("load", r);
});
startSlowXHR();
}
runTest();
</script>
</body>
</html>

View File

@ -33,7 +33,8 @@ nsresult txParseDocumentFromURI(const nsAString& aHref,
// Raw pointer, we want the resulting txXPathNode to hold a reference to
// the document.
Document* theDocument = nullptr;
nsAutoSyncOperation sync(loaderDocument);
nsAutoSyncOperation sync(loaderDocument,
SyncOperationBehavior::eSuspendInput);
rv = nsSyncLoadService::LoadDocument(
documentURI, nsIContentPolicy::TYPE_INTERNAL_XMLHTTPREQUEST,
loaderDocument->NodePrincipal(),

View File

@ -557,7 +557,8 @@ nsresult txSyncCompileObserver::loadURI(const nsAString& aUri,
if (mProcessor) {
source = mProcessor->GetSourceContentModel();
}
dom::nsAutoSyncOperation sync(source ? source->OwnerDoc() : nullptr);
dom::nsAutoSyncOperation sync(source ? source->OwnerDoc() : nullptr,
dom::SyncOperationBehavior::eSuspendInput);
nsCOMPtr<Document> document;
rv = nsSyncLoadService::LoadDocument(

View File

@ -200,6 +200,7 @@
#include "mozilla/ServoStyleSet.h"
#include "mozilla/StyleSheet.h"
#include "mozilla/StyleSheetInlines.h"
#include "mozilla/InputTaskManager.h"
#include "mozilla/dom/ImageTracker.h"
#include "nsIDocShellTreeOwner.h"
#include "nsClassHashtable.h"
@ -7590,6 +7591,10 @@ bool PresShell::EventHandler::MaybeDiscardOrDelayKeyboardEvent(
return false;
}
MOZ_ASSERT_IF(InputTaskManager::CanSuspendInputEvent() &&
aGUIEvent->mMessage != eMouseMove,
!InputTaskManager::Get()->IsSuspended());
if (aGUIEvent->mMessage == eKeyDown) {
mPresShell->mNoDelayedKeyEvents = true;
} else if (!mPresShell->mNoDelayedKeyEvents) {
@ -7616,6 +7621,10 @@ bool PresShell::EventHandler::MaybeDiscardOrDelayMouseEvent(
return false;
}
MOZ_ASSERT_IF(InputTaskManager::CanSuspendInputEvent() &&
aGUIEvent->mMessage != eMouseMove,
!InputTaskManager::Get()->IsSuspended());
if (aGUIEvent->mMessage == eMouseDown) {
mPresShell->mNoDelayedMouseEvents = true;
} else if (!mPresShell->mNoDelayedMouseEvents &&

View File

@ -1261,7 +1261,7 @@ nsDocumentViewer::PermitUnload(PermitUnloadAction aAction,
return NS_OK;
}
nsAutoSyncOperation sync(mDocument);
nsAutoSyncOperation sync(mDocument, SyncOperationBehavior::eSuspendInput);
AutoSuppressEventHandlingAndSuspend seh(bc->Group());
mInPermitUnloadPrompt = true;

View File

@ -1951,6 +1951,12 @@
value: true
mirror: always
# Whether we allow BrowsingContextGroup to suspend input events
- name: dom.input_events.canSuspendInBCG.enabled
type: bool
value: @IS_NIGHTLY_BUILD@
mirror: once
# Enable not moving the cursor to end when a text input or textarea has .value
# set to the value it already has. By default, enabled.
- name: dom.input.skip_cursor_move_for_same_value_set

View File

@ -172,7 +172,7 @@ class IdlePeriodState {
// If we're in a content process, we use mIdleScheduler to communicate with
// the parent process for purposes of cross-process idle tracking.
RefPtr<ipc::IdleSchedulerChild> mIdleScheduler;
RefPtr<mozilla::ipc::IdleSchedulerChild> mIdleScheduler;
// Our cached idle deadline. This is set by UpdateCachedIdleDeadline() and
// cleared by ClearCachedIdleDeadline(). Consumers should do the former while

View File

@ -9,6 +9,7 @@
#include "TaskController.h"
#include "mozilla/StaticPtr.h"
#include "mozilla/StaticPrefs_dom.h"
namespace mozilla {
@ -45,6 +46,34 @@ class InputTaskManager : public TaskManager {
static void Cleanup() { gInputTaskManager = nullptr; }
static void Init();
bool IsSuspended(const MutexAutoLock& aProofOfLock) override {
MOZ_ASSERT(NS_IsMainThread());
return mIsSuspended;
}
bool IsSuspended() {
MOZ_ASSERT(NS_IsMainThread());
return mIsSuspended;
}
void SetIsSuspended(bool aIsSuspended) {
MOZ_ASSERT(NS_IsMainThread());
mIsSuspended = aIsSuspended;
}
static bool CanSuspendInputEvent() {
// Ensure it's content process because InputTaskManager only
// works in e10s.
//
// Input tasks will have nullptr as their task manager when the
// event queue state is STATE_DISABLED, so we can't suspend
// input events.
return XRE_IsContentProcess() &&
StaticPrefs::dom_input_events_canSuspendInBCG_enabled_AtStartup() &&
InputTaskManager::Get()->State() !=
InputEventQueueState::STATE_DISABLED;
}
private:
InputTaskManager() : mInputQueueState(STATE_DISABLED) {}
@ -53,6 +82,10 @@ class InputTaskManager : public TaskManager {
AutoTArray<TimeStamp, 4> mStartTimes;
static StaticRefPtr<InputTaskManager> gInputTaskManager;
// Unlike mInputQueueState, mIsSuspended is used by TaskController to
// indicate its status
bool mIsSuspended = false;
};
} // namespace mozilla