Commit 90ef8c2f authored by Scott Violet's avatar Scott Violet Committed by Commit Bot

[reland] weblayer: fixes strict mode issues for weblayer

This fixs a number of cases and enables strict mode in tests. Hopefully
that helps prevent any other access from being added.

The trickier cases are jacoco injects code that accesses the disk, and
LayoutInflater may trigger loading classes, which triggers disk
access.

The addition since first patch is SelectFileDialog may trigger reading
from disk. That file has been updated.

BUG=1059770,1062561
TEST=covered by tests now

Change-Id: Ie8262ae4e018055049115fcefc6a29bf97acbe9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2108603Reviewed-by: default avatarBo <boliu@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751465}
parent cc205c1f
...@@ -13,6 +13,7 @@ import android.view.WindowManager; ...@@ -13,6 +13,7 @@ import android.view.WindowManager;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import org.chromium.base.StrictModeContext;
import org.chromium.ui.modaldialog.DialogDismissalCause; import org.chromium.ui.modaldialog.DialogDismissalCause;
import org.chromium.ui.modaldialog.ModalDialogManager; import org.chromium.ui.modaldialog.ModalDialogManager;
import org.chromium.ui.modaldialog.ModalDialogProperties; import org.chromium.ui.modaldialog.ModalDialogProperties;
...@@ -46,6 +47,14 @@ public class AppModalPresenter extends ModalDialogManager.Presenter { ...@@ -46,6 +47,14 @@ public class AppModalPresenter extends ModalDialogManager.Presenter {
mContext = context; mContext = context;
} }
private ModalDialogView loadDialogView() {
// LayoutInflater may access the disk.
try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
return (ModalDialogView) LayoutInflater.from(mDialog.getContext())
.inflate(R.layout.modal_dialog_view, null);
}
}
@Override @Override
protected void addDialogView(PropertyModel model) { protected void addDialogView(PropertyModel model) {
int style = model.get(ModalDialogProperties.PRIMARY_BUTTON_FILLED) int style = model.get(ModalDialogProperties.PRIMARY_BUTTON_FILLED)
...@@ -57,11 +66,13 @@ public class AppModalPresenter extends ModalDialogManager.Presenter { ...@@ -57,11 +66,13 @@ public class AppModalPresenter extends ModalDialogManager.Presenter {
// Cancel on touch outside should be disabled by default. The ModelChangeProcessor wouldn't // Cancel on touch outside should be disabled by default. The ModelChangeProcessor wouldn't
// notify change if the property is not set during initialization. // notify change if the property is not set during initialization.
mDialog.setCanceledOnTouchOutside(false); mDialog.setCanceledOnTouchOutside(false);
ModalDialogView dialogView = (ModalDialogView) LayoutInflater.from(mDialog.getContext()) ModalDialogView dialogView = loadDialogView();
.inflate(R.layout.modal_dialog_view, null);
mModelChangeProcessor = mModelChangeProcessor =
PropertyModelChangeProcessor.create(model, dialogView, new ViewBinder()); PropertyModelChangeProcessor.create(model, dialogView, new ViewBinder());
mDialog.setContentView(dialogView); // setContentView() can trigger using LayoutInflater, which may read from disk.
try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
mDialog.setContentView(dialogView);
}
try { try {
mDialog.show(); mDialog.show();
......
...@@ -18,6 +18,7 @@ import androidx.annotation.NonNull; ...@@ -18,6 +18,7 @@ import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import androidx.core.view.ViewCompat; import androidx.core.view.ViewCompat;
import org.chromium.base.StrictModeContext;
import org.chromium.content_public.browser.SelectionPopupController; import org.chromium.content_public.browser.SelectionPopupController;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.interpolators.BakedBezierInterpolator; import org.chromium.ui.interpolators.BakedBezierInterpolator;
...@@ -105,6 +106,14 @@ public abstract class TabModalPresenter extends ModalDialogManager.Presenter { ...@@ -105,6 +106,14 @@ public abstract class TabModalPresenter extends ModalDialogManager.Presenter {
return mDialogContainer; return mDialogContainer;
} }
private ModalDialogView loadDialogView(int style) {
// LayoutInflater may access the disk.
try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
return (ModalDialogView) LayoutInflater.from(new ContextThemeWrapper(mContext, style))
.inflate(R.layout.modal_dialog_view, null);
}
}
@Override @Override
protected void addDialogView(PropertyModel model) { protected void addDialogView(PropertyModel model) {
if (mDialogContainer == null) mDialogContainer = createDialogContainer(); if (mDialogContainer == null) mDialogContainer = createDialogContainer();
...@@ -112,9 +121,7 @@ public abstract class TabModalPresenter extends ModalDialogManager.Presenter { ...@@ -112,9 +121,7 @@ public abstract class TabModalPresenter extends ModalDialogManager.Presenter {
int style = model.get(ModalDialogProperties.PRIMARY_BUTTON_FILLED) int style = model.get(ModalDialogProperties.PRIMARY_BUTTON_FILLED)
? R.style.Theme_Chromium_ModalDialog_FilledPrimaryButton ? R.style.Theme_Chromium_ModalDialog_FilledPrimaryButton
: R.style.Theme_Chromium_ModalDialog_TextPrimaryButton; : R.style.Theme_Chromium_ModalDialog_TextPrimaryButton;
mDialogView = mDialogView = loadDialogView(style);
(ModalDialogView) LayoutInflater.from(new ContextThemeWrapper(mContext, style))
.inflate(R.layout.modal_dialog_view, null);
mModelChangeProcessor = mModelChangeProcessor =
PropertyModelChangeProcessor.create(model, mDialogView, new ViewBinder()); PropertyModelChangeProcessor.create(model, mDialogView, new ViewBinder());
......
...@@ -7,6 +7,7 @@ package org.chromium.components.embedder_support.delegate; ...@@ -7,6 +7,7 @@ package org.chromium.components.embedder_support.delegate;
import android.content.Context; import android.content.Context;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.StrictModeContext;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
...@@ -37,7 +38,10 @@ public class ColorChooserAndroid { ...@@ -37,7 +38,10 @@ public class ColorChooserAndroid {
} }
private void openColorChooser() { private void openColorChooser() {
mDialog.show(); // This triggers LayoutInflater, which may access the disk.
try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
mDialog.show();
}
} }
@CalledByNative @CalledByNative
......
...@@ -13,6 +13,8 @@ import android.view.View; ...@@ -13,6 +13,8 @@ import android.view.View;
import android.widget.Button; import android.widget.Button;
import android.widget.TextView; import android.widget.TextView;
import org.chromium.base.StrictModeContext;
/** /**
* UI for the color chooser that shows on the Android platform as a result of * UI for the color chooser that shows on the Android platform as a result of
* &lt;input type=color &gt; form element. * &lt;input type=color &gt; form element.
...@@ -33,6 +35,15 @@ public class ColorPickerDialog extends AlertDialog implements OnColorChangedList ...@@ -33,6 +35,15 @@ public class ColorPickerDialog extends AlertDialog implements OnColorChangedList
private int mCurrentColor; private int mCurrentColor;
View inflateView(Context context, int id) {
// LayoutInflater may trigger accessing the disk.
try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
LayoutInflater inflater =
(LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
return inflater.inflate(id, null);
}
}
/** /**
* @param context The context the dialog is to run in. * @param context The context the dialog is to run in.
* @param listener The object to notify when the color is set. * @param listener The object to notify when the color is set.
...@@ -48,9 +59,7 @@ public class ColorPickerDialog extends AlertDialog implements OnColorChangedList ...@@ -48,9 +59,7 @@ public class ColorPickerDialog extends AlertDialog implements OnColorChangedList
mCurrentColor = mInitialColor; mCurrentColor = mInitialColor;
// Initialize title // Initialize title
LayoutInflater inflater = View title = inflateView(context, R.layout.color_picker_dialog_title);
(LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
View title = inflater.inflate(R.layout.color_picker_dialog_title, null);
setCustomTitle(title); setCustomTitle(title);
mCurrentColorView = title.findViewById(R.id.selected_color_view); mCurrentColorView = title.findViewById(R.id.selected_color_view);
...@@ -90,7 +99,7 @@ public class ColorPickerDialog extends AlertDialog implements OnColorChangedList ...@@ -90,7 +99,7 @@ public class ColorPickerDialog extends AlertDialog implements OnColorChangedList
}); });
// Initialize main content view // Initialize main content view
View content = inflater.inflate(R.layout.color_picker_dialog_content, null); View content = inflateView(context, R.layout.color_picker_dialog_content);
setView(content); setView(content);
// Initialize More button. // Initialize More button.
......
...@@ -11,6 +11,7 @@ import android.view.LayoutInflater; ...@@ -11,6 +11,7 @@ import android.view.LayoutInflater;
import androidx.annotation.StringRes; import androidx.annotation.StringRes;
import org.chromium.base.Log; import org.chromium.base.Log;
import org.chromium.base.StrictModeContext;
import org.chromium.ui.modaldialog.DialogDismissalCause; import org.chromium.ui.modaldialog.DialogDismissalCause;
import org.chromium.ui.modaldialog.ModalDialogManager; import org.chromium.ui.modaldialog.ModalDialogManager;
import org.chromium.ui.modaldialog.ModalDialogProperties; import org.chromium.ui.modaldialog.ModalDialogProperties;
...@@ -52,8 +53,11 @@ public abstract class JavascriptModalDialog implements ModalDialogProperties.Con ...@@ -52,8 +53,11 @@ public abstract class JavascriptModalDialog implements ModalDialogProperties.Con
protected void show(Context context, ModalDialogManager manager, protected void show(Context context, ModalDialogManager manager,
@ModalDialogManager.ModalDialogType int dialogType) { @ModalDialogManager.ModalDialogType int dialogType) {
assert manager != null; assert manager != null;
mDialogCustomView = (JavascriptDialogCustomView) LayoutInflater.from(context).inflate( // LayoutInflater may trigger accessing the disk.
R.layout.js_modal_dialog, null); try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
mDialogCustomView = (JavascriptDialogCustomView) LayoutInflater.from(context).inflate(
R.layout.js_modal_dialog, null);
}
mDialogCustomView.setPromptText(mDefaultPromptText); mDialogCustomView.setPromptText(mDefaultPromptText);
mDialogCustomView.setSuppressCheckBoxVisibility(mShouldShowSuppressCheckBox); mDialogCustomView.setSuppressCheckBoxVisibility(mShouldShowSuppressCheckBox);
......
...@@ -25,6 +25,7 @@ import org.chromium.base.ContentUriUtils; ...@@ -25,6 +25,7 @@ import org.chromium.base.ContentUriUtils;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.Log; import org.chromium.base.Log;
import org.chromium.base.PathUtils; import org.chromium.base.PathUtils;
import org.chromium.base.StrictModeContext;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.JNINamespace;
...@@ -461,9 +462,12 @@ public class SelectFileDialog implements WindowAndroid.IntentCallback, PhotoPick ...@@ -461,9 +462,12 @@ public class SelectFileDialog implements WindowAndroid.IntentCallback, PhotoPick
| Intent.FLAG_GRANT_WRITE_URI_PERMISSION); | Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
camera.putExtra(MediaStore.EXTRA_OUTPUT, mCameraOutputUri); camera.putExtra(MediaStore.EXTRA_OUTPUT, mCameraOutputUri);
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR2) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR2) {
camera.setClipData( // ClipData.newUri may access the disk (for reading mime types).
ClipData.newUri(ContextUtils.getApplicationContext().getContentResolver(), try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
UiUtils.IMAGE_FILE_PATH, mCameraOutputUri)); camera.setClipData(ClipData.newUri(
ContextUtils.getApplicationContext().getContentResolver(),
UiUtils.IMAGE_FILE_PATH, mCameraOutputUri));
}
} }
if (mDirectToCamera) { if (mDirectToCamera) {
mWindow.showIntent(camera, mCallback, R.string.low_memory_error); mWindow.showIntent(camera, mCallback, R.string.low_memory_error);
......
...@@ -405,17 +405,20 @@ public final class WebLayerImpl extends IWebLayer.Stub { ...@@ -405,17 +405,20 @@ public final class WebLayerImpl extends IWebLayer.Stub {
} }
private void loadNativeLibrary(String packageName) { private void loadNativeLibrary(String packageName) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { // Loading the library triggers disk access.
WebViewFactory.loadWebViewNativeLibraryFromPackage( try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
packageName, getClass().getClassLoader()); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
} else { WebViewFactory.loadWebViewNativeLibraryFromPackage(
try { packageName, getClass().getClassLoader());
Method loadNativeLibrary = } else {
WebViewFactory.class.getDeclaredMethod("loadNativeLibrary"); try {
loadNativeLibrary.setAccessible(true); Method loadNativeLibrary =
loadNativeLibrary.invoke(null); WebViewFactory.class.getDeclaredMethod("loadNativeLibrary");
} catch (ReflectiveOperationException e) { loadNativeLibrary.setAccessible(true);
Log.e(TAG, "Failed to load native library.", e); loadNativeLibrary.invoke(null);
} catch (ReflectiveOperationException e) {
Log.e(TAG, "Failed to load native library.", e);
}
} }
} }
} }
......
...@@ -10,6 +10,7 @@ import android.view.View; ...@@ -10,6 +10,7 @@ import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import android.widget.FrameLayout; import android.widget.FrameLayout;
import org.chromium.base.StrictModeContext;
import org.chromium.components.browser_ui.modaldialog.R; import org.chromium.components.browser_ui.modaldialog.R;
import org.chromium.components.browser_ui.modaldialog.TabModalPresenter; import org.chromium.components.browser_ui.modaldialog.TabModalPresenter;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
...@@ -43,10 +44,17 @@ public class WebLayerTabModalPresenter extends TabModalPresenter { ...@@ -43,10 +44,17 @@ public class WebLayerTabModalPresenter extends TabModalPresenter {
runEnterAnimation(); runEnterAnimation();
} }
private FrameLayout loadDialogContainer() {
// LayoutInflater may trigger accessing the disk.
try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
return (FrameLayout) LayoutInflater.from(mContext).inflate(
R.layout.modal_dialog_container, null);
}
}
@Override @Override
protected ViewGroup createDialogContainer() { protected ViewGroup createDialogContainer() {
FrameLayout dialogContainer = (FrameLayout) LayoutInflater.from(mContext).inflate( FrameLayout dialogContainer = loadDialogContainer();
R.layout.modal_dialog_container, null);
dialogContainer.setVisibility(View.GONE); dialogContainer.setVisibility(View.GONE);
dialogContainer.setClickable(true); dialogContainer.setClickable(true);
......
...@@ -8,6 +8,9 @@ import android.content.Context; ...@@ -8,6 +8,9 @@ import android.content.Context;
import android.content.Intent; import android.content.Intent;
import android.net.Uri; import android.net.Uri;
import android.os.Bundle; import android.os.Bundle;
import android.os.StrictMode;
import android.os.StrictMode.ThreadPolicy;
import android.os.StrictMode.VmPolicy;
import android.support.v4.app.Fragment; import android.support.v4.app.Fragment;
import android.support.v4.app.FragmentActivity; import android.support.v4.app.FragmentActivity;
import android.support.v4.app.FragmentManager; import android.support.v4.app.FragmentManager;
...@@ -58,6 +61,17 @@ public class InstrumentationActivity extends FragmentActivity { ...@@ -58,6 +61,17 @@ public class InstrumentationActivity extends FragmentActivity {
private Bundle mSavedInstanceState; private Bundle mSavedInstanceState;
private TabCallback mTabCallback; private TabCallback mTabCallback;
private static boolean isJaCoCoEnabled() {
// Nothing is set at runtime indicating jacoco is being used. This looks for the existence
// of a javacoco class to determine if jacoco is enabled.
try {
Class.forName("org.jacoco.agent.rt.RT");
return true;
} catch (LinkageError | ClassNotFoundException e) {
}
return false;
}
public Tab getTab() { public Tab getTab() {
return mTab; return mTab;
} }
...@@ -95,6 +109,19 @@ public class InstrumentationActivity extends FragmentActivity { ...@@ -95,6 +109,19 @@ public class InstrumentationActivity extends FragmentActivity {
@Override @Override
protected void onCreate(final Bundle savedInstanceState) { protected void onCreate(final Bundle savedInstanceState) {
// JaCoCo injects code that does file access, which doesn't work well with strict mode.
if (!isJaCoCoEnabled()) {
StrictMode.setThreadPolicy(
new ThreadPolicy.Builder().detectAll().penaltyLog().penaltyDeath().build());
// This doesn't use detectAll() as the untagged sockets policy is encountered in tests
// using TestServer.
StrictMode.setVmPolicy(new VmPolicy.Builder()
.detectLeakedSqlLiteObjects()
.detectLeakedClosableObjects()
.penaltyLog()
.penaltyDeath()
.build());
}
super.onCreate(savedInstanceState); super.onCreate(savedInstanceState);
mSavedInstanceState = savedInstanceState; mSavedInstanceState = savedInstanceState;
LinearLayout mainView = new LinearLayout(this); LinearLayout mainView = new LinearLayout(this);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment