GridLayoutManager margin bug
am: de8a5de43b

Change-Id: Idefd953027c9aba9402be977b549d45956d634aa
diff --git a/v7/recyclerview/src/android/support/v7/widget/GridLayoutManager.java b/v7/recyclerview/src/android/support/v7/widget/GridLayoutManager.java
index f584834..ee00be3 100644
--- a/v7/recyclerview/src/android/support/v7/widget/GridLayoutManager.java
+++ b/v7/recyclerview/src/android/support/v7/widget/GridLayoutManager.java
@@ -562,29 +562,14 @@
                     addDisappearingView(view, 0);
                 }
             }
+            calculateItemDecorationsForChild(view, mDecorInsets);
 
-            final LayoutParams lp = (LayoutParams) view.getLayoutParams();
-            final int spec = getChildMeasureSpec(mCachedBorders[lp.mSpanIndex + lp.mSpanSize] -
-                    mCachedBorders[lp.mSpanIndex], otherDirSpecMode, 0,
-                    mOrientation == HORIZONTAL ? lp.height : lp.width,
-                    false);
-            final int mainSpec = getChildMeasureSpec(mOrientationHelper.getTotalSpace(),
-                    mOrientationHelper.getMode(), 0,
-                    mOrientation == VERTICAL ? lp.height : lp.width, true);
-            // Unless the child has MATCH_PARENT, measure it from its specs before adding insets.
-            if (mOrientation == VERTICAL) {
-                @SuppressWarnings("deprecation")
-                final boolean applyInsets = lp.height == ViewGroup.LayoutParams.FILL_PARENT;
-                measureChildWithDecorationsAndMargin(view, spec, mainSpec, applyInsets, false);
-            } else {
-                //noinspection deprecation
-                final boolean applyInsets = lp.width == ViewGroup.LayoutParams.FILL_PARENT;
-                measureChildWithDecorationsAndMargin(view, mainSpec, spec, applyInsets, false);
-            }
+            measureChild(view, otherDirSpecMode, false);
             final int size = mOrientationHelper.getDecoratedMeasurement(view);
             if (size > maxSize) {
                 maxSize = size;
             }
+            final LayoutParams lp = (LayoutParams) view.getLayoutParams();
             final float otherSize = 1f * mOrientationHelper.getDecoratedMeasurementInOther(view) /
                     lp.mSpanSize;
             if (otherSize > maxSizeInOther) {
@@ -598,40 +583,41 @@
             maxSize = 0;
             for (int i = 0; i < count; i++) {
                 View view = mSet[i];
-                final LayoutParams lp = (LayoutParams) view.getLayoutParams();
-                final int spec = getChildMeasureSpec(mCachedBorders[lp.mSpanIndex + lp.mSpanSize] -
-                                mCachedBorders[lp.mSpanIndex], View.MeasureSpec.EXACTLY, 0,
-                        mOrientation == HORIZONTAL ? lp.height : lp.width, false);
-                final int mainSpec = getChildMeasureSpec(mOrientationHelper.getTotalSpace(),
-                        mOrientationHelper.getMode(), 0,
-                        mOrientation == VERTICAL ? lp.height : lp.width, true);
-                if (mOrientation == VERTICAL) {
-                    measureChildWithDecorationsAndMargin(view, spec, mainSpec, false, true);
-                } else {
-                    measureChildWithDecorationsAndMargin(view, mainSpec, spec, false, true);
-                }
+                measureChild(view, View.MeasureSpec.EXACTLY, true);
                 final int size = mOrientationHelper.getDecoratedMeasurement(view);
                 if (size > maxSize) {
                     maxSize = size;
                 }
             }
         }
+
         // Views that did not measure the maxSize has to be re-measured
         // We will stop doing this once we introduce Gravity in the GLM layout params
-        final int maxMeasureSpec = View.MeasureSpec.makeMeasureSpec(maxSize,
-                View.MeasureSpec.EXACTLY);
         for (int i = 0; i < count; i ++) {
             final View view = mSet[i];
             if (mOrientationHelper.getDecoratedMeasurement(view) != maxSize) {
                 final LayoutParams lp = (LayoutParams) view.getLayoutParams();
-                final int spec = getChildMeasureSpec(mCachedBorders[lp.mSpanIndex + lp.mSpanSize]
-                                - mCachedBorders[lp.mSpanIndex], View.MeasureSpec.EXACTLY, 0,
-                        mOrientation == HORIZONTAL ? lp.height : lp.width, false);
+                final Rect decorInsets = lp.mDecorInsets;
+                final int verticalInsets = decorInsets.top + decorInsets.bottom
+                        + lp.topMargin + lp.bottomMargin;
+                final int horizontalInsets = decorInsets.left + decorInsets.right
+                        + lp.leftMargin + lp.rightMargin;
+                final int totalSpaceInOther = mCachedBorders[lp.mSpanIndex + lp.mSpanSize]
+                        - mCachedBorders[lp.mSpanIndex];
+                final int wSpec;
+                final int hSpec;
                 if (mOrientation == VERTICAL) {
-                    measureChildWithDecorationsAndMargin(view, spec, maxMeasureSpec, true, true);
+                    wSpec = getChildMeasureSpec(totalSpaceInOther, View.MeasureSpec.EXACTLY,
+                            horizontalInsets, lp.width, false);
+                    hSpec = View.MeasureSpec.makeMeasureSpec(maxSize - verticalInsets,
+                            View.MeasureSpec.EXACTLY);
                 } else {
-                    measureChildWithDecorationsAndMargin(view, maxMeasureSpec, spec, true, true);
+                    wSpec = View.MeasureSpec.makeMeasureSpec(maxSize - horizontalInsets,
+                            View.MeasureSpec.EXACTLY);
+                    hSpec = getChildMeasureSpec(totalSpaceInOther, View.MeasureSpec.EXACTLY,
+                            verticalInsets, lp.height, false);
                 }
+                measureChildWithDecorationsAndMargin(view, wSpec, hSpec, true);
             }
         }
 
@@ -689,6 +675,40 @@
     }
 
     /**
+     * Measures a child with currently known information. This is not necessarily the child's final
+     * measurement. (see fillChunk for details).
+     *
+     * @param view The child view to be measured
+     * @param otherDirParentSpecMode The RV measure spec that should be used in the secondary
+     *                               orientation
+     * @param alreadyMeasured True if we've already measured this view once
+     */
+    private void measureChild(View view, int otherDirParentSpecMode, boolean alreadyMeasured) {
+        final LayoutParams lp = (LayoutParams) view.getLayoutParams();
+        final Rect decorInsets = lp.mDecorInsets;
+        final int verticalInsets = decorInsets.top + decorInsets.bottom
+                + lp.topMargin + lp.bottomMargin;
+        final int horizontalInsets = decorInsets.left + decorInsets.right
+                + lp.leftMargin + lp.rightMargin;
+        final int availableSpaceInOther = mCachedBorders[lp.mSpanIndex + lp.mSpanSize] -
+                mCachedBorders[lp.mSpanIndex];
+        final int wSpec;
+        final int hSpec;
+        if (mOrientation == VERTICAL) {
+            wSpec = getChildMeasureSpec(availableSpaceInOther, otherDirParentSpecMode,
+                    horizontalInsets, lp.width, false);
+            hSpec = getChildMeasureSpec(mOrientationHelper.getTotalSpace(), getHeightMode(),
+                    verticalInsets, lp.height, true);
+        } else {
+            hSpec = getChildMeasureSpec(availableSpaceInOther, otherDirParentSpecMode,
+                    verticalInsets, lp.height, false);
+            wSpec = getChildMeasureSpec(mOrientationHelper.getTotalSpace(), getWidthMode(),
+                    horizontalInsets, lp.width, true);
+        }
+        measureChildWithDecorationsAndMargin(view, wSpec, hSpec, alreadyMeasured);
+    }
+
+    /**
      * This is called after laying out a row (if vertical) or a column (if horizontal) when the
      * RecyclerView does not have exact measurement specs.
      * <p>
@@ -705,17 +725,8 @@
     }
 
     private void measureChildWithDecorationsAndMargin(View child, int widthSpec, int heightSpec,
-            boolean capBothSpecs, boolean alreadyMeasured) {
-        calculateItemDecorationsForChild(child, mDecorInsets);
+            boolean alreadyMeasured) {
         RecyclerView.LayoutParams lp = (RecyclerView.LayoutParams) child.getLayoutParams();
-        if (capBothSpecs || mOrientation == VERTICAL) {
-            widthSpec = updateSpecWithExtra(widthSpec, lp.leftMargin + mDecorInsets.left,
-                    lp.rightMargin + mDecorInsets.right);
-        }
-        if (capBothSpecs || mOrientation == HORIZONTAL) {
-            heightSpec = updateSpecWithExtra(heightSpec, lp.topMargin + mDecorInsets.top,
-                    lp.bottomMargin + mDecorInsets.bottom);
-        }
         final boolean measure;
         if (alreadyMeasured) {
             measure = shouldReMeasureChild(child, widthSpec, heightSpec, lp);
@@ -725,7 +736,6 @@
         if (measure) {
             child.measure(widthSpec, heightSpec);
         }
-
     }
 
     private int updateSpecWithExtra(int spec, int startInset, int endInset) {
diff --git a/v7/recyclerview/src/android/support/v7/widget/RecyclerView.java b/v7/recyclerview/src/android/support/v7/widget/RecyclerView.java
index ce52b05..931a916 100644
--- a/v7/recyclerview/src/android/support/v7/widget/RecyclerView.java
+++ b/v7/recyclerview/src/android/support/v7/widget/RecyclerView.java
@@ -7876,11 +7876,11 @@
             int size = Math.max(0, parentSize - padding);
             int resultSize = 0;
             int resultMode = 0;
-            if (canScroll) {
-                if (childDimension >= 0) {
-                    resultSize = childDimension;
-                    resultMode = MeasureSpec.EXACTLY;
-                } else if (childDimension == LayoutParams.FILL_PARENT){
+            if (childDimension >= 0) {
+                resultSize = childDimension;
+                resultMode = MeasureSpec.EXACTLY;
+            } else if (canScroll) {
+                 if (childDimension == LayoutParams.FILL_PARENT){
                     switch (parentMode) {
                         case MeasureSpec.AT_MOST:
                         case MeasureSpec.EXACTLY:
@@ -7897,10 +7897,7 @@
                     resultMode = MeasureSpec.UNSPECIFIED;
                 }
             } else {
-                if (childDimension >= 0) {
-                    resultSize = childDimension;
-                    resultMode = MeasureSpec.EXACTLY;
-                } else if (childDimension == LayoutParams.FILL_PARENT) {
+                if (childDimension == LayoutParams.FILL_PARENT) {
                     resultSize = size;
                     resultMode = parentMode;
                 } else if (childDimension == LayoutParams.WRAP_CONTENT) {
diff --git a/v7/recyclerview/tests/src/android/support/v7/widget/BaseWrapContentTest.java b/v7/recyclerview/tests/src/android/support/v7/widget/BaseWrapContentTest.java
index 8f29750..281745c 100644
--- a/v7/recyclerview/tests/src/android/support/v7/widget/BaseWrapContentTest.java
+++ b/v7/recyclerview/tests/src/android/support/v7/widget/BaseWrapContentTest.java
@@ -208,8 +208,7 @@
         recyclerView.setAdapter(adapter);
         recyclerView.setLayoutParams(lp);
         Rect padding = mWrapContentConfig.padding;
-        recyclerView.setPadding(padding.left, padding.top,
-                padding.right, padding.bottom);
+        recyclerView.setPadding(padding.left, padding.top, padding.right, padding.bottom);
         setRecyclerView(recyclerView);
         recyclerView.waitUntilLayout();
         Snapshot snapshot = takeSnapshot();
diff --git a/v7/recyclerview/tests/src/android/support/v7/widget/BaseWrapContentWithAspectRatioTest.java b/v7/recyclerview/tests/src/android/support/v7/widget/BaseWrapContentWithAspectRatioTest.java
index f022eee..b91c37a 100644
--- a/v7/recyclerview/tests/src/android/support/v7/widget/BaseWrapContentWithAspectRatioTest.java
+++ b/v7/recyclerview/tests/src/android/support/v7/widget/BaseWrapContentWithAspectRatioTest.java
@@ -133,16 +133,24 @@
         int desiredW, desiredH;
         final long mId = idCounter.incrementAndGet();
 
-        ViewGroup.LayoutParams layoutParams;
+        ViewGroup.MarginLayoutParams layoutParams;
 
         public MeasureBehavior(int desiredW, int desiredH, int wMode, int hMode) {
             this.desiredW = desiredW;
             this.desiredH = desiredH;
-            layoutParams = new ViewGroup.LayoutParams(
+            layoutParams = new ViewGroup.MarginLayoutParams(
                     wMode, hMode
             );
         }
 
+        public MeasureBehavior withMargins(int left, int top, int right, int bottom) {
+            layoutParams.leftMargin = left;
+            layoutParams.topMargin = top;
+            layoutParams.rightMargin = right;
+            layoutParams.bottomMargin = bottom;
+            return this;
+        }
+
         public long getId() {
             return mId;
         }
diff --git a/v7/recyclerview/tests/src/android/support/v7/widget/GridLayoutManagerWrapContentTest.java b/v7/recyclerview/tests/src/android/support/v7/widget/GridLayoutManagerWrapContentTest.java
index a96b2f6..d074b07 100644
--- a/v7/recyclerview/tests/src/android/support/v7/widget/GridLayoutManagerWrapContentTest.java
+++ b/v7/recyclerview/tests/src/android/support/v7/widget/GridLayoutManagerWrapContentTest.java
@@ -40,6 +40,7 @@
     private boolean mHorizontal = false;
     private int mSpanCount = 3;
     private RecyclerView.ItemDecoration mItemDecoration;
+
     public GridLayoutManagerWrapContentTest(Rect padding) {
         super(new WrapContentConfig(false, false, padding));
     }
@@ -177,6 +178,36 @@
         layoutAndCheck(lp, adapter, expected, 60, 20);
     }
 
+    @Test
+    public void testVerticalWithHorizontalMargins() throws Throwable {
+        TestedFrameLayout.FullControlLayoutParams lp =
+                mWrapContentConfig.toLayoutParams(WRAP_CONTENT, WRAP_CONTENT);
+        WrapContentAdapter adapter = new WrapContentAdapter(
+                new MeasureBehavior(100, 50, 100, WRAP_CONTENT).withMargins(10, 0, 5, 0)
+        );
+        Rect[] expected = new Rect[] {
+                new Rect(0, 0, 115, 50)
+        };
+        layoutAndCheck(lp, adapter, expected, 345, 50);
+    }
+
+    @Test
+    public void testHorizontalWithHorizontalMargins() throws Throwable {
+        mHorizontal = true;
+        mSpanCount = 1;
+        TestedFrameLayout.FullControlLayoutParams lp =
+                mWrapContentConfig.toLayoutParams(WRAP_CONTENT, WRAP_CONTENT);
+        WrapContentAdapter adapter = new WrapContentAdapter(
+                new MeasureBehavior(100, 50, 100, WRAP_CONTENT).withMargins(10, 0, 5, 0),
+                new MeasureBehavior(100, 50, 100, WRAP_CONTENT).withMargins(3, 4, 5, 6)
+        );
+        Rect[] expected = new Rect[] {
+                new Rect(0, 0, 115, 50),
+                new Rect(115, 0, 223, 60)
+        };
+        layoutAndCheck(lp, adapter, expected, 223, 60);
+    }
+
     @Override
     protected int getVerticalGravity(RecyclerView.LayoutManager layoutManager) {
         return Gravity.TOP;