New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 922638 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 16
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
DFM
Proj-VR
Proj-XR
Proj-XR-VR

Blocking:
issue 914494



Sign in to add a comment

Monochrome crashes when entering VR with VR module

Project Member Reported by tiborg@chromium.org, Jan 16 (6 days ago)

Issue description

When entering VR on Monochrome with the VR module enabled I see the following crash:

01-16 19:45:16.632: E/AndroidRuntime(28271): FATAL EXCEPTION: main
01-16 19:45:16.632: E/AndroidRuntime(28271): Process: org.chromium.chrome, PID: 28271
01-16 19:45:16.632: E/AndroidRuntime(28271): java.lang.NoClassDefFoundError: Failed resolution of: Lorg/chromium/content_public/browser/ImeAdapter$-CC;
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.chrome.browser.vr.VrShell.configWebContentsImeForVr(VrShell.java:484)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.chrome.browser.vr.VrShell.initializeTabForVR(VrShell.java:507)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.chrome.browser.vr.VrShell.swapToTab(VrShell.java:474)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.chrome.browser.vr.VrShell.initializeNative(VrShell.java:427)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.chrome.browser.vr.VrShellDelegate.enterVr(VrShellDelegate.java:1149)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.chrome.browser.vr.VrShellDelegate.enterVrAfterDon(VrShellDelegate.java:1107)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.chrome.browser.vr.VrShellDelegate.handleDonFlowSuccess(VrShellDelegate.java:1548)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.chrome.browser.vr.VrShellDelegate.onResume(VrShellDelegate.java:1516)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.chrome.browser.vr.VrShellDelegate.onActivityStateChange(VrShellDelegate.java:975)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.chrome.browser.vr.VrShellDelegate$VrLifecycleObserver.onActivityStateChange(VrShellDelegate.java:220)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.base.ApplicationStatus.onStateChange(ApplicationStatus.java:351)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.base.ApplicationStatus.access$200(ApplicationStatus.java:33)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.base.ApplicationStatus$2.onActivityResumed(ApplicationStatus.java:262)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.app.Application.dispatchActivityResumed(Application.java:239)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.app.Activity.onResume(Activity.java:1343)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.support.v4.app.FragmentActivity.onResume(FragmentActivity.java:458)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at org.chromium.chrome.browser.init.AsyncInitializationActivity.onResume(AsyncInitializationActivity.java:456)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.app.Instrumentation.callActivityOnResume(Instrumentation.java:1412)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.app.Activity.performResume(Activity.java:7300)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.app.ActivityThread.performResumeActivity(ActivityThread.java:3777)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.app.ActivityThread.handleResumeActivity(ActivityThread.java:3817)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.app.servertransaction.ResumeActivityItem.execute(ResumeActivityItem.java:51)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.app.servertransaction.TransactionExecutor.executeLifecycleState(TransactionExecutor.java:145)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:70)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1809)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.os.Handler.dispatchMessage(Handler.java:106)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.os.Looper.loop(Looper.java:193)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at android.app.ActivityThread.main(ActivityThread.java:6680)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at java.lang.reflect.Method.invoke(Native Method)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
01-16 19:45:16.632: E/AndroidRuntime(28271): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

When VR is compiled into the base module this does not happen.
 

Comment 1 by tiborg@chromium.org, Jan 16 (6 days ago)

Cc: wnwen@chromium.org
Owner: tiborg@chromium.org
Status: Assigned (was: Available)
Per agrieve@'s suggestion 856c6214bf1fbd7730a7ef609720470f2999befc is the culprit. This CL may have just revealed a bug in dexsplitter. Not sure.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 16 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0807a3f79fd0572ad73f127111d7626262c1e655

commit 0807a3f79fd0572ad73f127111d7626262c1e655
Author: Tibor Goldschwendt <tiborg@chromium.org>
Date: Wed Jan 16 23:30:00 2019

Revert "Reland "Android: Replace desugar with d8 and r8 for debug""

This reverts commit 856c6214bf1fbd7730a7ef609720470f2999befc.

Reason for revert: breaks modularization (crbug/922638)

Original change's description:
> Reland "Android: Replace desugar with d8 and r8 for debug"
> 
> Original CL: https://chromium-review.googlesource.com/c/chromium/src/+/1384945
> 
> Fixes:
>  - Use the new r8 flags
>  - Use java_runtime_classpath when javac_full_interface_classpath is not
>    available.
> 
> Bug: 906803, 913679, 916733
> Change-Id: Iedf88ac5b0b9a9277a97d3cbcad24ee44eb61607
> Reviewed-on: https://chromium-review.googlesource.com/c/1393466
> Reviewed-by: Sam Maier <smaier@chromium.org>
> Commit-Queue: Peter Wen <wnwen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#619498}

TBR=wnwen@chromium.org,smaier@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 906803, 913679, 916733,  922638 
Change-Id: I28bdeec6c482cdd7a663a69cb6475f2fa25bd1f8
Reviewed-on: https://chromium-review.googlesource.com/c/1415793
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623446}
[modify] https://crrev.com/0807a3f79fd0572ad73f127111d7626262c1e655/build/android/gyp/dex.py
[modify] https://crrev.com/0807a3f79fd0572ad73f127111d7626262c1e655/build/android/gyp/proguard.py
[modify] https://crrev.com/0807a3f79fd0572ad73f127111d7626262c1e655/build/config/android/internal_rules.gni

Comment 3 by tiborg@chromium.org, Jan 16 (6 days ago)

Status: Fixed (was: Assigned)

Comment 4 by wnwen@chromium.org, Jan 17 (5 days ago)

Hi Tibor, would you mind posting repro steps to this bug?

Comment 5 by tiborg@chromium.org, Jan 17 (5 days ago)

For sure:

1. Patch in https://chromium-review.googlesource.com/c/chromium/src/+/1415761
2. Build debug monochrome_public_bundle
3. Install via out/Debug/bin/monochrome_public_bundle -m base -m vr
4. Enable flag #webxr
5. Go to https://klausw.github.io/webxr-samples/xr-presentation.html
6. Tap enter VR

Comment 6 by smaier@chromium.org, Jan 17 (5 days ago)

I am able to minimally repro here (it looks like it's a desugaring bug):

test/InterfaceWithStaticMethod.java:
package test;

public interface InterfaceWithStaticMethod {
    int b();
    static boolean c() {
        return true;
    }
}

test2/FeatureModule.java:
package test2;

import test.InterfaceWithStaticMethod;

public class FeatureModule {
    boolean a() {
        return InterfaceWithStaticMethod.c();
    }
}


Then I run:
javac -cp . test/InterfaceWithStaticMethod.java
jar cfv test/InterfaceWithStaticMethod.jar test/InterfaceWithStaticMethod.class
javac -classpath . test2/FeatureModule.java
jar cfv test2/FeatureModule.jar test2/FeatureModule.class
java -jar /usr/local/google/code/clankium/src/third_party/r8/lib/d8.jar --classpath test/InterfaceWithStaticMethod.jar --output test2/ test2/FeatureModule.jar
dexdump -d test2/classes.dex > test2/classes.dexdump
cat test2/classes.dexdump
...
000104:                                        |[000104] test2.FeatureModule.a:()Z
000114: 7100 0100 0000                         |0000: invoke-static {}, Ltest/InterfaceWithStaticMethod$-CC;.c:()Z // method@0001
00011a: 0a00                                   |0003: move-result v0
...

However, when I messed up the classpath in an earlier attempt, and D8 output the following message: 
Warning in /tmp/test2/FeatureModule.jar:tmp/test2/FeatureModule.class:
  Type `test.InterfaceWithStaticMethod` was not found, it is required for default or static interface methods desugaring of `boolean test2.FeatureModule.a()`

Then the dexdump looked perfectly normal (it called the c() function, no "-CC" anywhere)

Comment 7 Deleted

Comment 8 by smaier@chromium.org, Jan 17 (5 days ago)

Filed a bug on D8 to see what's going on https://issuetracker.google.com/123025513

Comment 9 by ricow@google.com, Jan 18 (4 days ago)

re #5
when I run:
$ out/Debug/bin/monochrome_public_bundle -m base -m vr
I get
usage: monochrome_public_bundle [-h]
                                {devices,install,uninstall,launch,stop,clear-data,argv,gdb,logcat,ps,disk-usage,mem-usage,shell,compile-dex,profile,run,build-bundle-apks}
                                ...
monochrome_public_bundle: error: invalid choice: 'base' (choose from 'devices', 'install', 'uninstall', 'launch', 'stop', 'clear-data', 'argv', 'gdb', 'logcat', 'ps', 'disk-usage', 'mem-usage', 'shell', 'compile-dex', 'profile', 'run', 'build-bundle-apks')

Secondly, how do I enable the #webxr flag?

Comment 10 by ricow@google.com, Jan 18 (4 days ago)

Cc: ricow@chromium.org

Comment 11 by ricow@google.com, Jan 18 (4 days ago)

Cc: r8-team+bugs@google.com

Comment 12 by tiborg@chromium.org, Jan 18 (4 days ago)

Oh, sorry, the command should actually be

$ out/Debug/bin/monochrome_public_bundle install -m base -m vr

To enable WebXR: Navigate to chrome://flags in Chromium. Search for WebXR. Enable "WebXR Device API".

Comment 13 by smaier@chromium.org, Jan 18 (4 days ago)

After the R8 team's comments in the https://issuetracker.google.com/123025513 we investigated this further. It turns out our base module was being compiled with min-api of 24, however our feature module was being compiled with a min-api lower than 24. Since API level 24 no longer requires this to be desugared, it would make sense that this desugaring doesn't happen in the base module. This looks like it's our problem, D8 is working fine.
Project Member

Comment 14 by bugdroid1@chromium.org, Jan 18 (4 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7bbff9a81d2d9344bcf45b9a9f2ba6aebb455a78

commit 7bbff9a81d2d9344bcf45b9a9f2ba6aebb455a78
Author: Sam Maier <smaier@chromium.org>
Date: Fri Jan 18 20:53:29 2019

Android Build: Make debug dexing take dex files instead of jar files

This also has a nice side effect - the build for multidex dexing speeds up
pretty nicely. I tested for monochrome_apk:
rm gen/clank/java/monochrome_apk/classes.dex.zip && \
  time python ../../build/android/gyp/dex.py ...

OLD:
real	0m26.702s
user	3m47.861s
sys	0m4.234s

NEW:
real	0m8.124s
user	0m27.175s
sys	0m2.435s

This also fixes the issue with D8 desugaring breaking the VR module.

Bug:  922638 
Change-Id: Ief54c3c6b99d4dbbf8c067ee2825d23aa1c4c87d
Reviewed-on: https://chromium-review.googlesource.com/c/1421860
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Auto-Submit: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624273}
[modify] https://crrev.com/7bbff9a81d2d9344bcf45b9a9f2ba6aebb455a78/build/config/android/internal_rules.gni
[modify] https://crrev.com/7bbff9a81d2d9344bcf45b9a9f2ba6aebb455a78/build/config/android/rules.gni

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 18 (4 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2dc440a153bdad284455645c3a5a8dc9d08e6c32

commit 2dc440a153bdad284455645c3a5a8dc9d08e6c32
Author: Peter Wen <wnwen@chromium.org>
Date: Fri Jan 18 22:02:31 2019

Reland "Android: Replace desugar with d8 and r8 for debug"

Previous reland: https://crrev.com/c/1393466

Bug: 906803,  922638 
Change-Id: I8aa0382aaa822ad37b299a9737411130e0c75b63
Reviewed-on: https://chromium-review.googlesource.com/c/1415699
Reviewed-by: Sam Maier <smaier@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624309}
[modify] https://crrev.com/2dc440a153bdad284455645c3a5a8dc9d08e6c32/build/android/gyp/dex.py
[modify] https://crrev.com/2dc440a153bdad284455645c3a5a8dc9d08e6c32/build/android/gyp/proguard.py
[modify] https://crrev.com/2dc440a153bdad284455645c3a5a8dc9d08e6c32/build/config/android/internal_rules.gni

Project Member

Comment 16 by bugdroid1@chromium.org, Today (10 hours ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a4627f23e2ecc8a7eeb1aeab4294404a0b17f144

commit a4627f23e2ecc8a7eeb1aeab4294404a0b17f144
Author: agrieve <agrieve@chromium.org>
Date: Tue Jan 22 20:20:26 2019

Revert "Reland "Android: Replace desugar with d8 and r8 for debug""

This reverts commit 2dc440a153bdad284455645c3a5a8dc9d08e6c32.

Reason for revert: Broke downstream test (see bug)

Original change's description:
> Reland "Android: Replace desugar with d8 and r8 for debug"
> 
> Previous reland: https://crrev.com/c/1393466
> 
> Bug: 906803,  922638 
> Change-Id: I8aa0382aaa822ad37b299a9737411130e0c75b63
> Reviewed-on: https://chromium-review.googlesource.com/c/1415699
> Reviewed-by: Sam Maier <smaier@chromium.org>
> Commit-Queue: Sam Maier <smaier@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#624309}

TBR=wnwen@chromium.org,smaier@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 906803,  922638 , 923862
Change-Id: Ib2003e0e9da279ac586d039e50bb779a0365fb0c
Reviewed-on: https://chromium-review.googlesource.com/c/1427222
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624898}
[modify] https://crrev.com/a4627f23e2ecc8a7eeb1aeab4294404a0b17f144/build/android/gyp/dex.py
[modify] https://crrev.com/a4627f23e2ecc8a7eeb1aeab4294404a0b17f144/build/android/gyp/proguard.py
[modify] https://crrev.com/a4627f23e2ecc8a7eeb1aeab4294404a0b17f144/build/config/android/internal_rules.gni

Sign in to add a comment