Skip to content

Commit

Permalink
Simplify android_webview wrap_contents mode.
Browse files Browse the repository at this point in the history 
This changes the webview implementation of wrap_contents mode
from custom code explicitly calculating the layout size to a
pair of settings: "ignore viewport tag" (for older apps) and
"set layout size height to 0".

This means this code gets simpler and means we don't have to
fix a bunch of races present in the current version of the
code.

BUG=392460, 398445
TBR=cevans@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#288992}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288992 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
mkosiba@chromium.org committed Aug 12, 2014
1 parent 2ef383d commit 9954ea9
Show file tree
Hide file tree
Showing 18 changed files with 251 additions and 272 deletions.
5 changes: 0 additions & 5 deletions android_webview/browser/renderer_host/aw_render_view_host_ext.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,6 @@ void AwRenderViewHostExt::SetTextZoomFactor(float factor) {
Send(new AwViewMsg_SetTextZoomFactor(web_contents()->GetRoutingID(), factor));
}

void AwRenderViewHostExt::SetFixedLayoutSize(const gfx::Size& size) {
DCHECK(CalledOnValidThread());
Send(new AwViewMsg_SetFixedLayoutSize(web_contents()->GetRoutingID(), size));
}

void AwRenderViewHostExt::ResetScrollAndScaleState() {
DCHECK(CalledOnValidThread());
Send(new AwViewMsg_ResetScrollAndScaleState(web_contents()->GetRoutingID()));
Expand Down
2 changes: 0 additions & 2 deletions android_webview/browser/renderer_host/aw_render_view_host_ext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ class AwRenderViewHostExt : public content::WebContentsObserver,
// Text Autosizing.
void SetTextZoomFactor(float factor);

void SetFixedLayoutSize(const gfx::Size& size);

void ResetScrollAndScaleState();

// Sets the initial page scale. This overrides initial scale set by
Expand Down
5 changes: 0 additions & 5 deletions android_webview/common/render_view_messages.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,6 @@ IPC_MESSAGE_ROUTED0(AwViewMsg_ResetScrollAndScaleState)
IPC_MESSAGE_ROUTED1(AwViewMsg_SetInitialPageScale,
double /* page_scale_factor */)

// Makes the blink::WebView use the given size for layout regardless of what
// the size of the RenderWidget or viewport settings are.
IPC_MESSAGE_ROUTED1(AwViewMsg_SetFixedLayoutSize,
gfx::Size /* size */)

// Sets the base background color for this view.
IPC_MESSAGE_ROUTED1(AwViewMsg_SetBackgroundColor,
SkColor)
Expand Down
13 changes: 5 additions & 8 deletions android_webview/java/src/org/chromium/android_webview/AwContents.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,17 +420,16 @@ public void setMeasuredDimension(int measuredWidth, int measuredHeight) {
mInternalAccessAdapter.setMeasuredDimension(measuredWidth, measuredHeight);
}

@Override
public void setFixedLayoutSize(int widthDip, int heightDip) {
if (mNativeAwContents == 0) return;
nativeSetFixedLayoutSize(mNativeAwContents, widthDip, heightDip);
}

@Override
public boolean isLayoutParamsHeightWrapContent() {
return mContainerView.getLayoutParams() != null &&
mContainerView.getLayoutParams().height == ViewGroup.LayoutParams.WRAP_CONTENT;
}

@Override
public void setForceZeroLayoutHeight(boolean forceZeroHeight) {
getSettings().setForceZeroLayoutHeight(forceZeroHeight);
}
}

//--------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -2471,8 +2470,6 @@ private native boolean nativeOnDraw(long nativeAwContents, Canvas canvas,
private native void nativeOnAttachedToWindow(long nativeAwContents, int w, int h);
private static native void nativeOnDetachedFromWindow(long nativeAwContents);
private native void nativeSetDipScale(long nativeAwContents, float dipScale);
private native void nativeSetFixedLayoutSize(long nativeAwContents,
int widthDip, int heightDip);

// Returns null if save state fails.
private native byte[] nativeGetOpaqueState(long nativeAwContents);
Expand Down
97 changes: 10 additions & 87 deletions android_webview/java/src/org/chromium/android_webview/AwLayoutSizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
* Helper methods used to manage the layout of the View that contains AwContents.
*/
public class AwLayoutSizer {
public static final int FIXED_LAYOUT_HEIGHT = 0;

// These are used to prevent a re-layout if the content size changes within a dimension that is
// fixed by the view system.
private boolean mWidthMeasurementIsFixed;
Expand All @@ -25,8 +23,6 @@ public class AwLayoutSizer {
// Page scale factor. This is set to zero initially so that we don't attempt to do a layout if
// we get the content size change notification first and a page scale change second.
private float mPageScaleFactor = 0.0f;
// The page scale factor that was used in the most recent onMeasure call.
private float mLastMeasuredPageScaleFactor = 0.0f;

// Whether to postpone layout requests.
private boolean mFreezeLayoutRequests;
Expand All @@ -40,22 +36,17 @@ public class AwLayoutSizer {
// If mHeightMeasurementLimited is true then this contains the height limit.
private int mHeightMeasurementLimit;

// The most recent width and height seen in onSizeChanged.
private int mLastWidth;
private int mLastHeight;

// Used to prevent sending multiple setFixedLayoutSize notifications with the same values.
private int mLastSentFixedLayoutSizeWidth = -1;
private int mLastSentFixedLayoutSizeHeight = -1;

// Callback object for interacting with the View.
private Delegate mDelegate;

/**
* Delegate interface through which the AwLayoutSizer communicates with the view it's sizing.
*/
public interface Delegate {
void requestLayout();
void setMeasuredDimension(int measuredWidth, int measuredHeight);
void setFixedLayoutSize(int widthDip, int heightDip);
boolean isLayoutParamsHeightWrapContent();
void setForceZeroLayoutHeight(boolean forceZeroHeight);
}

/**
Expand Down Expand Up @@ -136,10 +127,6 @@ private void doUpdate(int widthCss, int heightCss, float pageScaleFactor) {
} else {
mDelegate.requestLayout();
}
} else if (pageScaleChanged && mLastWidth != 0) {
// Because the fixed layout size is directly impacted by the pageScaleFactor we must
// update it even if the physical size of the view doesn't change.
updateFixedLayoutSize(mLastWidth, mLastHeight, mPageScaleFactor);
}
}

Expand All @@ -159,8 +146,6 @@ public void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
int measuredHeight = contentHeightPix;
int measuredWidth = contentWidthPix;

mLastMeasuredPageScaleFactor = mPageScaleFactor;

// Always use the given size unless unspecified. This matches WebViewClassic behavior.
mWidthMeasurementIsFixed = (widthMode != MeasureSpec.UNSPECIFIED);
mHeightMeasurementIsFixed = (heightMode == MeasureSpec.EXACTLY);
Expand Down Expand Up @@ -193,9 +178,7 @@ public void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
* changed.
*/
public void onSizeChanged(int w, int h, int ow, int oh) {
mLastWidth = w;
mLastHeight = h;
updateFixedLayoutSize(mLastWidth, mLastHeight, mLastMeasuredPageScaleFactor);
updateLayoutSettings();
}

/**
Expand All @@ -204,72 +187,12 @@ public void onSizeChanged(int w, int h, int ow, int oh) {
* This should be called after onSizeChanged regardless of whether the size has changed or not.
*/
public void onLayoutChange() {
updateFixedLayoutSize(mLastWidth, mLastHeight, mLastMeasuredPageScaleFactor);
updateLayoutSettings();
}

private void setFixedLayoutSize(int widthDip, int heightDip) {
if (widthDip == mLastSentFixedLayoutSizeWidth &&
heightDip == mLastSentFixedLayoutSizeHeight)
return;
mLastSentFixedLayoutSizeWidth = widthDip;
mLastSentFixedLayoutSizeHeight = heightDip;

mDelegate.setFixedLayoutSize(widthDip, heightDip);
}

// This needs to be called every time either the physical size of the view is changed or the
// pageScale is changed. Since we need to ensure that this is called immediately after
// onSizeChanged we can't just wait for onLayoutChange. At the same time we can't only make this
// 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) {
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.
// 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;
}

final double dipAndPageScale = pageScaleFactor * mDIPScale;
final int contentWidthPix = (int) (mContentWidthCss * dipAndPageScale);

int widthDip = (int) Math.ceil(w / dipAndPageScale);

// Make sure that we don't introduce rounding errors if the viewport is to be exactly as
// wide as the contents.
if (w == contentWidthPix) {
widthDip = mContentWidthCss;
}

// This is workaround due to the fact that in wrap content mode we need to use a fixed
// layout size independent of view height, otherwise things like <div style="height:120%">
// cause the webview to grow indefinitely. We need to use a height independent of the
// webview's height. 0 is the value used in WebViewClassic.
setFixedLayoutSize(widthDip, FIXED_LAYOUT_HEIGHT);
// This needs to be called every time either the physical size of the view is changed or layout
// params are updated.
private void updateLayoutSettings() {
mDelegate.setForceZeroLayoutHeight(mDelegate.isLayoutParamsHeightWrapContent());
}
}
44 changes: 44 additions & 0 deletions android_webview/java/src/org/chromium/android_webview/AwSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ public enum LayoutAlgorithm {
private boolean mDomStorageEnabled = false;
private boolean mDatabaseEnabled = false;
private boolean mUseWideViewport = false;
private boolean mZeroLayoutHeightDisablesViewportQuirk = false;
private boolean mForceZeroLayoutHeight = false;
private boolean mLoadWithOverviewMode = false;
private boolean mMediaPlaybackRequiresUserGesture = true;
private String mDefaultVideoPosterURL;
Expand Down Expand Up @@ -1227,6 +1229,48 @@ private boolean getUseWideViewportLocked() {
return mUseWideViewport;
}

public void setZeroLayoutHeightDisablesViewportQuirk(boolean enabled) {
synchronized (mAwSettingsLock) {
if (mZeroLayoutHeightDisablesViewportQuirk != enabled) {
mZeroLayoutHeightDisablesViewportQuirk = enabled;
mEventHandler.updateWebkitPreferencesLocked();
}
}
}

public boolean getZeroLayoutHeightDisablesViewportQuirk() {
synchronized (mAwSettingsLock) {
return getZeroLayoutHeightDisablesViewportQuirkLocked();
}
}

@CalledByNative
private boolean getZeroLayoutHeightDisablesViewportQuirkLocked() {
assert Thread.holdsLock(mAwSettingsLock);
return mZeroLayoutHeightDisablesViewportQuirk;
}

public void setForceZeroLayoutHeight(boolean enabled) {
synchronized (mAwSettingsLock) {
if (mForceZeroLayoutHeight != enabled) {
mForceZeroLayoutHeight = enabled;
mEventHandler.updateWebkitPreferencesLocked();
}
}
}

public boolean getForceZeroLayoutHeight() {
synchronized (mAwSettingsLock) {
return getForceZeroLayoutHeightLocked();
}
}

@CalledByNative
private boolean getForceZeroLayoutHeightLocked() {
assert Thread.holdsLock(mAwSettingsLock);
return mForceZeroLayoutHeight;
}

@CalledByNative
private boolean getPasswordEchoEnabledLocked() {
assert Thread.holdsLock(mAwSettingsLock);
Expand Down
5 changes: 1 addition & 4 deletions ...d_webview/javatests/src/org/chromium/android_webview/test/AndroidViewIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,10 +343,7 @@ public void testViewSizedCorrectlyInWrapContentModeWithDynamicContents() throws

final int expectedWidthCss =
(int) Math.ceil(getRootLayoutWidthOnMainThread() / deviceDIPScale);
final int expectedHeightCss = contentHeightCss +
// The second div in the contents is styled to have 150% of the viewport height, hence
// the 1.5.
(int) (AwLayoutSizer.FIXED_LAYOUT_HEIGHT * 1.5);
final int expectedHeightCss = contentHeightCss;

loadPageOfSizeAndWaitForSizeChange(testContainerView.getAwContents(),
mOnContentSizeChangedHelper, expectedWidthCss, contentHeightCss, true);
Expand Down
Loading

0 comments on commit 9954ea9

Please sign in to comment.