[tor-commits] [Git][tpo/applications/tor-browser][tor-browser-115.1.0esr-13.0-1] 4 commits: Bug 1821884 - Ensure consistent state for fullscreen/pointerlock warnings; r=Gijs
ma1 (@ma1)
git at gitlab.torproject.org
Tue Aug 1 09:47:51 UTC 2023
ma1 pushed to branch tor-browser-115.1.0esr-13.0-1 at The Tor Project / Applications / Tor Browser
Commits:
a9279174 by Edgar Chen at 2023-07-31T23:49:06+02:00
Bug 1821884 - Ensure consistent state for fullscreen/pointerlock warnings; r=Gijs
Fullscreen/PointerLock warnings are initialized with hidden="true", but
change to hidden="" after being shown and hidden again. I think this
started happening when we began using HTML elements instead of XUL as
they handle hidden attribute differently.
Differential Revision: https://phabricator.services.mozilla.com/D177790
- - - - -
9f2eaedb by Edgar Chen at 2023-07-31T23:49:06+02:00
Bug 1821884 - Reshow initial fullscreen notification; r=Gijs
Depends on D177790
Differential Revision: https://phabricator.services.mozilla.com/D178339
- - - - -
30d19aa0 by Eitan Isaacson at 2023-07-31T23:49:07+02:00
Bug 1819160 - Map Android ids to doc/accessible id pairs. r=Jamie
Differential Revision: https://phabricator.services.mozilla.com/D179737
- - - - -
efee8978 by Jon Coppeard at 2023-07-31T23:49:07+02:00
Bug 1828024 - Require the helper thread lock in the GC helper thread count getter r=sfink
This makes us take a lock to read this state (we already lock when writing it).
Also it adds a release assert in case something goes wrong with the thread
count calculations, as a crash is preferable to the potential deadlock.
Differential Revision: https://phabricator.services.mozilla.com/D181257
- - - - -
12 changed files:
- accessible/android/SessionAccessibility.cpp
- accessible/android/SessionAccessibility.h
- accessible/ipc/DocAccessibleParent.cpp
- accessible/ipc/DocAccessibleParent.h
- accessible/ipc/moz.build
- browser/base/content/browser-fullScreenAndPointerLock.js
- browser/base/content/fullscreen-and-pointerlock.inc.xhtml
- browser/base/content/test/fullscreen/browser_fullscreen_warning.js
- dom/tests/browser/browser_pointerlock_warning.js
- js/src/gc/GC.cpp
- js/src/gc/ParallelMarking.cpp
- js/src/vm/HelperThreadState.h
Changes:
=====================================
accessible/android/SessionAccessibility.cpp
=====================================
@@ -269,12 +269,9 @@ RefPtr<SessionAccessibility> SessionAccessibility::GetInstanceFor(
return GetInstanceFor(doc->GetPresShell());
}
} else {
- DocAccessibleParent* remoteDoc = aAccessible->AsRemote()->Document();
- if (remoteDoc->mSessionAccessibility) {
- return remoteDoc->mSessionAccessibility;
- }
dom::CanonicalBrowsingContext* cbc =
- static_cast<dom::BrowserParent*>(remoteDoc->Manager())
+ static_cast<dom::BrowserParent*>(
+ aAccessible->AsRemote()->Document()->Manager())
->GetBrowsingContext()
->Top();
dom::BrowserParent* bp = cbc->GetBrowserParent();
@@ -285,10 +282,7 @@ RefPtr<SessionAccessibility> SessionAccessibility::GetInstanceFor(
if (auto element = bp->GetOwnerElement()) {
if (auto doc = element->OwnerDoc()) {
if (nsPresContext* presContext = doc->GetPresContext()) {
- RefPtr<SessionAccessibility> sessionAcc =
- GetInstanceFor(presContext->PresShell());
- remoteDoc->mSessionAccessibility = sessionAcc;
- return sessionAcc;
+ return GetInstanceFor(presContext->PresShell());
}
} else {
MOZ_ASSERT_UNREACHABLE(
@@ -684,14 +678,7 @@ void SessionAccessibility::PopulateNodeInfo(
}
Accessible* SessionAccessibility::GetAccessibleByID(int32_t aID) const {
- Accessible* accessible = mIDToAccessibleMap.Get(aID);
- if (accessible && accessible->IsLocal() &&
- accessible->AsLocal()->IsDefunct()) {
- MOZ_ASSERT_UNREACHABLE("Registered accessible is defunct!");
- return nullptr;
- }
-
- return accessible;
+ return mIDToAccessibleMap.Get(aID);
}
#ifdef DEBUG
@@ -705,6 +692,58 @@ static bool IsDetachedDoc(Accessible* aAccessible) {
}
#endif
+SessionAccessibility::IDMappingEntry::IDMappingEntry(Accessible* aAccessible)
+ : mInternalID(0) {
+ *this = aAccessible;
+}
+
+SessionAccessibility::IDMappingEntry&
+SessionAccessibility::IDMappingEntry::operator=(Accessible* aAccessible) {
+ mInternalID = aAccessible->ID();
+ MOZ_ASSERT(!(mInternalID & IS_REMOTE), "First bit is used in accessible ID!");
+ if (aAccessible->IsRemote()) {
+ mInternalID |= IS_REMOTE;
+ }
+
+ Accessible* docAcc = nsAccUtils::DocumentFor(aAccessible);
+ MOZ_ASSERT(docAcc);
+ if (docAcc) {
+ MOZ_ASSERT(docAcc->IsRemote() == aAccessible->IsRemote());
+ if (docAcc->IsRemote()) {
+ mDoc = docAcc->AsRemote()->AsDoc();
+ } else {
+ mDoc = docAcc->AsLocal();
+ }
+ }
+
+ return *this;
+}
+
+SessionAccessibility::IDMappingEntry::operator Accessible*() const {
+ if (mInternalID == 0) {
+ return static_cast<LocalAccessible*>(mDoc.get());
+ }
+
+ if (mInternalID == IS_REMOTE) {
+ return static_cast<DocAccessibleParent*>(mDoc.get());
+ }
+
+ if (mInternalID & IS_REMOTE) {
+ return static_cast<DocAccessibleParent*>(mDoc.get())
+ ->GetAccessible(mInternalID & ~IS_REMOTE);
+ }
+
+ Accessible* accessible =
+ static_cast<LocalAccessible*>(mDoc.get())
+ ->AsDoc()
+ ->GetAccessibleByUniqueID(reinterpret_cast<void*>(mInternalID));
+ // If the accessible is retrievable from the DocAccessible, it can't be
+ // defunct.
+ MOZ_ASSERT(!accessible->AsLocal()->IsDefunct());
+
+ return accessible;
+}
+
void SessionAccessibility::RegisterAccessible(Accessible* aAccessible) {
if (IPCAccessibilityActive()) {
// Don't register accessible in content process.
@@ -766,7 +805,6 @@ void SessionAccessibility::UnregisterAccessible(Accessible* aAccessible) {
}
RefPtr<SessionAccessibility> sessionAcc = GetInstanceFor(aAccessible);
- MOZ_ASSERT(sessionAcc, "Need SessionAccessibility to unregister Accessible!");
if (sessionAcc) {
Accessible* registeredAcc =
sessionAcc->mIDToAccessibleMap.Get(virtualViewID);
=====================================
accessible/android/SessionAccessibility.h
=====================================
@@ -110,10 +110,34 @@ class SessionAccessibility final
jni::NativeWeakPtr<widget::GeckoViewSupport> mWindow; // Parent only
java::SessionAccessibility::NativeProvider::GlobalRef mSessionAccessibility;
+ class IDMappingEntry {
+ public:
+ explicit IDMappingEntry(Accessible* aAccessible);
+
+ IDMappingEntry& operator=(Accessible* aAccessible);
+
+ operator Accessible*() const;
+
+ private:
+ // A strong reference to a DocAccessible or DocAccessibleParent. They don't
+ // share any useful base class except nsISupports, so we use that.
+ // When we retrieve the document from this reference we cast it to
+ // LocalAccessible in the DocAccessible case because DocAccessible has
+ // multiple inheritance paths for nsISupports.
+ RefPtr<nsISupports> mDoc;
+ // The ID of the accessible as used in the internal doc mapping.
+ // We rely on this ID being pointer derived and therefore divisible by two
+ // so we can use the first bit to mark if it is remote or not.
+ uint64_t mInternalID;
+
+ static const uintptr_t IS_REMOTE = 0x1;
+ };
+
/*
* This provides a mapping from 32 bit id to accessible objects.
*/
- nsTHashMap<nsUint32HashKey, Accessible*> mIDToAccessibleMap;
+ nsBaseHashtable<nsUint32HashKey, IDMappingEntry, Accessible*>
+ mIDToAccessibleMap;
};
} // namespace a11y
=====================================
accessible/ipc/DocAccessibleParent.cpp
=====================================
@@ -29,7 +29,6 @@
#endif
#if defined(ANDROID)
-# include "mozilla/a11y/SessionAccessibility.h"
# define ACQUIRE_ANDROID_LOCK \
MonitorAutoLock mal(nsAccessibilityService::GetAndroidMonitor());
#else
=====================================
accessible/ipc/DocAccessibleParent.h
=====================================
@@ -29,10 +29,6 @@ class xpcAccessibleGeneric;
class DocAccessiblePlatformExtParent;
#endif
-#ifdef ANDROID
-class SessionAccessibility;
-#endif
-
/*
* These objects live in the main process and comunicate with and represent
* an accessible document in a content process.
@@ -348,10 +344,6 @@ class DocAccessibleParent : public RemoteAccessible,
size_t SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) override;
-#ifdef ANDROID
- RefPtr<SessionAccessibility> mSessionAccessibility;
-#endif
-
private:
~DocAccessibleParent();
=====================================
accessible/ipc/moz.build
=====================================
@@ -24,11 +24,6 @@ else:
LOCAL_INCLUDES += [
"/accessible/mac",
]
- elif CONFIG["MOZ_WIDGET_TOOLKIT"] == "android":
- LOCAL_INCLUDES += [
- "/accessible/android",
- "/widget/android",
- ]
else:
LOCAL_INCLUDES += [
"/accessible/other",
=====================================
browser/base/content/browser-fullScreenAndPointerLock.js
=====================================
@@ -62,9 +62,14 @@ var PointerlockFsWarning = {
this._element = document.getElementById(elementId);
// Setup event listeners
this._element.addEventListener("transitionend", this);
+ this._element.addEventListener("transitioncancel", this);
window.addEventListener("mousemove", this, true);
+ window.addEventListener("activate", this);
+ window.addEventListener("deactivate", this);
// The timeout to hide the warning box after a while.
this._timeoutHide = new this.Timeout(() => {
+ window.removeEventListener("activate", this);
+ window.removeEventListener("deactivate", this);
this._state = "hidden";
}, timeout);
// The timeout to show the warning box when the pointer is at the top
@@ -116,11 +121,10 @@ var PointerlockFsWarning = {
return;
}
- // Explicitly set the last state to hidden to avoid the warning
- // box being hidden immediately because of mousemove.
- this._state = "onscreen";
- this._lastState = "hidden";
- this._timeoutHide.start();
+ if (Services.focus.activeWindow == window) {
+ this._state = "onscreen";
+ this._timeoutHide.start();
+ }
},
/**
@@ -148,7 +152,10 @@ var PointerlockFsWarning = {
this._element.hidden = true;
// Remove all event listeners
this._element.removeEventListener("transitionend", this);
+ this._element.removeEventListener("transitioncancel", this);
window.removeEventListener("mousemove", this, true);
+ window.removeEventListener("activate", this);
+ window.removeEventListener("deactivate", this);
// Clear fields
this._element = null;
this._timeoutHide = null;
@@ -186,7 +193,7 @@ var PointerlockFsWarning = {
}
if (newState != "hidden") {
if (currentState != "hidden") {
- this._element.setAttribute(newState, true);
+ this._element.setAttribute(newState, "");
} else {
// When the previous state is hidden, the display was none,
// thus no box was constructed. We need to wait for the new
@@ -197,7 +204,7 @@ var PointerlockFsWarning = {
requestAnimationFrame(() => {
requestAnimationFrame(() => {
if (this._element) {
- this._element.setAttribute(newState, true);
+ this._element.setAttribute(newState, "");
}
});
});
@@ -217,7 +224,7 @@ var PointerlockFsWarning = {
} else if (this._timeoutShow.delay >= 0) {
this._timeoutShow.start();
}
- } else {
+ } else if (state != "onscreen") {
let elemRect = this._element.getBoundingClientRect();
if (state == "hiding" && this._lastState != "hidden") {
// If we are on the hiding transition, and the pointer
@@ -239,12 +246,23 @@ var PointerlockFsWarning = {
}
break;
}
- case "transitionend": {
+ case "transitionend":
+ case "transitioncancel": {
if (this._state == "hiding") {
this._element.hidden = true;
}
break;
}
+ case "activate": {
+ this._state = "onscreen";
+ this._timeoutHide.start();
+ break;
+ }
+ case "deactivate": {
+ this._state = "hidden";
+ this._timeoutHide.cancel();
+ break;
+ }
}
},
};
=====================================
browser/base/content/fullscreen-and-pointerlock.inc.xhtml
=====================================
@@ -3,7 +3,7 @@
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
<html:div id="fullscreen-and-pointerlock-wrapper">
- <html:div id="fullscreen-warning" class="pointerlockfswarning" hidden="true">
+ <html:div id="fullscreen-warning" class="pointerlockfswarning" hidden="">
<html:div class="pointerlockfswarning-domain-text">
<html:span class="pointerlockfswarning-domain" data-l10n-name="domain"/>
</html:div>
@@ -20,7 +20,7 @@
</html:button>
</html:div>
- <html:div id="pointerlock-warning" class="pointerlockfswarning" hidden="true">
+ <html:div id="pointerlock-warning" class="pointerlockfswarning" hidden="">
<html:div class="pointerlockfswarning-domain-text">
<html:span class="pointerlockfswarning-domain" data-l10n-name="domain"/>
</html:div>
=====================================
browser/base/content/test/fullscreen/browser_fullscreen_warning.js
=====================================
@@ -3,14 +3,35 @@
"use strict";
-add_task(async function test_fullscreen_display_none() {
+function checkWarningState(aWarningElement, aExpectedState, aMsg) {
+ ["hidden", "ontop", "onscreen"].forEach(state => {
+ is(
+ aWarningElement.hasAttribute(state),
+ state == aExpectedState,
+ `${aMsg} - check ${state} attribute.`
+ );
+ });
+}
+
+async function waitForWarningState(aWarningElement, aExpectedState) {
+ await BrowserTestUtils.waitForAttribute(aExpectedState, aWarningElement, "");
+ checkWarningState(
+ aWarningElement,
+ aExpectedState,
+ `Wait for ${aExpectedState} state`
+ );
+}
+
+add_setup(async function init() {
await SpecialPowers.pushPrefEnv({
set: [
["full-screen-api.enabled", true],
["full-screen-api.allow-trusted-requests-only", false],
],
});
+});
+add_task(async function test_fullscreen_display_none() {
await BrowserTestUtils.withNewTab(
{
gBrowser,
@@ -30,11 +51,13 @@ add_task(async function test_fullscreen_display_none() {
},
async function (browser) {
let warning = document.getElementById("fullscreen-warning");
- let warningShownPromise = BrowserTestUtils.waitForAttribute(
- "onscreen",
+ checkWarningState(
warning,
- "true"
+ "hidden",
+ "Should not show full screen warning initially"
);
+
+ let warningShownPromise = waitForWarningState(warning, "onscreen");
// Enter fullscreen
await SpecialPowers.spawn(browser, [], async () => {
let frame = content.document.querySelector("iframe");
@@ -54,39 +77,33 @@ add_task(async function test_fullscreen_display_none() {
);
document.getElementById("fullscreen-exit-button").click();
await exitFullscreenPromise;
+
+ checkWarningState(
+ warning,
+ "hidden",
+ "Should hide fullscreen warning after exiting fullscreen"
+ );
}
);
});
add_task(async function test_fullscreen_pointerlock_conflict() {
- await SpecialPowers.pushPrefEnv({
- set: [
- ["full-screen-api.enabled", true],
- ["full-screen-api.allow-trusted-requests-only", false],
- ],
- });
-
await BrowserTestUtils.withNewTab("https://example.com", async browser => {
let fsWarning = document.getElementById("fullscreen-warning");
let plWarning = document.getElementById("pointerlock-warning");
- is(
- fsWarning.getAttribute("onscreen"),
- null,
- "Should not show full screen warning initially."
- );
- is(
- plWarning.getAttribute("onscreen"),
- null,
- "Should not show pointer lock warning initially."
- );
-
- let fsWarningShownPromise = BrowserTestUtils.waitForAttribute(
- "onscreen",
+ checkWarningState(
fsWarning,
- "true"
+ "hidden",
+ "Should not show full screen warning initially"
+ );
+ checkWarningState(
+ plWarning,
+ "hidden",
+ "Should not show pointer lock warning initially"
);
+ let fsWarningShownPromise = waitForWarningState(fsWarning, "onscreen");
info("Entering full screen and pointer lock.");
await SpecialPowers.spawn(browser, [], async () => {
await content.document.body.requestFullscreen();
@@ -94,15 +111,10 @@ add_task(async function test_fullscreen_pointerlock_conflict() {
});
await fsWarningShownPromise;
- is(
- fsWarning.getAttribute("onscreen"),
- "true",
- "Should show full screen warning."
- );
- is(
- plWarning.getAttribute("onscreen"),
- null,
- "Should not show pointer lock warning."
+ checkWarningState(
+ plWarning,
+ "hidden",
+ "Should not show pointer lock warning"
);
info("Exiting pointerlock");
@@ -110,18 +122,19 @@ add_task(async function test_fullscreen_pointerlock_conflict() {
await content.document.exitPointerLock();
});
- is(
- fsWarning.getAttribute("onscreen"),
- "true",
- "Should still show full screen warning."
+ checkWarningState(
+ fsWarning,
+ "onscreen",
+ "Should still show full screen warning"
);
- is(
- plWarning.getAttribute("onscreen"),
- null,
- "Should not show pointer lock warning."
+ checkWarningState(
+ plWarning,
+ "hidden",
+ "Should not show pointer lock warning"
);
// Cleanup
+ info("Exiting fullscreen");
await document.exitFullscreen();
});
});
=====================================
dom/tests/browser/browser_pointerlock_warning.js
=====================================
@@ -15,6 +15,25 @@ const FRAME_TEST_URL =
encodeURI(BODY_URL) +
'"></iframe></body>';
+function checkWarningState(aWarningElement, aExpectedState, aMsg) {
+ ["hidden", "ontop", "onscreen"].forEach(state => {
+ is(
+ aWarningElement.hasAttribute(state),
+ state == aExpectedState,
+ `${aMsg} - check ${state} attribute.`
+ );
+ });
+}
+
+async function waitForWarningState(aWarningElement, aExpectedState) {
+ await BrowserTestUtils.waitForAttribute(aExpectedState, aWarningElement, "");
+ checkWarningState(
+ aWarningElement,
+ aExpectedState,
+ `Wait for ${aExpectedState} state`
+ );
+}
+
// Make sure the pointerlock warning is shown and exited with the escape key
add_task(async function show_pointerlock_warning_escape() {
let urls = [TEST_URL, FRAME_TEST_URL];
@@ -24,11 +43,7 @@ add_task(async function show_pointerlock_warning_escape() {
let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, url);
let warning = document.getElementById("pointerlock-warning");
- let warningShownPromise = BrowserTestUtils.waitForAttribute(
- "onscreen",
- warning,
- "true"
- );
+ let warningShownPromise = waitForWarningState(warning, "onscreen");
let expectedWarningText;
@@ -49,11 +64,7 @@ add_task(async function show_pointerlock_warning_escape() {
ok(true, "Pointerlock warning shown");
- let warningHiddenPromise = BrowserTestUtils.waitForAttribute(
- "hidden",
- warning,
- ""
- );
+ let warningHiddenPromise = waitForWarningState(warning, "hidden");
await BrowserTestUtils.waitForCondition(
() => warning.innerText == expectedWarningText,
=====================================
js/src/gc/GC.cpp
=====================================
@@ -1331,6 +1331,11 @@ void GCRuntime::assertNoMarkingWork() const {
}
#endif
+static size_t GetGCParallelThreadCount() {
+ AutoLockHelperThreadState lock;
+ return HelperThreadState().getGCParallelThreadCount(lock);
+}
+
bool GCRuntime::updateMarkersVector() {
MOZ_ASSERT(helperThreadCount >= 1,
"There must always be at least one mark task");
@@ -1339,8 +1344,8 @@ bool GCRuntime::updateMarkersVector() {
// Limit worker count to number of GC parallel tasks that can run
// concurrently, otherwise one thread can deadlock waiting on another.
- size_t targetCount = std::min(markingWorkerCount(),
- HelperThreadState().getGCParallelThreadCount());
+ size_t targetCount =
+ std::min(markingWorkerCount(), GetGCParallelThreadCount());
if (markers.length() > targetCount) {
return markers.resize(targetCount);
=====================================
js/src/gc/ParallelMarking.cpp
=====================================
@@ -103,6 +103,10 @@ bool ParallelMarker::markOneColor(MarkColor color, SliceBudget& sliceBudget) {
{
AutoLockHelperThreadState lock;
+ // There should always be enough parallel tasks to run our marking work.
+ MOZ_RELEASE_ASSERT(HelperThreadState().getGCParallelThreadCount(lock) >=
+ workerCount());
+
for (size_t i = 0; i < workerCount(); i++) {
gc->startTask(*tasks[i], lock);
}
=====================================
js/src/vm/HelperThreadState.h
=====================================
@@ -333,9 +333,11 @@ class GlobalHelperThreadState {
GCParallelTaskList& gcParallelWorklist() { return gcParallelWorklist_; }
- size_t getGCParallelThreadCount() const { return gcParallelThreadCount; }
+ size_t getGCParallelThreadCount(const AutoLockHelperThreadState& lock) const {
+ return gcParallelThreadCount;
+ }
void setGCParallelThreadCount(size_t count,
- const AutoLockHelperThreadState&) {
+ const AutoLockHelperThreadState& lock) {
MOZ_ASSERT(count >= 1);
MOZ_ASSERT(count <= threadCount);
gcParallelThreadCount = count;
View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/c4e8cd0f05884220aaf13fad10b1ac16d561a2a6...efee89783a7d79442a260275e93f39fce228f8c8
--
View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/c4e8cd0f05884220aaf13fad10b1ac16d561a2a6...efee89783a7d79442a260275e93f39fce228f8c8
You're receiving this email because of your account on gitlab.torproject.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.torproject.org/pipermail/tor-commits/attachments/20230801/78267dd4/attachment-0001.htm>
More information about the tor-commits
mailing list