Bug 982172 - Fix race condition with filter stack in panel adapter. r=lucasr a=lsblakk
authorJosh Dover <gerfuls@gmail.com>
Mon, 17 Mar 2014 09:06:17 -0700
changeset 192126 1fcf88c25bb8b8226555b109d3e1fd9d643fc542
parent 192125 3f1c2649de3517d577236a7a9afa3efdc762e89b
child 192127 d85cff6f6ade0101951ab59192b48e889479585d
push id3503
push userraliiev@mozilla.com
push dateMon, 28 Apr 2014 18:51:11 +0000
treeherdermozilla-beta@c95ac01e332e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslucasr, lsblakk
bugs982172
milestone30.0a2
Bug 982172 - Fix race condition with filter stack in panel adapter. r=lucasr a=lsblakk * * * Bug 982172 - Encapsulate DatasetRequest fields. r=lucasr * * * Bug 982172 - Add request type to delay filter stack changes until data loads. r=lucasr
mobile/android/base/home/DynamicPanel.java
mobile/android/base/home/PanelLayout.java
--- a/mobile/android/base/home/DynamicPanel.java
+++ b/mobile/android/base/home/DynamicPanel.java
@@ -170,17 +170,17 @@ public class DynamicPanel extends HomeFr
             if (!getCanLoadHint()) {
                 return;
             }
 
             final Bundle bundle = new Bundle();
             bundle.putParcelable(DATASET_REQUEST, request);
 
             // Ensure one loader per dataset
-            final int loaderId = generateLoaderId(request.datasetId);
+            final int loaderId = generateLoaderId(request.getDatasetId());
             getLoaderManager().restartLoader(loaderId, bundle, mLoaderCallbacks);
         }
 
         @Override
         public void resetDataset(String datasetId) {
             Log.d(LOGTAG, "Resetting dataset: " + datasetId);
 
             final LoaderManager lm = getLoaderManager();
@@ -213,22 +213,22 @@ public class DynamicPanel extends HomeFr
         @Override
         public Cursor loadCursor() {
             final ContentResolver cr = getContext().getContentResolver();
 
             final String selection;
             final String[] selectionArgs;
 
             // Null represents the root filter
-            if (mRequest.filter == null) {
+            if (mRequest.getFilter() == null) {
                 selection = DBUtils.concatenateWhere(HomeItems.DATASET_ID + " = ?", HomeItems.FILTER + " IS NULL");
-                selectionArgs = new String[] { mRequest.datasetId };
+                selectionArgs = new String[] { mRequest.getDatasetId() };
             } else {
                 selection = DBUtils.concatenateWhere(HomeItems.DATASET_ID + " = ?", HomeItems.FILTER + " = ?");
-                selectionArgs = new String[] { mRequest.datasetId, mRequest.filter };
+                selectionArgs = new String[] { mRequest.getDatasetId(), mRequest.getFilter() };
             }
 
             // XXX: You can use CONTENT_FAKE_URI for development to pull items from fake_home_items.json.
             return cr.query(HomeItems.CONTENT_URI, null, selection, selectionArgs, null);
         }
     }
 
     /**
@@ -251,17 +251,17 @@ public class DynamicPanel extends HomeFr
             mLayout.deliverDataset(request, cursor);
         }
 
         @Override
         public void onLoaderReset(Loader<Cursor> loader) {
             final DatasetRequest request = getRequestFromLoader(loader);
             Log.d(LOGTAG, "Resetting loader for request: " + request);
             if (mLayout != null) {
-                mLayout.releaseDataset(request.datasetId);
+                mLayout.releaseDataset(request.getDatasetId());
             }
         }
 
         private DatasetRequest getRequestFromLoader(Loader<Cursor> loader) {
             final PanelDatasetLoader datasetLoader = (PanelDatasetLoader) loader;
             return datasetLoader.getRequest();
         }
     }
--- a/mobile/android/base/home/PanelLayout.java
+++ b/mobile/android/base/home/PanelLayout.java
@@ -79,42 +79,94 @@ abstract class PanelLayout extends Frame
         public void setFilterManager(FilterManager manager);
     }
 
     /**
      * To be used by requests made to {@code DatasetHandler}s to couple dataset ID with current
      * filter for queries on the database.
      */
     public static class DatasetRequest implements Parcelable {
-        public final String datasetId;
-        public final String filter;
+        public enum Type implements Parcelable {
+            DATASET_LOAD,
+            FILTER_PUSH,
+            FILTER_POP;
+
+            @Override
+            public int describeContents() {
+                return 0;
+            }
+
+            @Override
+            public void writeToParcel(Parcel dest, int flags) {
+                dest.writeInt(ordinal());
+            }
+
+            public static final Creator<Type> CREATOR = new Creator<Type>() {
+                @Override
+                public Type createFromParcel(final Parcel source) {
+                    return Type.values()[source.readInt()];
+                }
+
+                @Override
+                public Type[] newArray(final int size) {
+                    return new Type[size];
+                }
+            };
+        }
+
+        private final Type mType;
+        private final String mDatasetId;
+        private final FilterDetail mFilterDetail;
 
         private DatasetRequest(Parcel in) {
-            this.datasetId = in.readString();
-            this.filter = in.readString();
+            this.mType = (Type) in.readParcelable(getClass().getClassLoader());
+            this.mDatasetId = in.readString();
+            this.mFilterDetail = (FilterDetail) in.readParcelable(getClass().getClassLoader());
+        }
+
+        public DatasetRequest(String datasetId, FilterDetail filterDetail) {
+            this(Type.DATASET_LOAD, datasetId, filterDetail);
+        }
+
+        public DatasetRequest(Type type, String datasetId, FilterDetail filterDetail) {
+            this.mType = type;
+            this.mDatasetId = datasetId;
+            this.mFilterDetail = filterDetail;
         }
 
-        public DatasetRequest(String datasetId, String filter) {
-            this.datasetId = datasetId;
-            this.filter = filter;
+        public Type getType() {
+            return mType;
+        }
+
+        public String getDatasetId() {
+            return mDatasetId;
+        }
+
+        public String getFilter() {
+            return (mFilterDetail != null ? mFilterDetail.filter : null);
+        }
+
+        public FilterDetail getFilterDetail() {
+            return mFilterDetail;
         }
 
         @Override
         public int describeContents() {
             return 0;
         }
 
         @Override
         public void writeToParcel(Parcel dest, int flags) {
-            dest.writeString(datasetId);
-            dest.writeString(filter);
+            dest.writeParcelable(mType, 0);
+            dest.writeString(mDatasetId);
+            dest.writeParcelable(mFilterDetail, 0);
         }
 
         public String toString() {
-            return "{dataset: " + datasetId + ", filter: " + filter + "}";
+            return "{type: " + mType + " dataset: " + mDatasetId + ", filter: " + mFilterDetail + "}";
         }
 
         public static final Creator<DatasetRequest> CREATOR = new Creator<DatasetRequest>() {
             public DatasetRequest createFromParcel(Parcel in) {
                 return new DatasetRequest(in);
             }
 
             public DatasetRequest[] newArray(int size) {
@@ -163,26 +215,26 @@ abstract class PanelLayout extends Frame
 
     /**
      * Delivers the dataset as a {@code Cursor} to be bound to the
      * panel views backed by it. This is used by the {@code DatasetHandler}
      * in response to a dataset request.
      */
     public final void deliverDataset(DatasetRequest request, Cursor cursor) {
         Log.d(LOGTAG, "Delivering request: " + request);
-        updateViewsWithDataset(request.datasetId, cursor);
+        updateViewsFromRequest(request, cursor);
     }
 
     /**
      * Releases any references to the given dataset from all
      * existing panel views.
      */
     public final void releaseDataset(String datasetId) {
-        Log.d(LOGTAG, "Resetting dataset: " + datasetId);
-        updateViewsWithDataset(datasetId, null);
+        Log.d(LOGTAG, "Releasing dataset: " + datasetId);
+        releaseViewsWithDataset(datasetId);
     }
 
     /**
      * Requests a dataset to be loaded and bound to any existing
      * panel view backed by it.
      */
     protected final void requestDataset(DatasetRequest request) {
         Log.d(LOGTAG, "Requesting request: " + request);
@@ -248,24 +300,45 @@ abstract class PanelLayout extends Frame
             // if it's backed by a dataset.
             maybeSetDataset(view, null);
 
             // Remove the view entry from the map
             mViewStateMap.remove(view);
         }
     }
 
-    private void updateViewsWithDataset(String datasetId, Cursor cursor) {
+    private void updateViewsFromRequest(DatasetRequest request, Cursor cursor) {
         for (Map.Entry<View, ViewState> entry : mViewStateMap.entrySet()) {
             final ViewState detail = entry.getValue();
 
             // Update any views associated with the given dataset ID
+            if (TextUtils.equals(detail.getDatasetId(), request.getDatasetId())) {
+                switch (request.getType()) {
+                    case FILTER_PUSH:
+                        detail.pushFilter(request.getFilterDetail());
+                        break;
+                    case FILTER_POP:
+                        detail.popFilter();
+                        break;
+                }
+
+                final View view = entry.getKey();
+                maybeSetDataset(view, cursor);
+            }
+        }
+    }
+
+    private void releaseViewsWithDataset(String datasetId) {
+        for (Map.Entry<View, ViewState> entry : mViewStateMap.entrySet()) {
+            final ViewState detail = entry.getValue();
+
+            // Release the cursor on views associated with the given dataset ID
             if (TextUtils.equals(detail.getDatasetId(), datasetId)) {
                 final View view = entry.getKey();
-                maybeSetDataset(view, cursor);
+                maybeSetDataset(view, null);
             }
         }
     }
 
     private void maybeSetDataset(View view, Cursor cursor) {
         if (view instanceof DatasetBacked) {
             final DatasetBacked dsb = (DatasetBacked) view;
             dsb.setDataset(cursor);
@@ -350,49 +423,72 @@ abstract class PanelLayout extends Frame
             return true;
         }
 
         public boolean canPopFilter() {
             return (mFilterStack != null && mFilterStack.size() > 1);
         }
     }
 
-    static class FilterDetail {
+    static class FilterDetail implements Parcelable {
         final String filter;
         final String title;
 
+        private FilterDetail(Parcel in) {
+            this.filter = in.readString();
+            this.title = in.readString();
+        }
+
         public FilterDetail(String filter, String title) {
             this.filter = filter;
             this.title = title;
         }
+
+        @Override
+        public int describeContents() {
+            return 0;
+        }
+
+        @Override
+        public void writeToParcel(Parcel dest, int flags) {
+            dest.writeString(filter);
+            dest.writeString(title);
+        }
+
+        public static final Creator<FilterDetail> CREATOR = new Creator<FilterDetail>() {
+            public FilterDetail createFromParcel(Parcel in) {
+                return new FilterDetail(in);
+            }
+
+            public FilterDetail[] newArray(int size) {
+                return new FilterDetail[size];
+            }
+        };
     }
 
     /**
      * Pushes filter to {@code ViewState}'s stack and makes request for new filter value.
      */
     private void pushFilterOnView(ViewState viewState, FilterDetail filterDetail) {
-        viewState.pushFilter(filterDetail);
-
-        final String filter = filterDetail.filter;
         final String datasetId = viewState.getDatasetId();
-        mDatasetHandler.requestDataset(new DatasetRequest(datasetId, filter));
+        mDatasetHandler.requestDataset(
+            new DatasetRequest(DatasetRequest.Type.FILTER_PUSH, datasetId, filterDetail));
     }
 
     /**
      * Pops filter from {@code ViewState}'s stack and makes request for previous filter value.
      *
      * @return whether the filter has changed
      */
     private boolean popFilterOnView(ViewState viewState) {
-        if (viewState.popFilter()) {
-            final FilterDetail current = viewState.getCurrentFilter();
-
-            final String filter = (current == null ? null : current.filter);
+        if (viewState.canPopFilter()) {
+            final FilterDetail filterDetail = viewState.getPreviousFilter();
             final String datasetId = viewState.getDatasetId();
-            mDatasetHandler.requestDataset(new DatasetRequest(datasetId, filter));
+            mDatasetHandler.requestDataset(
+                new DatasetRequest(DatasetRequest.Type.FILTER_POP, datasetId, filterDetail));
             return true;
         } else {
             return false;
         }
     }
 
     public interface OnItemOpenListener {
         public void onItemOpen(String url, String title);