A tale of OOMs and thumbnails: the abstract

Richard Newman rnewman at mozilla.com
Tue Nov 5 15:36:46 PST 2013

> To summarize:
> 1. Prefer not to use inner-classes. If used, please try to make them static as much as possible.

Inner classes are actually fine. Make them static if it's easy to do so, but don't contort the code just to make it static. (A great example of that would be having a static inner class with a constructor parameter that is the outer class... that's just a non-static inner class!)

It often makes a lot of sense for the inner class to have a reference to a parent -- just don't leave it non-static unless it needs that reference.

(An IDE will often help you here by suggesting the change.)

Anonymous classes are more pernicious: unless they're scoped to a static context, they will always hold a reference. So in general, change from this:

  Foo.bar(new Baz() {
    public int noo() {
      return 5 + 6;

to this:

  private static class MyBaz implements Baz {
    public ...

  Foo.bar(new MyBaz());

or, indeed, to use a single instance of MyBaz if it doesn't have any state of its own.

However -- and very importantly -- this restriction isn't really necessary *unless* the lifespan of the inner instance will be longer than its outer instance. In our concrete example, the load task could live longer than the tab, but in our Foo Bar example it's likely that this is a throwaway delegate that won't cause a leak, and so the added code complication of a named inner class isn't worthwhile.

> 3. It’s better to create one private (preferably static) inner
> class member (and mark it final), that can handle different
> callbacks. This is a neat way of coding recommended everywhere.

That's usually true, but not always. Consider our "add to home screen" flow -- most users will never use this, so instantiating a static final listener would be a waste. Far better to pay the price at point of use.

I agree wholeheartedly with the rest!


More information about the mobile-firefox-dev mailing list