[Android] Simplify fragment management

Ludovic Fauvet git at videolan.org
Fri May 31 01:43:36 CEST 2013


vlc-ports/android | branch: master | Ludovic Fauvet <etix at videolan.org> | Wed May 29 15:38:30 2013 +0200| [bc4a6aa42f500e86d75001fa35f89aa52162083c] | committer: Edward Wang

Simplify fragment management

This commit removes a lot of code accumulated during the past months
while trying to fix all the issues we had with the Fragments, making it
bloated and hard to maintain. By simplifying the code most of the
problematic add() calls have been removed in favor of replace() which is
less prone to crash.

It is important to either use all replace() and avoid calls to add(), _or_
we use attach/detach and manage add() by ourselves. This is because
replace() itself internally calls add(). Mixing the two styles is unstable
and has lead to many crashes by IllegalStateException.

Back then, we were using replace() and add() simultaneously. Now, we have
removed all add() calls to theoretically prevent interference with replace().

Most of the original code and complexity came from the time all the scanning
was done within the fragments and when they had to be loaded even before
they were shown.

I do not pretend this commit will be a silver-bullet, but at least it's
an attempt to make things simpler to use and maintain.

It is also worth noting that this commit also fixes the long standing
issue of the audio fragment that was always stacked behind the current
active one.

> http://git.videolan.org/gitweb.cgi/vlc-ports/android.git/?a=commit;h=bc4a6aa42f500e86d75001fa35f89aa52162083c
---

 .../src/org/videolan/vlc/gui/MainActivity.java     |   38 +++-----------------
 1 file changed, 5 insertions(+), 33 deletions(-)

diff --git a/vlc-android/src/org/videolan/vlc/gui/MainActivity.java b/vlc-android/src/org/videolan/vlc/gui/MainActivity.java
index bb3da82..4a92c11 100644
--- a/vlc-android/src/org/videolan/vlc/gui/MainActivity.java
+++ b/vlc-android/src/org/videolan/vlc/gui/MainActivity.java
@@ -208,19 +208,9 @@ public class MainActivity extends SherlockFragmentActivity {
                  */
                 getSupportFragmentManager().popBackStack(null, FragmentManager.POP_BACK_STACK_INCLUSIVE);
 
-                /**
-                 * Do not move this getFragment("audio")!
-                 * This is to ensure that if audio is not already loaded, it
-                 * will be loaded ahead of the detach/attach below.
-                 * Otherwise if you add() a fragment after an attach/detach,
-                 * it will take over the placeholder and you will end up with
-                 * the audio fragment when some other fragment should be there.
-                 */
-                Fragment audioFragment = getFragment("audio");
-
+                /* Switch the fragment */
                 FragmentTransaction ft = getSupportFragmentManager().beginTransaction();
-                ft.detach(current);
-                ft.attach(getFragment(entry.id));
+                ft.replace(R.id.fragment_placeholder, getFragment(entry.id), entry.id);
                 ft.commit();
                 mCurrentFragment = entry.id;
 
@@ -232,7 +222,7 @@ public class MainActivity extends SherlockFragmentActivity {
                 getFragment(mCurrentFragment).setUserVisibleHint(true);
                 // HACK ALERT: Set underlying audio browser to be invisible too.
                 if(current.getTag().equals("tracks"))
-                    audioFragment.setUserVisibleHint(false);
+                    getFragment("audio").setUserVisibleHint(false);
 
                 mMenu.showContent();
             }
@@ -332,7 +322,6 @@ public class MainActivity extends SherlockFragmentActivity {
             found = true;
         }
 
-        mSidebarAdapter.lockSemaphore();
         /**
          * Let's see if Android recreated anything for us in the bundle.
          * Prevent duplicate creation of fragments, since mSidebarAdapter might
@@ -346,7 +335,6 @@ public class MainActivity extends SherlockFragmentActivity {
                 mSidebarAdapter.restoreFragment(fragmentTag, fragment);
             }
         }
-        mSidebarAdapter.unlockSemaphore();
 
         /**
          * Restore the last view.
@@ -364,9 +352,7 @@ public class MainActivity extends SherlockFragmentActivity {
             Log.d(TAG, "Reloading displayed fragment");
             Fragment ff = getFragment(mCurrentFragment);
             FragmentTransaction ft = getSupportFragmentManager().beginTransaction();
-            if(current != null)
-                ft.detach(current);
-            ft.attach(ff);
+            ft.replace(R.id.fragment_placeholder, ff, mCurrentFragment);
             ft.commit();
         }
     }
@@ -427,21 +413,7 @@ public class MainActivity extends SherlockFragmentActivity {
 
     private Fragment getFragment(String id)
     {
-        Fragment fragment = mSidebarAdapter.fetchFragment(id);
-
-        // Prevent fragment from being added twice. (IllegalStateException)
-        // See http://stackoverflow.com/questions/13745787/fragment-already-added-support-lib
-        mSidebarAdapter.lockSemaphore();
-        if(!mSidebarAdapter.isFragmentAdded(id)) {
-            getSupportFragmentManager()
-                .beginTransaction()
-                .add(R.id.fragment_placeholder, fragment, id)
-                .commitAllowingStateLoss();
-            mSidebarAdapter.setFragmentAdded(id);
-        }
-        mSidebarAdapter.unlockSemaphore();
-
-        return fragment;
+        return mSidebarAdapter.fetchFragment(id);
     }
 
     /** Create menu from XML



More information about the Android mailing list