1070340 - Avoid interacting with the observer service from imgFrame during XPCOM shutdown

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Seth Fowler [:seth] [:s2h]]

So bug 1069652 makes us call imgFrame::Optimize() during imgFrame::UnlockImageData(), which is almost always what we want, except for in one scenario: during XPCOM shutdown. In this situation, we can't let Optimize() run, because it calls getPlatform::GetPlatform(), which needs the observer service to be available. During shutdown, it's not, and we get this kind of failure:

https://tbpl.mozilla.org/php/getParsedLog.php?id=48431848&tree=Try#error1

This bug will add an imagelib-global service called ShutdownTracker which lets us know whether the 'xpcom-shutdown' notification has been delivered. It's preferable to do this in a single place because shutdown observers aren't free, and we don't want to add the additional logic everywhere we need it. (And I suspect we'll need it in more places.)

This bug also adds a call to ShutdownTracker::ShutdownHasStarted() in imgFrame::Optimize(), so we don't try to optimize during shutdown.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Attachment: (Part 1) - Add ShutdownTracker to imagelib

This patch adds ShutdownTracker.

Comment 2

[Seth Fowler [:seth] [:s2h]]

Attachment: (Part 2) - Avoid optimizing imgFrame's during shutdown

We put ShutdownTracker to work immediately by avoiding imgFrame optimization during shutdown.

Comment 3

[Seth Fowler [:seth] [:s2h]]

Attachment: (Part 2) - Avoid optimizing imgFrame's during shutdown

Trivial fix.

Comment 4

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8492485 [details] [diff] [review]
(Part 1) - Add ShutdownTracker to imagelib

>+  /**
>+   * Check whether shutdown has started. Callers can use this to check whether
>+   * it's safe to access XPCOM services; if shutdown has started, such calls
>+   * must be avoided.
>+   *
>+   * @return true if shutdown has already started.
>+   */
>+  static bool ShutdownHasStarted();

This might be clearer if it was called GetHasShutdownStarted.

Comment 5

[Seth Fowler [:seth] [:s2h]]

Thanks for the review, Timothy! Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7e8bda1300c
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac1a42852e62

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f7e8bda1300c
https://hg.mozilla.org/mozilla-central/rev/ac1a42852e62