Set a fixed layout size only if the container view has a WRAP_CONTENT layout mode.

Using the measureSpec from the onMeasure callback to decide whether to set a
fixed layout size or not has proven to be a bit unpredictable. Changing the
code to look at the LayoutParams instead.

BUG=None
TEST=AndroidWebViewTest
// android_webview-only, trybots were happy.
NOTRY=true

Review URL: https://codereview.chromium.org/35403002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@231815 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 96 insertions, 8 deletions

diff --git a/android_webview/java/src/org/chromium/android_webview/AwContents.java b/android_webview/java/src/org/chromium/android_webview/AwContents.java
index 1f7987f..7032b72 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContents.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContents.java
@@ -359,6 +359,12 @@ public class AwContents {
             if (mNativeAwContents == 0) return;
             nativeSetFixedLayoutSize(mNativeAwContents, widthDip, heightDip);
         }
+
+        @Override
+        public boolean isLayoutParamsHeightWrapContent() {
+            return mContainerView.getLayoutParams() != null &&
+                mContainerView.getLayoutParams().height == ViewGroup.LayoutParams.WRAP_CONTENT;
+        }
     }
 
     //--------------------------------------------------------------------------------------------
diff --git a/android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java b/android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java
index c3a9ec9..3b996a0 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java
@@ -58,6 +58,7 @@ public class AwLayoutSizer {
         void requestLayout();
         void setMeasuredDimension(int measuredWidth, int measuredHeight);
         void setFixedLayoutSize(int widthDip, int heightDip);
+        boolean isLayoutParamsHeightWrapContent();
     }
 
     /**
@@ -225,14 +226,34 @@ public class AwLayoutSizer {
     // call from onSizeChanged, since onSizeChanged won't fire if the view's physical size doesn't
     // change.
     private void updateFixedLayoutSize(int w, int h, float pageScaleFactor) {
-        // If the WebView's measuredDimension depends on the size of its contents (which is the
-        // case if any of the measurement modes are AT_MOST or UNSPECIFIED) the viewport size
-        // cannot be directly calculated from the size as that can result in the layout being
-        // unstable or unpredictable.
-        // If both the width and height are fixed (specified by the parent) then content size
+        boolean wrapContentForHeight = mDelegate.isLayoutParamsHeightWrapContent();
+        // If the WebView's size in the Android view system depends on the size of its contents then
+        // the viewport size cannot be directly calculated from the WebView's physical size as that
+        // can result in the layout being unstable (for example loading the following contents
+        //   <div style="height:150%">a</a>
+        // would cause the WebView to indefinitely attempt to increase its height by 50%).
+        // If both the width and height are fixed (specified by the parent View) then content size
         // changes will not cause subsequent layout passes and so we don't need to do anything
         // special.
-        if ((mWidthMeasurementIsFixed && mHeightMeasurementIsFixed) || pageScaleFactor == 0) {
+        // We assume the width is 'fixed' if the parent View specified an EXACT or an AT_MOST
+        // measureSpec for the width (in which case the AT_MOST upper bound is the width).
+        // That means that the WebView will ignore LayoutParams.width set to WRAP_CONTENT and will
+        // instead try to take up as much width as possible. This is necessary because it's not
+        // practical to do web layout without a set width.
+        // For height the behavior is different because for a given width it is possible to
+        // calculate the minimum height required to display all of the content. As such the WebView
+        // can size itself vertically to match the content height. Because certain container views
+        // (LinearLayout with a WRAP_CONTENT height, for example) can result in onMeasure calls with
+        // both EXACTLY and AT_MOST height measureSpecs it is not possible to infer the sizing
+        // policy for the whole subtree based on the parameters passed to the onMeasure call.
+        // For that reason the LayoutParams.height property of the WebView is used. This behaves
+        // more predictably and means that toggling the fixedLayoutSize mode (which can have
+        // significant impact on how the web contents is laid out) is a direct consequence of the
+        // developer's choice. The downside is that it could result in the Android layout being
+        // unstable if a parent of the WebView has a wrap_content height while the WebView itself
+        // has height set to match_parent. Unfortunately addressing this edge case is costly so it
+        // will have to stay as is (this is compatible with Classic behavior).
+        if ((mWidthMeasurementIsFixed && !wrapContentForHeight) || pageScaleFactor == 0) {
             setFixedLayoutSize(0, 0);
             return;
         }
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwLayoutSizerTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwLayoutSizerTest.java
index 5d8b56b..357e922 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwLayoutSizerTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwLayoutSizerTest.java
@@ -20,6 +20,7 @@ public class AwLayoutSizerTest extends InstrumentationTestCase {
         public int measuredHeight;
         public int fixedLayoutWidth;
         public int fixedLayoutHeight;
+        public boolean heightWrapContent;
 
         @Override
         public void requestLayout() {
@@ -38,6 +39,11 @@ public class AwLayoutSizerTest extends InstrumentationTestCase {
             fixedLayoutWidth = widthDip;
             fixedLayoutHeight = heightDip;
         }
+
+        @Override
+        public boolean isLayoutParamsHeightWrapContent() {
+            return heightWrapContent;
+        }
     }
 
     private static final int FIRST_CONTENT_WIDTH = 101;
@@ -382,7 +388,7 @@ public class AwLayoutSizerTest extends InstrumentationTestCase {
 
     @SmallTest
     @Feature({"AndroidWebView"})
-    public void testViewportWithWrapContentMeasureSpec() {
+    public void testViewportWithUnspecifiedMeasureSpec() {
         AwLayoutSizer layoutSizer = new AwLayoutSizer();
         LayoutSizerDelegate delegate = new LayoutSizerDelegate();
         layoutSizer.setDelegate(delegate);
@@ -391,7 +397,6 @@ public class AwLayoutSizerTest extends InstrumentationTestCase {
         final int pageScale = 2;
         final int dipAndPageScale = (int) (dipScale * pageScale);
 
-
         int contentWidth = 800;
         int contentHeight = 400;
         int atMostWidth = contentWidth * dipAndPageScale;
@@ -431,6 +436,41 @@ public class AwLayoutSizerTest extends InstrumentationTestCase {
 
     @SmallTest
     @Feature({"AndroidWebView"})
+    public void testViewportWithAtMostMeasureSpec() {
+        AwLayoutSizer layoutSizer = new AwLayoutSizer();
+        LayoutSizerDelegate delegate = new LayoutSizerDelegate();
+        delegate.heightWrapContent = true;
+        layoutSizer.setDelegate(delegate);
+
+        final float dipScale = 1.5f;
+        final int pageScale = 2;
+        final int dipAndPageScale = (int) (dipScale * pageScale);
+
+        int contentWidth = 800;
+        int contentHeight = 400;
+        int contentWidthPix = contentWidth * dipAndPageScale;
+        int contentHeightPix = contentHeight * dipAndPageScale;
+
+        layoutSizer.setDIPScale(dipScale);
+        layoutSizer.onContentSizeChanged(contentWidth, contentHeight);
+        layoutSizer.onPageScaleChanged(pageScale);
+        layoutSizer.onMeasure(MeasureSpec.makeMeasureSpec(contentWidthPix, MeasureSpec.EXACTLY),
+                MeasureSpec.makeMeasureSpec(contentHeightPix * 2, MeasureSpec.AT_MOST));
+
+        assertTrue(delegate.setMeasuredDimensionCalled);
+        int measuredWidth = delegate.measuredWidth & View.MEASURED_SIZE_MASK;
+        int measuredHeight = delegate.measuredHeight & View.MEASURED_SIZE_MASK;
+
+        int sizeWidth = measuredWidth;
+        int sizeHeight = measuredHeight;
+        layoutSizer.onSizeChanged(sizeWidth, sizeHeight, 0, 0);
+
+        assertEquals(contentWidth, delegate.fixedLayoutWidth);
+        assertEquals(AwLayoutSizer.FIXED_LAYOUT_HEIGHT, delegate.fixedLayoutHeight);
+    }
+
+    @SmallTest
+    @Feature({"AndroidWebView"})
     public void testFixedLayoutViewportGoesBackToZeroWithWrapContentMeasureSpec() {
         AwLayoutSizer layoutSizer = new AwLayoutSizer();
         LayoutSizerDelegate delegate = new LayoutSizerDelegate();
@@ -465,6 +505,7 @@ public class AwLayoutSizerTest extends InstrumentationTestCase {
     public void testFixedLayoutSizeUpdatedOnPageScaleChangeItNoLayoutRequest() {
         AwLayoutSizer layoutSizer = new AwLayoutSizer();
         LayoutSizerDelegate delegate = new LayoutSizerDelegate();
+        delegate.heightWrapContent = true;
         layoutSizer.setDelegate(delegate);
         layoutSizer.setDIPScale(DIP_SCALE);
 
@@ -514,4 +555,24 @@ public class AwLayoutSizerTest extends InstrumentationTestCase {
 
         assertEquals(fixedLayoutWidth * 2, delegate.fixedLayoutWidth);
     }
+
+    @SmallTest
+    @Feature({"AndroidWebView"})
+    public void testFixedLayoutSizeDoesNotDependOnMeasureSpec() {
+        AwLayoutSizer layoutSizer = new AwLayoutSizer();
+        LayoutSizerDelegate delegate = new LayoutSizerDelegate();
+        delegate.heightWrapContent = false;
+        layoutSizer.setDelegate(delegate);
+        layoutSizer.setDIPScale(DIP_SCALE);
+
+        layoutSizer.onContentSizeChanged(TOO_LARGE_CONTENT_SIZE, TOO_LARGE_CONTENT_SIZE);
+        layoutSizer.onPageScaleChanged(INITIAL_PAGE_SCALE);
+        layoutSizer.onMeasure(
+                MeasureSpec.makeMeasureSpec(AT_MOST_MEASURE_SIZE, MeasureSpec.AT_MOST),
+                MeasureSpec.makeMeasureSpec(AT_MOST_MEASURE_SIZE, MeasureSpec.AT_MOST));
+        layoutSizer.onSizeChanged(AT_MOST_MEASURE_SIZE, AT_MOST_MEASURE_SIZE, 0, 0);
+
+        assertEquals(0, delegate.fixedLayoutWidth);
+        assertEquals(0, delegate.fixedLayoutHeight);
+    }
 }