VR Native UI test crashes due to thumbnail DCHECK |
|||||
Issue descriptionVrShellNativeUiTest#testUrlOnNativeUi has started failing (https://build.chromium.org/p/chromium.fyi/builders/Android%20VR%20Tests/builds/11047) due to hitting a DCHECK in thumbnail_generator.cc (https://cs.chromium.org/chromium/src/chrome/browser/android/download/ui/thumbnail_generator.cc?q=ThumbnailGenerator_&sq=package:chromium&l=149). This was introduced by https://chromium-review.googlesource.com/c/chromium/src/+/577720, although it's not entirely clear whether the issue is the CL introducing something bad or VR doing something it shouldn't. The Java code has an assert to catch this, although it doesn't appear to be hit https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/download/ui/ThumbnailGenerator.java?q=thumbnailgenerator.java&sq=package:chromium&dr&l=52. This should be easily reproducible locally on a Pixel or Pixel XL by running the VR instrumentation tests with either Daydream View or Cardboard paired. Full stack tool dump: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FAndroid_VR_Tests%2F11047%2F%2B%2Frecipes%2Fsteps%2Fstack_tool_with_logcat_dump%2F0%2Fstdout Relevant stack trace: signal 6 (SIGABRT), code -6 in tid 24109 (chromium.chrome) pid: 24109, tid: 24109, name: chromium.chrome >>> org.chromium.chrome <<< signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr -------- [FATAL:ThumbnailGenerator_jni.h(57)] Check failed: native. Destroy Stack Trace: RELADDR FUNCTION FILE:LINE 000afdd3 <unknown> /data/app/org.chromium.chrome-1/lib/arm/libchrome.so 01d8eccf WTF::Vector<std::__ndk1::unique_ptr<blink::JSONValue, std::__ndk1::default_delete<blink::JSONValue> >, 0u, WTF::PartitionAllocator>::ReserveCapacity(unsigned int)+54 /mnt/data/b/c/builder/Android_Builder__dbg_/src/third_party/WebKit/Source/platform/wtf/Vector.h:1607 00e6b2d7 <unknown> /data/app/org.chromium.chrome-1/oat/arm/base.odex ----------------------------------------------------- r0 00000000 r1 00005e2d r2 00000006 r3 00000008 r4 eb70158c r5 00000006 r6 eb701534 r7 0000010c r8 ffa520fc r9 00000042 sl ffa51cac fp ffa52224 ip 00000009 sp ffa51c28 lr e9e78537 pc e9e7ada0 Stack Trace: RELADDR FUNCTION FILE:LINE 00049da0 tgkill+12 /system/lib/libc.so 00047533 pthread_kill+34 /system/lib/libc.so 0001d635 raise+10 /system/lib/libc.so 00019181 __libc_android_abort+34 /system/lib/libc.so 00017048 abort+4 /system/lib/libc.so v------> base::debug::(anonymous namespace)::DebugBreak() /mnt/data/b/c/builder/Android_Builder__dbg_/src/base/debug/debugger_posix.cc:228 0035ef11 base::debug::BreakDebugger()+20 /mnt/data/b/c/builder/Android_Builder__dbg_/src/base/debug/debugger_posix.cc:258 00371005 logging::LogMessage::~LogMessage()+668 /mnt/data/b/c/builder/Android_Builder__dbg_/src/base/logging.cc:784 0204fccd Java_org_chromium_chrome_browser_download_ui_ThumbnailGenerator_nativeDestroy+48 /mnt/data/b/c/builder/Android_Builder__dbg_/src/out/Debug/gen/chrome/browser/jni_headers/chrome/jni/ThumbnailGenerator_jni.h:57 00e6b2d7 offset 0xd55000 /data/app/org.chromium.chrome-1/oat/arm/base.odex
,
Aug 23 2017
CL in review: https://chromium-review.googlesource.com/c/chromium/src/+/628757
,
Aug 23 2017
Issue 758262 has been merged into this issue.
,
Aug 23 2017
a revert is created as more test flakiness is found: https://chromium-review.googlesource.com/c/chromium/src/+/628660
,
Jun 26 2018
This still an issue Brian?
,
Jun 26 2018
,
Jul 4
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by nyquist@chromium.org
, Aug 23 2017So, the crash here is in the generated code, so in out-something/SomeBuildType/gen/chrome/browser/jni_headers/chrome/jni/ThumbnailGenerator_jni.h. This is the code that fails: JNI_GENERATOR_EXPORT void Java_org_chromium_chrome_browser_download_ui_ThumbnailGenerator_nativeDestroy(JNIEnv* env, jobject jcaller, jlong nativeThumbnailGenerator) { ThumbnailGenerator* native = reinterpret_cast<ThumbnailGenerator*>(nativeThumbnailGenerator); CHECK_NATIVE_PTR(env, jcaller, native, "Destroy"); return native->Destroy(env, base::android::JavaParamRef<jobject>(env, jcaller)); } CHECK_NATIVE_PTR is defined as: // Project-specific macros used by the header files generated by // jni_generator.py. Different projects can then specify their own // implementation for this file. #define CHECK_NATIVE_PTR(env, jcaller, native_ptr, method_name, ...) \ DCHECK(native_ptr) << method_name; See https://cs.chromium.org/chromium/src/base/android/jni_generator/jni_generator_helper.h?type=cs&q=check_native_ptr&sq=package:chromium&l=18 The native pointer is lazily initialized, which means that it is only set in when getNativeThumbnailGenerator() is called which is only in retrieveThumbnail(). So it seems like someone is creating a ThumbnailGenerator, but never using it, but still destroying it. That seems wrong though. We should support being able to construct and destroy it without crashing, since the lazy init is an implementation detail. Basically, we do not want this to crash: ### ThumbnailGenerator generator = new ThumbnailGenerator(); generator.destroy(); ### We could avoid this simply by doing an early return in the destroy()-method, ie.: ### public void destroy() { ThreadUtils.assertOnUiThread(); if (mNativeThumbnailGenerator == 0) return; nativeDestroy(mNativeThumbnailGenerator); } ###