2.45 KB, patch
|Details | Diff | Splinter Review|
This bug was filed from the Socorro interface and is report bp-357b89c0-fc0a-49ed-836c-9a7f42141104. ============================================================= java.lang.NullPointerException at org.mozilla.gecko.gfx.GeckoLayerClient.activateProgram(GeckoLayerClient.java:745) at dalvik.system.NativeStart.run(Native Method) at dalvik.system.NativeStart.run(Native Method) at dalvik.system.NativeStart.run(Native Method)
Sotaro, anything you can do to help?
(In reply to Milan Sreckovic [:milan] from comment #1) > Sotaro, anything you can do to help? It seems that there is a case that mLayerRenderer is null. https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/gfx/GeckoLayerClient.java#735
I take a look a bit more.
From the code, when nsWindow::DrawWindowOverlay() should have mLayerRendererFrame and nsWindow should not be destroyed. https://dxr.mozilla.org/mozilla-central/source/widget/android/nsWindow.cpp#3210 And before calling the DrawWindowOverlay(), nsWindow::DrawWindowUnderlay() should be called by Compositor. When the DrawWindowUnderlay() was called, GeckoLayerClient.mLayerRenderer seemed valid, because mLayerRendererFrame should be valid in the DrawWindowOverlay(). It seemed to mean that GeckoLayerClient.mLayerRenderer becomes invalid between DrawWindowOverlay() call and DrawWindowOverlay().
GeckoLayerClient dies not release GeckoLayerClient.mLayerRenderer during its life time since it get in GeckoLayerClient.onGeckoReady() https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/gfx/GeckoLayerClient.java#158 It seemed that GeckoLayerClient was also released when the the crash happened.
GeckoLayerClient and LayerRenderer are destroyed in LayerView.destroy(). https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/gfx/LayerView.java#194
There is a code that to trigger to pause compositor. It is triggered from surfaceDestroyed(). SurfaceListener.surfaceDestroyed() ->LayerView.onDestroyed() ->GLController.serverSurfaceDestroyed() ->GLController.pauseCompositor() ->nsWindow::GLControllerSupport::PauseCompositor() ->CompositorChild::SendPause() // sync ipc ->CompositorParent::RecvPause()
FYI: diagram around GeckoView https://github.com/sotaroikeda/firefox-diagrams/blob/master/mobile/mobile_GeckoView_47.pdf
LayerView.destroy() could be called from GeckoView.onDetachedFromWindow() https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/GeckoView.java#325
All the crash reports I looked at with this signature seem to have some memory pressure stuff on the stack, so it might be that low-memory issues are causing the LayerView to get destroyed in the middle of a composite. The native code holds a reference to the GeckoLayerClient and tries to call drawWindowUnderlay but the layer renderer has already been destroyed. Also I'm not sure if the transition to GeckoView and Jim Chen's patches will affect this - a lot of the code was modified recently.
Yea, I agree. It seems to related to low-memory situation.
SurfaceListener.surfaceDestroyed() is normally called when SurfaceView is hidden. onDetachedFromWindow() is called when Activity is going to be destroyed. But in normal use case, onDetachedFromWindow() seems not called.
(In reply to Sotaro Ikeda [:sotaro] from comment #12) > But in normal use case, onDetachedFromWindow() seems not called. I run fennec on my phone and used it several ways, but I did not see the onDetachedFromWindow().
(In reply to Sotaro Ikeda [:sotaro] from comment #12) > SurfaceListener.surfaceDestroyed() is normally called when SurfaceView is > hidden. onDetachedFromWindow() is called when Activity is going to be > destroyed. But in normal use case, onDetachedFromWindow() seems not called. Other possibility is that BrowserApp(Activity) is destroyed without surfaceDestroyed().
Created attachment 8717360 [details] [diff] [review] patch - Add manual LayerView.onDestroyed() callings
I could not reproduce the crash. But attachment 8717360 [details] [diff] [review] seems to address the problem.
Are there any recent crash reports? The latest reports I've seen are from 44, which is before the major LayerView/GLController refactoring that landed in 46. Without STR, we don't know if this still happens or not, and I'd rather wait before we make speculative changes.
Comment on attachment 8717360 [details] [diff] [review] patch - Add manual LayerView.onDestroyed() callings Ok, I cancel the review request and wait if there could be a new crash.
No recent crashes.