Closed Bug 711426 Opened 8 years ago Closed 8 years ago

Crash on rotation after adjusting viewport

Categories

(Firefox for Android :: General, defect, P2, critical)

ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 11
Tracking Status
firefox11 --- verified
firefox12 --- verified
fennec 11+ ---

People

(Reporter: aaronmt, Assigned: cwiiis)

References

()

Details

(4 keywords, Whiteboard: [native-crash], [QA^])

Crash Data

Attachments

(2 files, 3 obsolete files)

GeckoFavicons( 1123): Loaded favicon from DB successfully for URL = http://www.neowin.net/
D/GeckoFavicons( 1123): LoadFaviconTask finished for URL = http://www.neowin.net/ (2)
I/GeckoApp( 1123): Favicon successfully loaded for URL = http://www.neowin.net/
I/GeckoApp( 1123): Favicon is for current URL = http://www.neowin.net/
I/GeckoTab( 1123): Updated favicon for tab with id: 1
I/GeckoSoftwareLayerClient( 1123): Adjusting viewport
I/Gecko   ( 1123): AndroidGeckoEvent: 0x405f56a8 : 19
I/Gecko   ( 1123): WARNING: NS_ENSURE_TRUE(mState == STATE_TRANSFERRING) failed: file /builds/slave/m-cen-andrd-dbg/build/netwerk/base/src/nsSocketTransport2.cpp, line 1911
I/Gecko   ( 1123): WARNING: NS_ENSURE_TRUE(mState == STATE_TRANSFERRING) failed: file /builds/slave/m-cen-andrd-dbg/build/netwerk/base/src/nsSocketTransport2.cpp, line 1911
I/Gecko   ( 1123): AndroidBridge::GetDPI
I/Gecko   ( 1123): void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_internal&, nsAString_internal&)
I/GeckoApp( 1123): Got message: Content:StateChange
I/GeckoApp( 1123): State - 196612
I/Gecko   ( 1123): leaving void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_internal&, nsAString_internal&)
I/WindowManager(  110): Setting rotation to 1, animFlags=0
I/ActivityManager(  110): Config changed: { scale=1.0 imsi=302/220 loc=en_US touch=3 keys=1/1/2 nav=1/1 orien=2 layout=34 uiMode=17 seq=24}
I/GeckoApp( 1123): configuration changed
I/Gecko   ( 1123): void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_internal&, nsAString_internal&)
I/GeckoApp( 1123): Got message: Content:StateChange
I/GeckoApp( 1123): State - 131088
I/GeckoApp( 1123): Got a document stop
I/Gecko   ( 1123): leaving void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_internal&, nsAString_internal&)
I/Gecko   ( 1123): void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_internal&, nsAString_internal&)
I/GeckoSoftwareLayerClient( 1123): Screen-size changed to (800,480)
I/GeckoApp( 1123): Got message: Content:StateChange
I/Gecko   ( 1123): AndroidGeckoEvent: 0x40644cb8 : 8
I/GeckoSoftwareLayerClient( 1123): Adjusting viewport
I/GeckoApp( 1123): State - 131088
I/GeckoApp( 1123): Got a document stop
I/Gecko   ( 1123): leaving void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_internal&, nsAString_internal&)
I/Gecko   ( 1123): AndroidGeckoEvent: 0x40568280 : 19
I/Gecko   ( 1123): void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_internal&, nsAString_internal&)
I/Gecko   ( 1123): leaving void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_internal&, nsAString_internal&)
I/Gecko   ( 1123): nsWindow[0x46d36500]::Resize [0 0 2048 2048] (repaint 1)
I/Gecko   ( 1123): nsWindow: 0x46d36500 OnSizeChanged [2048 2048]
I/Gecko   ( 1123): nsWindow[0x46d36c80]::Resize [0 0 2048 2048] (repaint 0)
I/Gecko   ( 1123): nsWindow: 0x46d36c80 OnSizeChanged [2048 2048]
I/GeckoSoftwareLayerClient( 1123): Adjusting viewport
I/Gecko   ( 1123): AndroidGeckoEvent: 0x40643b48 : 19
I/WindowManager(  110): Setting rotation to 0, animFlags=0
I/ActivityManager(  110): Config changed: { scale=1.0 imsi=302/220 loc=en_US touch=3 keys=1/1/2 nav=1/1 orien=1 layout=34 uiMode=17 seq=25}
I/GeckoApp( 1123): configuration changed
I/GeckoSoftwareLayerClient( 1123): Screen-size changed to (480,800)
I/Gecko   ( 1123): AndroidGeckoEvent: 0x40607178 : 8
I/GeckoSoftwareLayerClient( 1123): Adjusting viewport
I/Gecko   ( 1123): AndroidGeckoEvent: 0x405b2b68 : 19
I/GeckoSoftwareLayerClient( 1123): Adjusting viewport
I/Gecko   ( 1123): AndroidGeckoEvent: 0x405ac868 : 19
I/WindowManager(  110): Setting rotation to 1, animFlags=0
I/ActivityManager(  110): Config changed: { scale=1.0 imsi=302/220 loc=en_US touch=3 keys=1/1/2 nav=1/1 orien=2 layout=34 uiMode=17 seq=26}
I/GeckoApp( 1123): configuration changed
I/GeckoSoftwareLayerClient( 1123): Screen-size changed to (800,480)
I/Gecko   ( 1123): AndroidGeckoEvent: 0x4054e650 : 8
I/GeckoSoftwareLayerClient( 1123): Adjusting viewport
I/ActivityManager(  110): Process org.mozilla.fennec (pid 1123) has died.
I/WindowManager(  110): WIN DEATH: Window{406f5648 SurfaceView paused=false}
I/WindowManager(  110): WIN DEATH: Window{406d09e0 org.mozilla.fennec/org.mozilla.fennec.App paused=false}

STR:

1. www.neowin.net
2. Rotate to landscape, rotate to portrait, rotate to landscape

--
Samsung Nexus S (Android 2.3.6)
20111216052315
http://hg.mozilla.org/mozilla-central/rev/80ac06ad280d
Blocks: 708307
Whiteboard: [native-crash] → [native-crash], [QA^]
I'm trying to reproduce this, and can't on my device (HTC Flyer), but this could depend on resolution. Are these absolutely concrete reproduction instructions?

My stab-in-the-dark would be that possibly it's an OOM and we should force freeing textures before allocating new ones (currently, it flushes on draw), but that doesn't seem hugely likely...

Can we get a stack-trace?
(In reply to Chris Lord [:cwiiis] from comment #2)
> I'm trying to reproduce this, and can't on my device (HTC Flyer), but this
> could depend on resolution. Are these absolutely concrete reproduction
> instructions?
> 
> My stab-in-the-dark would be that possibly it's an OOM and we should force
> freeing textures before allocating new ones (currently, it flushes on draw),
> but that doesn't seem hugely likely...
> 
> Can we get a stack-trace?

Not sure what I'm doing with the prebuilt binaries at https://wiki.mozilla.org/Mobile/Fennec/Android/GDB what to do with them. Would be easier for QA to get builds that dump Gecko stack traces from threads.
Also reproducible easier on timecube.com  --- http://www.youtube.com/watch?v=75eJMsVDTn4
Duplicate of this bug: 711735
I was able to reproduce this using the initial steps on the latest fennec native:
Mozilla/5.0(Android;Linux armv7l;rv:11.0a1)Gecko/20111218;Firefox/11.0a1 Fennec/11.0a1.
Device: HTC Desire (Android 2.2)

Crash report: https://crash-stats.mozilla.com/report/index/bp-54dfc905-9c8b-4b04-b4be-9298c2111219
Crash Signature: @ libGLESv2_adreno200.so@0x82070
Assignee: nobody → bugmail.mozilla
Priority: -- → P2
The two crash reports linked here seem to have different signatures. The second one (bp-9dec3a30-6151-4069-9ebb-c48dd2111219) is the same as bug 712655, and will be tracked there. I'll leave this bug for the adreno signature.
A lot of info tossed around in channel today.

I can reproduce on my Samsung Galaxy SII (Android 2.3.4), Samsung Nexus S (4.0.3), and Samsung Nexus One (2.3.6).

I can't reproduce on my Motorola Droid Pro (Android 2.3.4).

On both the SII and Nexus S, rotating from landscape to portrait (on about:home and any other page) will yield the crash.

On both the SII and Nexus S, restoring from session restore in landscape, and rotating to portrait will not yield the crash. But, rotating back to landscape will yield the crash.

On both the SII/Nexus S, I was getting plenty of the following entries (not sure if that's related).

/AndroidGraphicBuffer(10410): GL error [glEGLImageTargetTexture2DOES]:                                      501
I think this might be the issue I'm having too-
Its not consistently reproducible, unless an animated GIF is playing.
It might be easier to test with, e.g., http://fukung.net/v/31665/2dc5bb8a8e8aba3724eec34fe3e48083.gif
Immediate crash on rotate, 100% of the time on my device (Droid X, CyanogenMod firmware).
(In reply to CoJaBo from comment #11)
> I think this might be the issue I'm having too-
> Its not consistently reproducible, unless an animated GIF is playing.
> It might be easier to test with, e.g.,
> http://fukung.net/v/31665/2dc5bb8a8e8aba3724eec34fe3e48083.gif
> Immediate crash on rotate, 100% of the time on my device (Droid X,
> CyanogenMod firmware).

Oh, I have same crashings in that page. Desire HD, Gingerbread 2.3.3 (HTC Official).
This fixes destroying the buffer while it's being used and fixes the temporarily increased memory requirement when changing buffers. I've also added a check that the buffer exists in the drawing function, for robustness.
Assignee: bugmail.mozilla → chrislord.net
Status: NEW → ASSIGNED
Attachment #584029 - Flags: review?(bugmail.mozilla)
Argh, damn queues, seem to have missed half of the patch - reuploading in a second.
Attachment #584029 - Attachment is obsolete: true
Attachment #584029 - Flags: review?(bugmail.mozilla)
Attachment #584043 - Flags: review?(bugmail.mozilla)
Comment on attachment 584043 [details] [diff] [review]
Fix one very likely and one unlikely cause of rotation crash

>The lock on the software buffer was not being respected when destroying the
>surface on screen rotation, meaning we could destroy it while Gecko was still
>drawing to it. This would certainly cause a crash on rotation under the right
>conditions.

Nice catch! :) This could definitely explain all the crashes we've been seeing.

>diff --git a/mobile/android/base/gfx/GeckoSoftwareLayerClient.java b/mobile/android/base/gfx/GeckoSoftwareLayerClient.java
>             Log.i(LOGTAG, "Screen-size changed to " + mScreenSize);
>             GeckoEvent event = new GeckoEvent(GeckoEvent.SIZE_CHANGED,
>-                                              mBufferSize.width, mBufferSize.height,
>+                                              mNewBufferSize.width, mNewBufferSize.height,
>                                               metrics.widthPixels, metrics.heightPixels);
>             GeckoAppShell.sendEventToGecko(event);

As I mentioned on IRC, we might want to defer sending this event so that Java and Gecko don't get out of sync with respect to the buffer size. Your idea of passing in the expected buffer size in beginDrawing() probably works best.

Rest looks good.
Attachment #584043 - Flags: review?(bugmail.mozilla) → review+
As mentioned, in the previous patch there's still a possible race where we send the SIZE_CHANGED event, but we deal with a draw event before the SIZE_CHANGED is processed - in this situation, Gecko will have an incorrectly sized buffer, which may cause a crash.

This version of the patch fixes (hopefully) that case by handing over control of the buffer size to Gecko via width/height parameters in beginDrawing.
Attachment #584043 - Attachment is obsolete: true
Attachment #584051 - Flags: review?(bugmail.mozilla)
Comment on attachment 584051 [details] [diff] [review]
Fix two very likely and one unlikely cause of rotation crash

>diff --git a/mobile/android/base/gfx/GeckoSoftwareLayerClient.java b/mobile/android/base/gfx/GeckoSoftwareLayerClient.java
>-            // * 2 because it's a 16-bit buffer (so 2 bytes per pixel).
>-            mBuffer = GeckoAppShell.allocateDirectBuffer(mBufferSize.getArea() * 2);
>+            // We store this in mNewBufferSize so that mBufferSize always
>+            // represents the size of the current buffer. The buffer is refreshed
>+            // on the next draw.

This comment needs updating, rest looks good.
Attachment #584051 - Flags: review?(bugmail.mozilla) → review+
This rectifies that comment (by removing it, I don't think the situation warrants commenting on), and also adds one extra fix in GeckoSoftwareLayerClient.getBitmap(), which was also not respecting the data lock.
Attachment #584051 - Attachment is obsolete: true
Attachment #584076 - Flags: review?(bugmail.mozilla)
Comment on attachment 584076 [details] [diff] [review]
Fix some likely and unlikely rotation crash causes

Good catch :)
Attachment #584076 - Flags: review?(bugmail.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/2f6873cba874
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
The above checked-in patch does not fix the crash I reported. Suggest keeping this open until the issue I filed is resolved.

Still reproducible on Samsung Galaxy SII (Android 2.3.4), and Samsung Nexus S (Android 4.0.3) and Samsung Nexus One (Android 2.3.4)

--
20111225031006
http://hg.mozilla.org/mozilla-central/rev/798b00a6fe29
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 584076 [details] [diff] [review]
Fix some likely and unlikely rotation crash causes

We want these fixes on aurora
Attachment #584076 - Flags: approval-mozilla-aurora?
Seeing this crash with Nightly on Nexus S with Android 4.0.3 and crash signature libxul.so@0xae938c
https://crash-stats.mozilla.com/report/index/bbbdb6c6-03c0-4f92-a5af-8044b2111226
Crash Signature: @ libGLESv2_adreno200.so@0x82070 → @ libGLESv2_adreno200.so@0x82070 @ libxul.so@0xae938c
Comment on attachment 584076 [details] [diff] [review]
Fix some likely and unlikely rotation crash causes

[Triage Comment]
Approved for Aurora.
Attachment #584076 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/25ca24acbf49

With the complexity of tracking in m-c and m-a, I think we should file a new bug instead of leaving this one open.
Target Milestone: Firefox 12 → Firefox 11
(In reply to Mark Finkle (:mfinkle) from comment #26)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/25ca24acbf49
> 
> With the complexity of tracking in m-c and m-a, I think we should file a new
> bug instead of leaving this one open.

To be clear, the patches on this bug have landed on m-c and m-a. If there is any additional work, a new bug should be opened because I am closing this one.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Could be that bug 713299 covers the rest of the needed fixes
Crash Signature: @ libGLESv2_adreno200.so@0x82070 @ libxul.so@0xae938c → @ libGLESv2_adreno200.so@0x82070
Blocks: 713774
tracking-fennec: --- → 11+
Verified with:
Aurora 11.0a2 (2012-01-10) Samsung Nexus S (Android 2.3.6)
Nightly 12.0a1 (2012-01-10) Samsung Nexus S (Android 2.3.6)

Verified on site from bug, and others:
- www.neowin.net
- timecube.com
- http://fukung.net/v/31665/2dc5bb8a8e8aba3724eec34fe3e48083.gif
Status: RESOLVED → VERIFIED
(In reply to Brad Lassey [:blassey] from comment #30)
> this chunk of the patch that landed on mozilla-central: 
> https://hg.mozilla.org/mozilla-central/diff/2f6873cba874/mobile/android/base/
> gfx/GeckoSoftwareLayerClient.java#l1.41
> 
> didn't make it to mozilla-aurora:
> https://hg.mozilla.org/releases/mozilla-aurora/diff/25ca24acbf49/mobile/
> android/base/gfx/GeckoSoftwareLayerClient.java

I don't think we saw any crashes that that fixed, but it's theoretically possible (if getBitmap is called immediately after the keyboard pops up/down or the screen rotates) - we probably want to land it too...

What do I need to do to get this in? And is there a reason it wasn't landed in the first place?
(In reply to Chris Lord [:cwiiis] from comment #31)
> What do I need to do to get this in? And is there a reason it wasn't landed
> in the first place?

I landed the missing chunk:
https://hg.mozilla.org/releases/mozilla-aurora/rev/28f81ac0a1fa
You need to log in before you can comment on or make changes to this bug.