ActivityTestRule leaks activity after orientation change [37065965]

Fixed

Bug

Status Update

No update yet.

Description

ja...@gmail.com

created issue #1

Oct 13, 2015 02:25PM

Library used (Espresso, Runner, Rules etc.): espresso-core, espresso-contrib, runner, rules
Version used: 0.4.1

What steps will reproduce the problem?
0. enable strict mode (StrictMode.setVmPolicy(new StrictMode.VmPolicy.Builder().detectAll().penaltyLog().penaltyDeath().build());)
1. run a test using ActivityTestRule
2. do an orientation change during test

How are you running your tests (via Android Studio, Gradle, adb, etc.)?
android studio and gradle

What is the expected output? What do you see instead?
the problem is that the ActivityTestRule keeps a reference to the first activity and after orientation change a new one is created. this causes a penalty for violation of VmPolicies.

Screen Shot 2015-10-13 at 15.23.01.png

108 KB

View

Download

Comments

ma...@gmail.com <ma...@gmail.com> #2Dec 29, 2015 10:01AM

Which version of R8 are you using?

ma...@gmail.com <ma...@gmail.com> #3Dec 29, 2015 10:05AM

I have tried the newest commit of main branch, it also has the similar issue.

nk...@google.com <nk...@google.com> #4Jan 5, 2016 01:16AM

In order to diagnose this we would probably need to be able to reproduce this issue.

Would it be possible for you to share a compiler dump file that contains the inputs to R8 (i.e., the Java class files and the keep rules going into R8)? That way we should be able to reproduce the build. You can generate this as below and could share this privately with christofferqa@google.com and sgjesse@google.com.

./gradlew assembleRelease --no-daemon -Dcom.android.tools.r8.dumpinputtodirectory=/path/to/dumps

Do you know if the class ViewEventSinkImpl is instantiated using reflection? If so, do you know if there is a corresponding keep rule for the class?

Can you try if the following rule "fixes" the issue:

-keep,allowobfuscation,allowshrinking class org.chromium.content.browser.ViewEventSinkImpl

When this rule is present, R8 should account for the fact that ViewEventSinkImpl may be instantiated by reflection, which I suspect may fix the issue you are seeing.

sl...@google.com <sl...@google.com> Mar 28, 2018 06:40PM

Assigned to nk...@google.com.

nk...@google.com <nk...@google.com> #5Mar 28, 2018 09:49PM

Marked as fixed.

Hi! I have also encountered a problem with r8 in --classfile mode incorrectly removing a null check, and curiously enough it also happened in and around ViewEventSinkImpl. I tried with the R8 version in Chromium and also with latest main branch local build from the repo which was be29229a110ce06989f6f3a63b0a83015ed3fefe.

The issue I'm seeing is from code which effectively does

public final class ViewEventSinkImpl ... {
    private @Nullable Boolean mHasViewFocus;  // not written to anywhere else

    public void onViewFocusChanged(boolean gainFocus) {
        if (mHasViewFocus != null && mHasViewFocus == gainFocus) return;
        mHasViewFocus = gainFocus;
        ....
    }
}

and after r8 with --classfile, the null check gets removed and the code is guaranteed to crash. Without --classfile, the null check remains in the dex. Note: for me, the code (with or without the check) was inlined at the only callsite to onViewFocusChanged.

Adding

-keep,allowobfuscation,allowshrinking class org.chromium.content.browser.ViewEventSinkImpl

works around the issue for me. I believe there is some reflection going on as one part of the code that affects that bug is the presence of

    public static ViewEventSinkImpl from(WebContents webContents) {
        ViewEventSinkImpl ret =
                ((WebContentsImpl) webContents)
                        .getOrSetUserData(
                                ViewEventSinkImpl.class, UserDataFactoryLazyHolder.INSTANCE);
        return ret;
    }

I hope the advice to send the repro case directly is still valid, I should have it down to a manageable size soon.

Issue 37065965