Bug 1867358 part 3: Adjust NotificationController to avoid modifying mObservingState after Shutdown has been called. r=eeejay

We can sometimes reach the code that this patch is touching, just we've been
shutdown (as can be seen by the fact that we have a null mDocument pointer); in
that case, it's problematic for us to be assigning 'mObservingState' here. The
mObservingState assignment is meant to be *undoing* the temporary assignments
earlier in the function, but in this post-Shutdown situation, it's actually
causing us to forget that we already unregistered as a refresh observer, in a
nested stack level's call to Shutdown().

I think this one place is the only mObservingState assignment that's got this
problem. The other mObservingState assignments should all be OK (they don't
need a "have we shut down" check), because they're all either:
a) modifying mObservingState to record an actual change that we just made to
our RefreshDriver registration,
...or:
b) immediately adjacent to code that dereferences mDocument, implying that
mDocument is known-to-be-non-null at that point, which means we know we haven't
been Shutdown.

This patch unblocks us from adding a release assert in the
NotificationController destructor in the next patch in this series. (Without
this patch, that assertion can be made to fail, via this inadvertent
mObservingState assignment.)

Depends on D195041

Differential Revision: https://phabricator.services.mozilla.com/D195200

accessible/base/NotificationController.cpp

@@ -1021,10 +1021,19 @@ void NotificationController::WillRefresh(mozilla::TimeStamp aTime) {
     }
   }
 
-  mObservingState = eRefreshObserving;
   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;
 
   // Stop further processing if there are no new notifications of any kind or
   // events and document load is processed.