Better Support Reduction in Page Count

[commonsguy]

MaterialTabs depends on a ViewPager, which in turn depends on a PagerAdapter. If the PagerAdapter is modified to return a lower value from getCount() than it had returned earlier, and if the selected tab's position is beyond the new getCount(), when we call notifyDataSetChanged() to update the PagerAdapter, MaterialTabs crashes. Exactly where and how it crashes depends a bit on the timing of events, but one common spot is what is currently line 443, where from what I can tell tabsContainer.getChildAt(position) returns null.

This sample project can be used to illustrate the problem, though I presently have a workaround set up. Here, the ViewPager has either 3 or 10 pages, based on the state of a "Fixed" checkable overflow item. In onOptionsItemSelected(), I need to adjust things based on the current checked state of that MenuItem:

  @Override
  public boolean onOptionsItemSelected(MenuItem item) {
    if (item.getItemId()==R.id.fixed) {
      item.setChecked(!item.isChecked());

      if (item.isChecked()) {
        if (pager.getCurrentItem()>2) {
          pager.setCurrentItem(2);
        }

        pager.postDelayed(new Runnable() {
          @Override
          public void run() {
            adapter.setPageCount(3);
            adapter.notifyDataSetChanged();
          }
        }, 100);
      }
      else {
        adapter.setPageCount(10);
        adapter.notifyDataSetChanged();
      }

      return(true);
    }

    return(super.onOptionsItemSelected(item));
  }

My original implementation just called setPageCount(3) on my SampleAdapter (subclass of FragmentPagerAdapter), then notifyDataSetChanged(). MaterialTabs crashed.

I then tried calling pager.setCurrentItem(2) before setPageCount(3), to try to switch the selected tab into the newly-safe range before reducing the page count. MaterialTabs crashed. Near as I can tell, it did not pick up the ViewPager current-item change before trying to deal with the page-count change.

I then tried moving the setPageCount(3) and notifyDataSetChanged() into a simple post() Runnable. That too tended to crash. Making it postDelayed(..., 100) gave me more reliable results. But any time we use post()/postDelayed() to try to postpone work until some other work gets done first, $DEITY kills a $CUTE_ANIMAL.

To reproduce the problem, roll back to my original implementation:

  @Override
  public boolean onOptionsItemSelected(MenuItem item) {
    if (item.getItemId()==R.id.fixed) {
      item.setChecked(!item.isChecked());

      if (item.isChecked()) {
        adapter.setPageCount(3);
        adapter.notifyDataSetChanged();
      }
      else {
        adapter.setPageCount(10);
        adapter.notifyDataSetChanged();
      }

      return(true);
    }

    return(super.onOptionsItemSelected(item));
  }

Then:

• Run the app, which starts off with three tabs
• Click the "Fixed" checkable overflow item to uncheck it, at which point you will have ten tabs
• Make "Editor #9" be the selected tab
• Click the "Fixed" checkable overflow item to check it

You should get one variation of the crash, dying on line 334:

java.lang.NullPointerException
        at io.karim.MaterialTabs.scrollToChild(MaterialTabs.java:334)
        at io.karim.MaterialTabs.access$1200(MaterialTabs.java:50)
        at io.karim.MaterialTabs$3.onGlobalLayout(MaterialTabs.java:407)
        at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:808)
        at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1768)
        at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1004)
        at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:5481)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:749)
        at android.view.Choreographer.doCallbacks(Choreographer.java:562)
        at android.view.Choreographer.doFrame(Choreographer.java:532)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:735)
        at android.os.Handler.handleCallback(Handler.java:730)
        at android.os.Handler.dispatchMessage(Handler.java:92)
        at android.os.Looper.loop(Looper.java:137)
        at android.app.ActivityThread.main(ActivityThread.java:5103)
        at java.lang.reflect.Method.invokeNative(Native Method)
        at java.lang.reflect.Method.invoke(Method.java:525)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:737)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553)
        at dalvik.system.NativeStart.main(Native Method)

Note that your existing code seems to handle expanding the number of pages just fine. It also works fine if the selected page is in the range of pages that is being retained (i.e., its position is inside the new getCount() value).

IMHO, asking us to move the selected page to a safe spot ourselves (via setCurrentItem()) is reasonable if you need that, but it really should not require us to then postDelayed() the actual page count change.

Thanks!