summaryrefslogtreecommitdiffstats
path: root/accessible/base
diff options
context:
space:
mode:
Diffstat (limited to 'accessible/base')
-rw-r--r--accessible/base/EventQueue.cpp8
-rw-r--r--accessible/base/EventQueue.h6
-rw-r--r--accessible/base/NotificationController.cpp67
3 files changed, 64 insertions, 17 deletions
diff --git a/accessible/base/EventQueue.cpp b/accessible/base/EventQueue.cpp
index d64b358f5e..36787a44b1 100644
--- a/accessible/base/EventQueue.cpp
+++ b/accessible/base/EventQueue.cpp
@@ -348,7 +348,9 @@ void EventQueue::ProcessEventQueue() {
AccEvent* event = events[idx];
uint32_t eventType = event->mEventType;
LocalAccessible* target = event->GetAccessible();
- if (!target || target->IsDefunct()) continue;
+ if (!target || target->IsDefunct()) {
+ continue;
+ }
// Collect select changes
if (IPCAccessibilityActive()) {
@@ -413,7 +415,9 @@ void EventQueue::ProcessEventQueue() {
nsEventShell::FireEvent(event);
- if (!mDocument) return;
+ if (!mDocument) {
+ return;
+ }
}
if (mDocument && IPCAccessibilityActive() &&
diff --git a/accessible/base/EventQueue.h b/accessible/base/EventQueue.h
index 434acd3c09..87bcf89340 100644
--- a/accessible/base/EventQueue.h
+++ b/accessible/base/EventQueue.h
@@ -7,6 +7,7 @@
#define mozilla_a11y_EventQueue_h_
#include "AccEvent.h"
+#include "mozilla/Assertions.h"
namespace mozilla {
namespace a11y {
@@ -18,7 +19,10 @@ class DocAccessible;
*/
class EventQueue {
protected:
- explicit EventQueue(DocAccessible* aDocument) : mDocument(aDocument) {}
+ explicit EventQueue(DocAccessible* aDocument) : mDocument(aDocument) {
+ MOZ_ASSERT(mDocument,
+ "There's no point creating an event queue for a null document");
+ }
/**
* Put an accessible event into the queue to process it later.
diff --git a/accessible/base/NotificationController.cpp b/accessible/base/NotificationController.cpp
index 6ac4223b7e..c7a702831f 100644
--- a/accessible/base/NotificationController.cpp
+++ b/accessible/base/NotificationController.cpp
@@ -41,7 +41,11 @@ NotificationController::NotificationController(DocAccessible* aDocument,
NotificationController::~NotificationController() {
NS_ASSERTION(!mDocument, "Controller wasn't shutdown properly!");
- if (mDocument) Shutdown();
+ if (mDocument) {
+ Shutdown();
+ }
+ MOZ_RELEASE_ASSERT(mObservingState == eNotObservingRefresh,
+ "Must unregister before being destroyed");
}
////////////////////////////////////////////////////////////////////////////////
@@ -53,7 +57,9 @@ NS_IMPL_CYCLE_COLLECTING_NATIVE_RELEASE(NotificationController)
NS_IMPL_CYCLE_COLLECTION_CLASS(NotificationController)
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(NotificationController)
- if (tmp->mDocument) tmp->Shutdown();
+ if (tmp->mDocument) {
+ tmp->Shutdown();
+ }
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(NotificationController)
@@ -78,8 +84,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
void NotificationController::Shutdown() {
if (mObservingState != eNotObservingRefresh &&
mPresShell->RemoveRefreshObserver(this, FlushType::Display)) {
+ // Note, this was our last chance to unregister, since we're about to
+ // clear mPresShell further down in this function.
mObservingState = eNotObservingRefresh;
}
+ MOZ_RELEASE_ASSERT(mObservingState == eNotObservingRefresh,
+ "Must unregister before being destroyed (and we just "
+ "passed our last change to unregister)");
+ // Immediately null out mPresShell, to prevent us from being registered as a
+ // refresh observer again.
+ mPresShell = nullptr;
// Shutdown handling child documents.
int32_t childDocCount = mHangingChildDocuments.Length();
@@ -92,7 +106,6 @@ void NotificationController::Shutdown() {
mHangingChildDocuments.Clear();
mDocument = nullptr;
- mPresShell = nullptr;
mTextHash.Clear();
mContentInsertions.Clear();
@@ -434,9 +447,11 @@ void NotificationController::ScheduleContentInsertion(
}
void NotificationController::ScheduleProcessing() {
- // If notification flush isn't planed yet start notification flush
+ // If notification flush isn't planned yet, start notification flush
// asynchronously (after style and layout).
- if (mObservingState == eNotObservingRefresh) {
+ // Note: the mPresShell null-check might be unnecessary; it's just to prevent
+ // a null-deref here, if we somehow get called after we've been shut down.
+ if (mObservingState == eNotObservingRefresh && mPresShell) {
if (mPresShell->AddRefreshObserver(this, FlushType::Display,
"Accessibility notifications")) {
mObservingState = eRefreshObserving;
@@ -656,12 +671,17 @@ void NotificationController::WillRefresh(mozilla::TimeStamp aTime) {
AUTO_PROFILER_LABEL("NotificationController::WillRefresh", A11Y);
- // If the document accessible that notification collector was created for is
- // now shut down, don't process notifications anymore.
- NS_ASSERTION(
+ // If mDocument is null, the document accessible that this notification
+ // controller was created for is now shut down. This means we've lost our
+ // ability to unregister ourselves, which is bad. (However, it also shouldn't
+ // be logically possible for us to get here with a null mDocument; the only
+ // thing that clears that pointer is our Shutdown() method, which first
+ // unregisters and fatally asserts if that fails).
+ MOZ_RELEASE_ASSERT(
mDocument,
"The document was shut down while refresh observer is attached!");
- if (!mDocument || ipc::ProcessChild::ExpectingShutdown()) {
+
+ if (ipc::ProcessChild::ExpectingShutdown()) {
return;
}
@@ -835,7 +855,9 @@ void NotificationController::WillRefresh(mozilla::TimeStamp aTime) {
nsTArray<RefPtr<DocAccessible>> newChildDocs;
for (uint32_t idx = 0; idx < hangingDocCnt; idx++) {
DocAccessible* childDoc = mHangingChildDocuments[idx];
- if (childDoc->IsDefunct()) continue;
+ if (childDoc->IsDefunct()) {
+ continue;
+ }
if (IPCAccessibilityActive() && !mDocument->IPCDoc()) {
childDoc->Shutdown();
@@ -874,12 +896,16 @@ void NotificationController::WillRefresh(mozilla::TimeStamp aTime) {
uint32_t childDocCnt = mDocument->ChildDocumentCount(), childDocIdx = 0;
for (; childDocIdx < childDocCnt; childDocIdx++) {
DocAccessible* childDoc = mDocument->GetChildDocumentAt(childDocIdx);
- if (!childDoc->HasLoadState(DocAccessible::eCompletelyLoaded)) break;
+ if (!childDoc->HasLoadState(DocAccessible::eCompletelyLoaded)) {
+ break;
+ }
}
if (childDocIdx == childDocCnt) {
mDocument->ProcessLoad();
- if (!mDocument) return;
+ if (!mDocument) {
+ return;
+ }
}
}
@@ -909,7 +935,9 @@ void NotificationController::WillRefresh(mozilla::TimeStamp aTime) {
uint32_t notificationCount = notifications.Length();
for (uint32_t idx = 0; idx < notificationCount; idx++) {
notifications[idx]->Process();
- if (!mDocument) return;
+ if (!mDocument) {
+ return;
+ }
}
if (ipc::ProcessChild::ExpectingShutdown()) {
@@ -991,8 +1019,19 @@ void NotificationController::WillRefresh(mozilla::TimeStamp aTime) {
}
}
+ if (!mDocument) {
+ // A null mDocument means we've gotten a Shutdown() call (presumably via
+ // some script that we triggered above), and that means we're done here.
+ // Note: in this case, it's important that don't modify mObservingState;
+ // Shutdown() will have *unregistered* us as a refresh observer, and we
+ // don't want to mistakenly overwrite mObservingState and fool ourselves
+ // into thinking we've re-registered when we really haven't!
+ MOZ_ASSERT(mObservingState == eNotObservingRefresh,
+ "We've been shutdown, which means we should've been "
+ "unregistered as a refresh observer");
+ return;
+ }
mObservingState = eRefreshObserving;
- if (!mDocument) return;
// Stop further processing if there are no new notifications of any kind or
// events and document load is processed.