diff options
Diffstat (limited to 'accessible')
-rw-r--r-- | accessible/base/EventQueue.cpp | 8 | ||||
-rw-r--r-- | accessible/base/EventQueue.h | 6 | ||||
-rw-r--r-- | accessible/base/NotificationController.cpp | 67 | ||||
-rw-r--r-- | accessible/generic/HyperTextAccessible.cpp | 11 | ||||
-rw-r--r-- | accessible/generic/LocalAccessible.h | 2 |
5 files changed, 76 insertions, 18 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. diff --git a/accessible/generic/HyperTextAccessible.cpp b/accessible/generic/HyperTextAccessible.cpp index 943ac63c1c..b5102fa514 100644 --- a/accessible/generic/HyperTextAccessible.cpp +++ b/accessible/generic/HyperTextAccessible.cpp @@ -776,6 +776,17 @@ LayoutDeviceIntRect HyperTextAccessible::GetCaretRect(nsIWidget** aWidget) { void HyperTextAccessible::GetSelectionDOMRanges(SelectionType aSelectionType, nsTArray<nsRange*>* aRanges) { + if (IsDoc() && !AsDoc()->HasLoadState(DocAccessible::eTreeConstructed)) { + // Rarely, a client query can be handled after a DocAccessible is created + // but before the initial tree is constructed, since DoInitialUpdate happens + // during a refresh tick. In that case, there might be a DOM selection, but + // we can't use it. We will crash if we try due to mContent being null, etc. + // This should only happen in the parent process because we should never + // try to push the cache in a content process before the initial tree is + // constructed. + MOZ_ASSERT(XRE_IsParentProcess(), "Query before DoInitialUpdate"); + return; + } // Ignore selection if it is not visible. RefPtr<nsFrameSelection> frameSelection = FrameSelection(); if (!frameSelection || frameSelection->GetDisplaySelection() <= diff --git a/accessible/generic/LocalAccessible.h b/accessible/generic/LocalAccessible.h index e4a4f659f5..d1dd0efa8f 100644 --- a/accessible/generic/LocalAccessible.h +++ b/accessible/generic/LocalAccessible.h @@ -928,7 +928,7 @@ class LocalAccessible : public nsISupports, public Accessible { // Data Members // mContent can be null in a DocAccessible if the document has no body or - // root element. + // root element, or if the initial tree hasn't been constructed yet. nsCOMPtr<nsIContent> mContent; RefPtr<DocAccessible> mDoc; |