From fed215a37a7e8c8d64c5b653c3a2aa7b6632b2c3 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Wed, 23 Oct 2013 11:09:07 -0700 Subject: [PATCH] Crash fix for rotate in media detail view We were crashing when rotating in the detail view, because the detail view fragment had assumed that its parent activity had already loaded up its cursor and such before it got created -- an assumption which fails dramatically when rotating, or switching away and back after we got paged out of memory. Check whether we got an object, and if not ask the parent to ping us when it's done loading. Also tweaked one of the older delay hacks to avoid a second crash related to the pager's adapter. Change-Id: Ib91c0fef0d8ffd0e6b8e0e4874f3ee9be3e9cf34 --- .../contributions/ContributionsActivity.java | 34 +++++- .../commons/media/MediaDetailFragment.java | 115 ++++++++---------- .../media/MediaDetailPagerFragment.java | 7 +- .../commons/upload/MultipleShareActivity.java | 8 ++ 4 files changed, 100 insertions(+), 64 deletions(-) diff --git a/commons/src/main/java/org/wikimedia/commons/contributions/ContributionsActivity.java b/commons/src/main/java/org/wikimedia/commons/contributions/ContributionsActivity.java index 9152cacab..62d462f1c 100644 --- a/commons/src/main/java/org/wikimedia/commons/contributions/ContributionsActivity.java +++ b/commons/src/main/java/org/wikimedia/commons/contributions/ContributionsActivity.java @@ -1,5 +1,6 @@ package org.wikimedia.commons.contributions; +import android.database.DataSetObserver; import android.os.IBinder; import android.support.v4.app.FragmentManager; import android.support.v4.app.LoaderManager; @@ -12,6 +13,7 @@ import android.support.v4.widget.CursorAdapter; import android.util.Log; import android.view.View; import android.widget.AdapterView; +import android.widget.Adapter; import com.actionbarsherlock.view.Menu; import com.actionbarsherlock.view.MenuItem; @@ -36,6 +38,7 @@ public class ContributionsActivity private Cursor allContributions; private ContributionsListFragment contributionsList; private MediaDetailPagerFragment mediaDetails; + private ArrayList observersWaitingForLoad = new ArrayList(); private Campaign campaign; @@ -226,6 +229,7 @@ public class ContributionsActivity ((MediaListAdapter)contributionsList.getAdapter()).updateMediaList((ArrayList) result); } } + notifyAndClearDataSetObservers(); } public void onLoaderReset(Loader cursorLoader) { @@ -237,7 +241,10 @@ public class ContributionsActivity } public Media getMediaAtPosition(int i) { - if(campaign == null) { + if (contributionsList.getAdapter() == null) { + // not yet ready to return data + return null; + } else if(campaign == null) { return Contribution.fromCursor((Cursor) contributionsList.getAdapter().getItem(i)); } else { return (Media) contributionsList.getAdapter().getItem(i); @@ -255,6 +262,31 @@ public class ContributionsActivity // Do nothing for now } + private void notifyAndClearDataSetObservers() { + for (DataSetObserver observer : observersWaitingForLoad) { + observer.onChanged(); + } + observersWaitingForLoad.clear(); + } + + public void registerDataSetObserver(DataSetObserver observer) { + Adapter adapter = contributionsList.getAdapter(); + if (adapter == null) { + observersWaitingForLoad.add(observer); + } else { + adapter.registerDataSetObserver(observer); + } + } + + public void unregisterDataSetObserver(DataSetObserver observer) { + Adapter adapter = contributionsList.getAdapter(); + if (adapter == null) { + observersWaitingForLoad.remove(observer); + } else { + adapter.registerDataSetObserver(observer); + } + } + public void onBackStackChanged() { if(mediaDetails != null && mediaDetails.isVisible()) { getSupportActionBar().setDisplayHomeAsUpEnabled(true); diff --git a/commons/src/main/java/org/wikimedia/commons/media/MediaDetailFragment.java b/commons/src/main/java/org/wikimedia/commons/media/MediaDetailFragment.java index cf0c79db4..bee987dcc 100644 --- a/commons/src/main/java/org/wikimedia/commons/media/MediaDetailFragment.java +++ b/commons/src/main/java/org/wikimedia/commons/media/MediaDetailFragment.java @@ -1,6 +1,7 @@ package org.wikimedia.commons.media; import android.content.Intent; +import android.database.DataSetObserver; import android.graphics.*; import android.os.*; import android.text.*; @@ -62,6 +63,7 @@ public class MediaDetailFragment extends SherlockFragment { private boolean categoriesPresent = false; private ViewTreeObserver.OnGlobalLayoutListener layoutListener; // for layout stuff, only used once! private ViewTreeObserver.OnScrollChangedListener scrollListener; + DataSetObserver dataObserver; private AsyncTask detailFetchTask; private LicenseList licenseList; @@ -93,7 +95,6 @@ public class MediaDetailFragment extends SherlockFragment { index = getArguments().getInt("index"); initialListTop = 0; } - final Media media = detailProvider.getMediaAtPosition(index); categoryNames = new ArrayList(); categoryNames.add(getString(R.string.detail_panel_cats_loading)); @@ -113,18 +114,60 @@ public class MediaDetailFragment extends SherlockFragment { licenseList = new LicenseList(getActivity()); - // Enable or disable editing on the title - /* - title.setClickable(editable); - title.setFocusable(editable); - title.setCursorVisible(editable); - title.setFocusableInTouchMode(editable); - if(!editable) { - title.setBackgroundDrawable(null); + Media media = detailProvider.getMediaAtPosition(index); + if (media == null) { + // Ask the detail provider to ping us when we're ready + Log.d("Commons", "MediaDetailFragment not yet ready to display details; registering observer"); + dataObserver = new DataSetObserver() { + public void onChanged() { + Log.d("Commons", "MediaDetailFragment ready to display delayed details!"); + detailProvider.unregisterDataSetObserver(dataObserver); + dataObserver = null; + displayMediaDetails(detailProvider.getMediaAtPosition(index)); + } + }; + detailProvider.registerDataSetObserver(dataObserver); + } else { + Log.d("Commons", "MediaDetailFragment ready to display details"); + displayMediaDetails(media); } - */ + // Progressively darken the image in the background when we scroll detail pane up + scrollListener = new ViewTreeObserver.OnScrollChangedListener() { + public void onScrollChanged() { + updateTheDarkness(); + } + }; + view.getViewTreeObserver().addOnScrollChangedListener(scrollListener); + // Layout layoutListener to size the spacer item relative to the available space. + // There may be a .... better way to do this. + layoutListener = new ViewTreeObserver.OnGlobalLayoutListener() { + private int currentHeight = -1; + + public void onGlobalLayout() { + int viewHeight = view.getHeight(); + //int textHeight = title.getLineHeight(); + int paddingDp = 112; + float paddingPx = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, paddingDp, getResources().getDisplayMetrics()); + int newHeight = viewHeight - Math.round(paddingPx); + + if (newHeight != currentHeight) { + currentHeight = newHeight; + ViewGroup.LayoutParams params = spacer.getLayoutParams(); + params.height = newHeight; + spacer.setLayoutParams(params); + + scrollView.scrollTo(0, initialListTop); + } + + } + }; + view.getViewTreeObserver().addOnGlobalLayoutListener(layoutListener); + return view; + } + + private void displayMediaDetails(final Media media) { String actualUrl = (media.getLocalUri() != null && TextUtils.isEmpty(media.getLocalUri().toString())) ? media.getLocalUri().toString() : media.getThumbnailUrl(640); if(actualUrl.startsWith("http")) { ImageLoader loader = ((CommonsApplication)getActivity().getApplicationContext()).getImageLoader(); @@ -212,58 +255,6 @@ public class MediaDetailFragment extends SherlockFragment { title.setText(media.getDisplayTitle()); desc.setText(""); // fill in from network... license.setText(""); // fill in from network... - - /* - title.addTextChangedListener(new TextWatcher() { - public void beforeTextChanged(CharSequence charSequence, int i, int i2, int i3) { - - } - - public void onTextChanged(CharSequence charSequence, int i, int i2, int i3) { - detailProvider.getMediaAtPosition(index).setFilename(title.getText().toString()); - detailProvider.getMediaAtPosition(index).setTag("isDirty", true); - detailProvider.notifyDatasetChanged(); - } - - public void afterTextChanged(Editable editable) { - - } - }); - */ - - // Progressively darken the image in the background when we scroll detail pane up - scrollListener = new ViewTreeObserver.OnScrollChangedListener() { - public void onScrollChanged() { - updateTheDarkness(); - } - }; - view.getViewTreeObserver().addOnScrollChangedListener(scrollListener); - - // Layout layoutListener to size the spacer item relative to the available space. - // There may be a .... better way to do this. - layoutListener = new ViewTreeObserver.OnGlobalLayoutListener() { - private int currentHeight = -1; - - public void onGlobalLayout() { - int viewHeight = view.getHeight(); - //int textHeight = title.getLineHeight(); - int paddingDp = 112; - float paddingPx = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, paddingDp, getResources().getDisplayMetrics()); - int newHeight = viewHeight - Math.round(paddingPx); - - if (newHeight != currentHeight) { - currentHeight = newHeight; - ViewGroup.LayoutParams params = spacer.getLayoutParams(); - params.height = newHeight; - spacer.setLayoutParams(params); - - scrollView.scrollTo(0, initialListTop); - } - - } - }; - view.getViewTreeObserver().addOnGlobalLayoutListener(layoutListener); - return view; } @Override diff --git a/commons/src/main/java/org/wikimedia/commons/media/MediaDetailPagerFragment.java b/commons/src/main/java/org/wikimedia/commons/media/MediaDetailPagerFragment.java index f47155cae..b0a2378b4 100644 --- a/commons/src/main/java/org/wikimedia/commons/media/MediaDetailPagerFragment.java +++ b/commons/src/main/java/org/wikimedia/commons/media/MediaDetailPagerFragment.java @@ -3,6 +3,7 @@ package org.wikimedia.commons.media; import android.app.DownloadManager; import android.content.*; import android.database.Cursor; +import android.database.DataSetObserver; import android.net.*; import android.os.*; import android.support.v4.app.Fragment; @@ -41,6 +42,8 @@ public class MediaDetailPagerFragment extends SherlockFragment implements ViewPa public Media getMediaAtPosition(int i); public int getTotalMediaCount(); public void notifyDatasetChanged(); + public void registerDataSetObserver(DataSetObserver observer); + public void unregisterDataSetObserver(DataSetObserver observer); } private class MediaDetailAdapter extends FragmentStatePagerAdapter { @@ -80,17 +83,19 @@ public class MediaDetailPagerFragment extends SherlockFragment implements ViewPa View view = inflater.inflate(R.layout.fragment_media_detail_pager, container, false); pager = (ViewPager) view.findViewById(R.id.mediaDetailsPager); pager.setOnPageChangeListener(this); - pager.setAdapter(new MediaDetailAdapter(getChildFragmentManager())); if(savedInstanceState != null) { final int pageNumber = savedInstanceState.getInt("current-page"); // Adapter doesn't seem to be loading immediately. // Dear God, please forgive us for our sins view.postDelayed(new Runnable() { public void run() { + pager.setAdapter(new MediaDetailAdapter(getChildFragmentManager())); pager.setCurrentItem(pageNumber, false); getSherlockActivity().supportInvalidateOptionsMenu(); } }, 100); + } else { + pager.setAdapter(new MediaDetailAdapter(getChildFragmentManager())); } return view; } diff --git a/commons/src/main/java/org/wikimedia/commons/upload/MultipleShareActivity.java b/commons/src/main/java/org/wikimedia/commons/upload/MultipleShareActivity.java index 57bdab1c8..e2aafc3f4 100644 --- a/commons/src/main/java/org/wikimedia/commons/upload/MultipleShareActivity.java +++ b/commons/src/main/java/org/wikimedia/commons/upload/MultipleShareActivity.java @@ -5,6 +5,7 @@ import java.util.concurrent.*; import android.app.*; import android.content.*; +import android.database.DataSetObserver; import android.net.*; import android.os.*; import android.support.v4.app.FragmentManager; @@ -61,6 +62,13 @@ public class MultipleShareActivity } } + public void registerDataSetObserver(DataSetObserver observer) { + // fixme implement me if needed + } + + public void unregisterDataSetObserver(DataSetObserver observer) { + // fixme implement me if needed + } public void onItemClick(AdapterView adapterView, View view, int index, long item) { showDetail(index);