Browse Source

prevent NPE
code enhancement

tobiaskaminsky 7 years ago
parent
commit
cc0130aac0

+ 6 - 6
src/main/java/com/owncloud/android/ui/preview/PreviewImageActivity.java

@@ -152,7 +152,7 @@ public class PreviewImageActivity extends FileActivity implements
                     parentFolder, getAccount(), getStorageManager(), MainApp.isOnlyOnDevice());
         }
 
-        mViewPager = (ExtendedViewPager) findViewById(R.id.fragmentPager);
+        mViewPager = findViewById(R.id.fragmentPager);
 
         int position = mHasSavedPosition ? mSavedPosition : mPreviewImagePagerAdapter.getFilePosition(getFile());
         position = (position >= 0) ? position : 0;
@@ -318,8 +318,6 @@ public class PreviewImageActivity extends FileActivity implements
                 AccountUtils.getCurrentOwnCloudAccount(this));
         showDetailsIntent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP | Intent.FLAG_ACTIVITY_SINGLE_TOP);
         startActivity(showDetailsIntent);
-        int pos = mPreviewImagePagerAdapter.getFilePosition(file);
-        file = mPreviewImagePagerAdapter.getFileAt(pos);
         finish();
     }
 
@@ -347,10 +345,12 @@ public class PreviewImageActivity extends FileActivity implements
         mHasSavedPosition = true;
         if (mDownloaderBinder == null) {
             mRequestWaitingForBinder = true;
-            
         } else {
-            OCFile currentFile = mPreviewImagePagerAdapter.getFileAt(position); 
-            getSupportActionBar().setTitle(currentFile.getFileName());
+            OCFile currentFile = mPreviewImagePagerAdapter.getFileAt(position);
+
+            if (getSupportActionBar() != null && currentFile != null) {
+                getSupportActionBar().setTitle(currentFile.getFileName());
+            }
             setDrawerIndicatorEnabled(false);
 
             // Call to reset image zoom to initial state

+ 37 - 200
src/main/java/com/owncloud/android/ui/preview/PreviewImagePagerAdapter.java

@@ -1,4 +1,4 @@
-/**
+/*
  *   ownCloud Android client application
  *
  *   @author David A. Velasco
@@ -23,6 +23,7 @@ import android.accounts.Account;
 import android.support.v4.app.Fragment;
 import android.support.v4.app.FragmentManager;
 import android.support.v4.app.FragmentStatePagerAdapter;
+import android.util.SparseArray;
 import android.view.ViewGroup;
 
 import com.owncloud.android.datamodel.FileDataStorageManager;
@@ -32,17 +33,15 @@ import com.owncloud.android.ui.fragment.FileFragment;
 import com.owncloud.android.utils.FileSortOrderByName;
 import com.owncloud.android.utils.FileStorageUtils;
 
-import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
-import java.util.Map;
 import java.util.Set;
 import java.util.Vector;
 
+import javax.annotation.Nullable;
+
 /**
  * Adapter class that provides Fragment instances
  */
-//public class PreviewImagePagerAdapter extends PagerAdapter {
 public class PreviewImagePagerAdapter extends FragmentStatePagerAdapter {
     
     private Vector<OCFile> mImageFiles;
@@ -52,10 +51,10 @@ public class PreviewImagePagerAdapter extends FragmentStatePagerAdapter {
     private Set<Integer> mDownloadErrors;
     private FileDataStorageManager mStorageManager;
 
-    private Map<Integer, FileFragment> mCachedFragments;
+    private SparseArray<FileFragment> mCachedFragments;
 
     /**
-     * Constructor.
+     * Constructor
      * 
      * @param fragmentManager   {@link FragmentManager} instance that will handle
      *                          the {@link Fragment}s provided by the adapter.
@@ -80,16 +79,17 @@ public class PreviewImagePagerAdapter extends FragmentStatePagerAdapter {
         mAccount = account;
         mStorageManager = storageManager;
         mImageFiles = mStorageManager.getFolderImages(parentFolder, onlyOnDevice);
+        
         mImageFiles = FileSortOrderByName.sort_a_to_z.sortCloudFiles(mImageFiles);
-        mObsoleteFragments = new HashSet<Object>();
-        mObsoletePositions = new HashSet<Integer>();
-        mDownloadErrors = new HashSet<Integer>();
-        //mFragmentManager = fragmentManager;
-        mCachedFragments = new HashMap<Integer, FileFragment>();
+        
+        mObsoleteFragments = new HashSet<>();
+        mObsoletePositions = new HashSet<>();
+        mDownloadErrors = new HashSet<>();
+        mCachedFragments = new SparseArray<>();
     }
 
     /**
-     * Constructor.
+     * Constructor
      *
      * @param fragmentManager {@link FragmentManager} instance that will handle
      *                        the {@link Fragment}s provided by the adapter.
@@ -118,29 +118,33 @@ public class PreviewImagePagerAdapter extends FragmentStatePagerAdapter {
             mImageFiles = FileStorageUtils.sortOcFolderDescDateModified(mImageFiles);
         }
 
-        mObsoleteFragments = new HashSet<Object>();
-        mObsoletePositions = new HashSet<Integer>();
-        mDownloadErrors = new HashSet<Integer>();
-        //mFragmentManager = fragmentManager;
-        mCachedFragments = new HashMap<Integer, FileFragment>();
+        mObsoleteFragments = new HashSet<>();
+        mObsoletePositions = new HashSet<>();
+        mDownloadErrors = new HashSet<>();
+        mCachedFragments = new SparseArray<>();
     }
 
     /**
      * Returns the image files handled by the adapter.
-     * 
-     * @return  A vector with the image files handled by the adapter.
+     *
+     * @return OCFile desired image or null if position is not in adapter
      */
-    protected OCFile getFileAt(int position) {
-        return mImageFiles.get(position);
+    @Nullable
+    OCFile getFileAt(int position) {
+        try {
+            return mImageFiles.get(position);
+        } catch (IndexOutOfBoundsException exception) {
+            return null;
+        }
     }
 
     
     public Fragment getItem(int i) {
         OCFile file = mImageFiles.get(i);
         Fragment fragment;
+
         if (file.isDown()) {
             fragment = PreviewImageFragment.newInstance(file, mObsoletePositions.contains(i), false);
-            
         } else {
             if (mDownloadErrors.contains(i)) {
                 fragment = FileDownloadFragment.newInstance(file, mAccount, true);
@@ -171,32 +175,23 @@ public class PreviewImagePagerAdapter extends FragmentStatePagerAdapter {
 
     
     public void updateFile(int position, OCFile file) {
-        FileFragment fragmentToUpdate = mCachedFragments.get(Integer.valueOf(position));
+        FileFragment fragmentToUpdate = mCachedFragments.get(position);
         if (fragmentToUpdate != null) {
             mObsoleteFragments.add(fragmentToUpdate);
         }
-        mObsoletePositions.add(Integer.valueOf(position));
+        mObsoletePositions.add(position);
         mImageFiles.set(position, file);
     }
     
     
     public void updateWithDownloadError(int position) {
-        FileFragment fragmentToUpdate = mCachedFragments.get(Integer.valueOf(position));
+        FileFragment fragmentToUpdate = mCachedFragments.get(position);
         if (fragmentToUpdate != null) {
             mObsoleteFragments.add(fragmentToUpdate);
         }
-        mDownloadErrors.add(Integer.valueOf(position));
+        mDownloadErrors.add(position);
     }
     
-    public void clearErrorAt(int position) {
-        FileFragment fragmentToUpdate = mCachedFragments.get(Integer.valueOf(position));
-        if (fragmentToUpdate != null) {
-            mObsoleteFragments.add(fragmentToUpdate);
-        }
-        mDownloadErrors.remove(Integer.valueOf(position));
-    }
-    
-    
     @Override
     public int getItemPosition(Object object) {
         if (mObsoleteFragments.contains(object)) {
@@ -206,187 +201,29 @@ public class PreviewImagePagerAdapter extends FragmentStatePagerAdapter {
         return super.getItemPosition(object);
     }
 
-
     @Override
     public Object instantiateItem(ViewGroup container, int position) {
         Object fragment = super.instantiateItem(container, position);
-        mCachedFragments.put(Integer.valueOf(position), (FileFragment)fragment);
+        mCachedFragments.put(position, (FileFragment) fragment);
         return fragment;
     }
     
     @Override
     public void destroyItem(ViewGroup container, int position, Object object) {
-       mCachedFragments.remove(Integer.valueOf(position));
+        mCachedFragments.remove(position);
        super.destroyItem(container, position, object);
     }
 
-
-    public boolean pendingErrorAt(int position) {
-        return mDownloadErrors.contains(Integer.valueOf(position));
-    }
-
     /**
      * Reset the image zoom to default value for each CachedFragments
      */
-    public void resetZoom() {
-        Iterator<FileFragment> entries = mCachedFragments.values().iterator();
-        while (entries.hasNext()) {
-        FileFragment fileFragment = entries.next();
+    void resetZoom() {
+        for (int i = 0; i < mCachedFragments.size(); i++) {
+            FileFragment fileFragment = mCachedFragments.valueAt(i);
+            
             if (fileFragment instanceof PreviewImageFragment) {
                 ((PreviewImageFragment) fileFragment).getImageView().resetZoom();
             }
         }
     }
-
-    /* -*
-     * Called when a change in the shown pages is going to start being made.
-     * 
-     * @param   container   The containing View which is displaying this adapter's page views.
-     *- /
-    @Override
-    public void startUpdate(ViewGroup container) {
-        Log_OC.e(TAG, "** startUpdate");
-    }
-
-    @Override
-    public Object instantiateItem(ViewGroup container, int position) {
-        Log_OC.e(TAG, "** instantiateItem " + position);
-        
-        if (mFragments.size() > position) {
-            Fragment fragment = mFragments.get(position);
-            if (fragment != null) {
-                Log_OC.e(TAG, "** \t returning cached item");
-                return fragment;
-            }
-        }
-
-        if (mCurTransaction == null) {
-            mCurTransaction = mFragmentManager.beginTransaction();
-        }
-
-        Fragment fragment = getItem(position);
-        if (mSavedState.size() > position) {
-            Fragment.SavedState savedState = mSavedState.get(position);
-            if (savedState != null) {
-                // TODO WATCH OUT:
-                // * The Fragment must currently be attached to the FragmentManager.
-                // * A new Fragment created using this saved state must be the same class type as the Fragment it was created from.
-                // * The saved state can not contain dependencies on other fragments -- that is it can't use putFragment(Bundle, String, Fragment) 
-                //   to store a fragment reference                 
-                fragment.setInitialSavedState(savedState);
-            }
-        }
-        while (mFragments.size() <= position) {
-            mFragments.add(null);
-        }
-        fragment.setMenuVisibility(false);
-        mFragments.set(position, fragment);
-        //Log_OC.e(TAG, "** \t adding fragment at position " + position + ", containerId " + container.getId());
-        mCurTransaction.add(container.getId(), fragment);
-
-        return fragment;
-    }
-
-    @Override
-    public void destroyItem(ViewGroup container, int position, Object object) {
-        Log_OC.e(TAG, "** destroyItem " + position);
-        Fragment fragment = (Fragment)object;
-        
-        if (mCurTransaction == null) {
-            mCurTransaction = mFragmentManager.beginTransaction();
-        }
-        Log_OC.e(TAG, "** \t removing fragment at position " + position);
-        while (mSavedState.size() <= position) {
-            mSavedState.add(null);
-        }
-        mSavedState.set(position, mFragmentManager.saveFragmentInstanceState(fragment));
-        mFragments.set(position, null);
-
-        mCurTransaction.remove(fragment);
-    }
-
-    @Override
-    public void setPrimaryItem(ViewGroup container, int position, Object object) {
-        Fragment fragment = (Fragment)object;
-        if (fragment != mCurrentPrimaryItem) {
-            if (mCurrentPrimaryItem != null) {
-                mCurrentPrimaryItem.setMenuVisibility(false);
-            }
-            if (fragment != null) {
-                fragment.setMenuVisibility(true);
-            }
-            mCurrentPrimaryItem = fragment;
-        }
-    }
-
-    @Override
-    public void finishUpdate(ViewGroup container) {
-        Log_OC.e(TAG, "** finishUpdate (start)");
-        if (mCurTransaction != null) {
-            mCurTransaction.commitAllowingStateLoss();
-            mCurTransaction = null;
-            mFragmentManager.executePendingTransactions();
-        }
-        Log_OC.e(TAG, "** finishUpdate (end)");
-    }
-
-    @Override
-    public boolean isViewFromObject(View view, Object object) {
-        return ((Fragment)object).getView() == view;
-    }
-
-    @Override
-    public Parcelable saveState() {
-        Bundle state = null;
-        if (mSavedState.size() > 0) {
-            state = new Bundle();
-            Fragment.SavedState[] savedStates = new Fragment.SavedState[mSavedState.size()];
-            mSavedState.toArray(savedStates);
-            state.putParcelableArray("states", savedStates);
-        }
-        for (int i=0; i<mFragments.size(); i++) {
-            Fragment fragment = mFragments.get(i);
-            if (fragment != null) {
-                if (state == null) {
-                    state = new Bundle();
-                }
-                String key = "f" + i;
-                mFragmentManager.putFragment(state, key, fragment);
-            }
-        }
-        return state;
-    }
-
-    @Override
-    public void restoreState(Parcelable state, ClassLoader loader) {
-        if (state != null) {
-            Bundle bundle = (Bundle)state;
-            bundle.setClassLoader(loader);
-            Parcelable[] states = bundle.getParcelableArray("states");
-            mSavedState.clear();
-            mFragments.clear();
-            if (states != null) {
-                for (int i=0; i<states.length; i++) {
-                    mSavedState.add((Fragment.SavedState)states[i]);
-                }
-            }
-            Iterable<String> keys = bundle.keySet();
-            for (String key: keys) {
-                if (key.startsWith("f")) {
-                    int index = Integer.parseInt(key.substring(1));
-                    Fragment f = mFragmentManager.getFragment(bundle, key);
-                    if (f != null) {
-                        while (mFragments.size() <= index) {
-                            mFragments.add(null);
-                        }
-                        f.setMenuVisibility(false);
-                        mFragments.set(index, f);
-                    } else {
-                        Log_OC.w(TAG, "Bad fragment at key " + key);
-                    }
-                }
-            }
-        }
-    }
-    */
 }