onCreateOptionsMenu is being called too many times in ActionBar using tabs

Question

Here is my problem. I have an app where I am using ActionBar Sherlock with tabs, fragments with option menus. Every time I rotate the emulator, menus are added for all the fragments even those that are hidded/removed (I tried both).

This is the setting: One FragmentActivity, that has an ActionBar with

  final ActionBar bar = getSupportActionBar();

  bar.addTab(bar.newTab()
        .setText("1")
        .setTabListener(new MyTabListener(new FragmentList1())));

  bar.addTab(bar.newTab()
        .setText("2")
        .setTabListener(new MyTabListener(new FragmentList2())));

  bar.addTab(bar.newTab()
        .setText("3")
        .setTabListener(new MyTabListener(new FragmentList3())));

  bar.setNavigationMode(ActionBar.NAVIGATION_MODE_TABS);
  bar.setDisplayShowHomeEnabled(true);
  bar.setDisplayShowTitleEnabled(true);

The tabs all use the same Listener:

private class MyTabListener implements ActionBar.TabListener {
  private final FragmentListBase m_fragment;


  public MyTabListener(FragmentListBase fragment) {
     m_fragment = fragment;
  }


  public void onTabSelected(ActionBar.Tab tab, FragmentTransaction ft) {
     FragmentManager fragmentMgr = ActivityList.this.getSupportFragmentManager();
     FragmentTransaction transaction = fragmentMgr.beginTransaction();

        transaction.add(R.id.frmlyt_list, m_fragment, m_fragment.LIST_TAG);

     transaction.commit();
  }


  public void onTabUnselected(ActionBar.Tab tab, FragmentTransaction ft) {
     FragmentManager fragmentMgr = ActivityList.this.getSupportFragmentManager();
     FragmentTransaction transaction = fragmentMgr.beginTransaction();

     transaction.remove(m_fragment);
     transaction.commit();
  }


  public void onTabReselected(ActionBar.Tab tab, FragmentTransaction ft) {
  }
}

Each subclass of FragmentListBase has its own menu and therefore all 3 subclasses have :

  setHasOptionsMenu(true);

and the appropriate

public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) {
  Log.d(TAG, "OnCreateOptionsMenu");

  inflater.inflate(R.menu.il_options_menu, menu);
}

When I run the app I can see that the onCreateOptionsMenu is being called multiple times, for all the different fragments.

I'm totally stumped.

I tried posting the most code as possible without being overwhelming, if you find that something is missing, please advise.

[Edit] I added more logging, and it turns out that the fragment is being attached twice (or more) on rotation. One thing that I notice is that everything is being called multiple times except for the onCreate() method which is being called only once.

06.704:/WindowManager(72): Setting rotation to 0, animFlags=0
06.926:/ActivityManager(72): Config changed: { scale=1.0 imsi=310/260 loc=en_US touch=3 keys=1/1/2 nav=1/2 orien=L layout=0x10000014 uiMode=0x11 seq=35}
07.374:/FragmentList1(6880): onAttach
07.524:/FragmentList1(6880): onCreateView
07.564:/FragmentList1(6880): onAttach
07.564:/FragmentListBase(6880): onCreate
07.564:/FragmentList1(6880): OnCreateOptionsMenu
07.574:/FragmentList1(6880): OnCreateOptionsMenu
07.604:/FragmentList1(6880): onCreateView

[Edit 2]

Ok, I started tracing back into Android code and found this part here (that I edited to shorten this post).

/com_actionbarsherlock/src/android/support/v4/app/FragmentManager.java

public boolean dispatchCreateOptionsMenu(Menu menu, MenuInflater inflater) {
    if (mActive != null) {
        for (int i=0; i<mAdded.size(); i++) {
            Fragment f = mAdded.get(i);
            if (f != null && !f.mHidden && f.mHasMenu) {
                f.onCreateOptionsMenu(menu, inflater);
            }
        }
    }

The problem is that mAdded does indeed have multiple instances of FragmentList1 in it, so the onCreateOptionsMenu() method is "correctly" being called 3 times, but for different instances of the the FragmentList1 class. What I don't understand is why that class is being added multiple times... But that is a hell of a good lead.

Answer 1

I seem to have found the problem(s). I say problem(s) because on top of the multitude of menus, there is now also an Exception.

1) the call to

  bar.setNavigationMode(ActionBar.NAVIGATION_MODE_TABS);

which is after the calls to addTab() has a side effect of calling onTabSelected(). My TabListener would then add a FragmentList1 to the FragmentManager

2) rotating the device would destroy the Activity as expected, but would not destroy the Fragments. When the new Activity is created after rotation it would do two things :

1. create another set of Fragments that it would add to the FragmentManager. This is what was causing the multitude of Menus

2. call onTabSelected (via setNavigationMode()) which would perform the following code:

    if (null != fragmentMgr.findFragmentByTag(m_fragment.LIST_TAG)) {
       transaction.attach(m_fragment);
       transaction.show(m_fragment);
    }
    else {
       transaction.add(R.id.frmlyt_list, m_fragment, m_fragment.LIST_TAG);
    }
   

Basically if the fragment is already in the FragmentManager there is no need to add it, just show it. But there lies the problem. It's not the same Fragment! It's the Fragment that was created by the earlier instance of the Activity. So it would try to attach and show this newly created Fragment which would cause an Exception

The Solution.

There were a few things to do in order to fix all of this.

1) I moved the setNavigationMode() above the addTab()s.

2) this is how I now create my tabs:

  FragmentListBase fragment = (FragmentListBase)fragmentMgr.findFragmentByTag(FragmentList1.LIST_TAG_STATIC);
  if (null == fragment) {
     fragment = new FragmentList1();
  }
  bar.addTab(bar.newTab()
        .setText("1")
        .setTabListener(new MyTabListener(fragment)));

So upon Activity creation I have to check to see if the Fragments are already in the FragmentManager. If they are I use those instances, if not then I create new ones. This is done for all three tabs.

You may have noticed that there are two similar labels: m_fragment.LIST_TAG and FragmentList1.LIST_TAG_STATIC. Ah, this is lovely... ( <- sarcasm)

In ordrer to use my TagListener polymorphically I have declared the following non static variable in the base class:

public class FragmentListBase extends Fragment {
   public String LIST_TAG = null;
}

It is assigned from inside the descendents and allows me to look in the FragmentManager for the different descendents of FragmentListBase .

But I also need to search for specific descendents BEFORE they are created (because I need to know if I must create them or not), so I also have to declare the following static variable.

public class FragmentList1 extends FragmentListBase {
   public final static String LIST_TAG_STATIC = "TAG_LIST_1";

   public FragmentList1() {
      LIST_TAG = LIST_TAG_STATIC;
   };
}

Suffice to say that I am disapointed that nobody came up with this simple and elegant solution ( <- more sarcasm)

Thanks a lot to Jake Wharton who took the time to look at this for me :)

Answer 2

public FragmentListBase() {
    setRetainInstance(true);
    setHasOptionsMenu(true);
}

This will save/restore the individual states of each of the fragments upon rotation.

---

Another simple change you might want to make is calling transaction.replace(R.id.frmlyt_list, m_fragment, m_fragment.LIST_TAG) in the tab selected callback and getting rid of the content in the unselected callback.