Commit 1ded75c5 authored by Daniel Holbert's avatar Daniel Holbert
Browse files

Bug 1867358 part 4: Add some release asserts (and upgrade an existing assert),... 

Bug 1867358 part 4: Add some release asserts (and upgrade an existing assert), to validate that NotificationController isn't still registered as a refresh observer when it's destroyed.  a=pascalc

If these assertions somehow fail, then we are probably doomed to crash anyway.
The new release assertions just make it so we'll crash in a controlled way,
with a more actionable backtrace, closer to the problem-spot.

This patch also removes one mDocument null-check that's becoming redundant
since it follows a release-assert that would make us abort if the pointer is
null.

Original Revision: https://phabricator.services.mozilla.com/D195042

Differential Revision: https://phabricator.services.mozilla.com/D199399
parent e3609aab

accessible/base/NotificationController.cpp

+16 −4
Original line number Diff line number Diff line
@@ -44,6 +44,8 @@ NotificationController::~NotificationController() { 
  if (mDocument) {

    Shutdown();

  }

  MOZ_RELEASE_ASSERT(mObservingState == eNotObservingRefresh,

                     "Must unregister before being destroyed");

}



////////////////////////////////////////////////////////////////////////////////
@@ -82,8 +84,13 @@ 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)");



  // Shutdown handling child documents.

  int32_t childDocCount = mHangingChildDocuments.Length();
@@ -660,12 +667,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;

  }