+++ This bug was initially created as a clone of Bug #711426 +++ See attached log STR: 1. about:home or any website 2. Rotate to landscape (if under portrait), rotate to portrait and back to landscape (if under landscape) 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 -- Mozilla/5.0 (Android; Linux armv7l; rv:12.0a1) Gecko/20111227 Firefox/12.0a1 Fennec/12.0a1

Range from central http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5efcb9c3b375&tochange=80ac06ad280d I see bug 708307.

Do you get an crash report in about:crashes? If so please place in the crash signature. I am guessing that this is due to the neon_arm_fill.

(In reply to Naoki Hirata :nhirata from comment #2) > Do you get an crash report in about:crashes? If so please place in the > crash signature. I am guessing that this is due to the neon_arm_fill. bp-09a72c3e-fd07-4c2f-be20-2909a2120104 -- and I'm positive it's bug 708307 from that range in comment #1.

Comment on attachment 585778 [details] [diff] [review] Fix typos causing a crash after orientation changes This should fix things up. Embarrassing. The last hunk is just a little cleanup, unrelated to the fix.

Patch posted in a custom build in #mobile works for me so far on the Nexus S and Galaxy SII.

Comment on attachment 585800 [details] [diff] [review] Same patch with more context Review of attachment 585800 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/src/android/AndroidGraphicBuffer.cpp @@ -306,5 @@ > AndroidGraphicBuffer::EnsureBufferCreated(PRUint32 aWidth, PRUint32 aHeight, PRUint32 aUsage, gfxImageFormat aFormat) > { > if (!mHandle) { > mHandle = malloc(GRAPHIC_BUFFER_SIZE); > - sGLFunctions.fGraphicBufferCtor(mHandle, mWidth, mHeight, GetAndroidFormat(aFormat), GetAndroidUsage(aUsage)); shouldn't mWidth == aWidth? When would they be different? ::: widget/src/android/nsWindow.cpp @@ -1176,5 @@ > unsigned char *bits = NULL; > if (sHasDirectTexture) { > - if ((sDirectTexture->Width() != gAndroidBounds.width || > - sDirectTexture->Height() != gAndroidBounds.height) && > - gAndroidBounds.width != 0 && gAndroidBounds.height != 0) { was this causing an issue, or are you just removing it because its unneeded? If its unneeded, why?

(In reply to Brad Lassey [:blassey] from comment #8) > Comment on attachment 585800 [details] [diff] [review] > Same patch with more context > > Review of attachment 585800 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: widget/src/android/AndroidGraphicBuffer.cpp > @@ -306,5 @@ > > AndroidGraphicBuffer::EnsureBufferCreated(PRUint32 aWidth, PRUint32 aHeight, PRUint32 aUsage, gfxImageFormat aFormat) > > { > > if (!mHandle) { > > mHandle = malloc(GRAPHIC_BUFFER_SIZE); > > - sGLFunctions.fGraphicBufferCtor(mHandle, mWidth, mHeight, GetAndroidFormat(aFormat), GetAndroidUsage(aUsage)); > > shouldn't mWidth == aWidth? When would they be different? When we are resizing (reallocating) they are different until we reassign. > > ::: widget/src/android/nsWindow.cpp > @@ -1176,5 @@ > > unsigned char *bits = NULL; > > if (sHasDirectTexture) { > > - if ((sDirectTexture->Width() != gAndroidBounds.width || > > - sDirectTexture->Height() != gAndroidBounds.height) && > > - gAndroidBounds.width != 0 && gAndroidBounds.height != 0) { > > was this causing an issue, or are you just removing it because its unneeded? > If its unneeded, why? It wasn't causing any problems, just removed it because I noticed it was redundant. Someone added the bounds == 0 check above it.

Comment on attachment 585800 [details] [diff] [review] Same patch with more context Review of attachment 585800 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/src/android/AndroidGraphicBuffer.cpp @@ -306,5 @@ > AndroidGraphicBuffer::EnsureBufferCreated(PRUint32 aWidth, PRUint32 aHeight, PRUint32 aUsage, gfxImageFormat aFormat) > { > if (!mHandle) { > mHandle = malloc(GRAPHIC_BUFFER_SIZE); > - sGLFunctions.fGraphicBufferCtor(mHandle, mWidth, mHeight, GetAndroidFormat(aFormat), GetAndroidUsage(aUsage)); It would seem better to me to set mWidth and mHeight when you allocate mHandle, and then use them in the constructor. r=me if you do that. If for some reason you don't think that's correct, please re-request review.

(In reply to Brad Lassey [:blassey] from comment #10) > Comment on attachment 585800 [details] [diff] [review] > Same patch with more context > > Review of attachment 585800 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: widget/src/android/AndroidGraphicBuffer.cpp > @@ -306,5 @@ > > AndroidGraphicBuffer::EnsureBufferCreated(PRUint32 aWidth, PRUint32 aHeight, PRUint32 aUsage, gfxImageFormat aFormat) > > { > > if (!mHandle) { > > mHandle = malloc(GRAPHIC_BUFFER_SIZE); > > - sGLFunctions.fGraphicBufferCtor(mHandle, mWidth, mHeight, GetAndroidFormat(aFormat), GetAndroidUsage(aUsage)); > > It would seem better to me to set mWidth and mHeight when you allocate > mHandle, and then use them in the constructor. > > r=me if you do that. If for some reason you don't think that's correct, > please re-request review. You're right, that's much better. Pushed to inbound. http://hg.mozilla.org/integration/mozilla-inbound/rev/df09a93f0887

Landed on m-c: https://hg.mozilla.org/mozilla-central/rev/df09a93f0887

Comment on attachment 586104 [details] Modified patch that was committed to m-c This is one of the top crashers in Aurora and has little risk. Without this patch, quite a few devices will crash when orientation is changed.

I'll leave this in the queue to let it bake on m-c for a day and come back to approve tomorrow.

Comment on attachment 586104 [details] Modified patch that was committed to m-c [Triage Comment] Mobile only and top crash - approved for Aurora.

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) No crashes seen on rotation.

Actually the gralloc stuff was (unknown to me) backed out in Aurora on 12/17, so we don't need this fix. I guess if we want to reland the gralloc bits on aurora we'd need this, but I don't know if we want that just yet.

(In reply to Camelia Urian from comment #18) > 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) > > No crashes seen on rotation. not sure how this was verified on aurora when it hadn't landed there yet https://hg.mozilla.org/releases/mozilla-aurora/rev/f7f3c25137b1