Closed Bug 763813 Opened 12 years ago Closed 11 years ago

Starting fennec using am start and non-http URLs attempts to open a top-level window and blows up

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 21

People

(Reporter: nhirata, Unassigned)

References

Details

(Keywords: crash, reproducible, Whiteboard: [native-crash][gfx])

Crash Data

This bug was filed from the Socorro interface and is 
report bp-c81c5ec2-b1ee-4170-b94c-ddb022120611 .
============================================================= 
Frame 	Module 	Signature 	Source
0 	libmozalloc.so 	mozalloc_abort 	memory/mozalloc/mozalloc_abort.cpp:19
1 	libc.so 	__swrite 	
2 	libxul.so 	libxul.so@0x10a9324 	
3 	dalvik-heap (deleted) 	dalvik-heap @0x11f011f 	
4 	dalvik-heap (deleted) 	dalvik-heap @0x11f4e6d

STR: 
1. use adb to do this command : 
adb shell am start -a android.intent.action.VIEW -n org.mozilla.fennec/.App -d fennec-16.0a1.multi.nightly.6_11_2012.apk

Expected: no crash/url not found
Actual: crash

Note: nightly native 6/11/2012; Samsung Galaxy S II
I don't think anyone will actually do this, but it does crash and is reproducible.
Summary: crash in mozalloc_abort → crash in [mozalloc_abort | __swrite]
(In reply to Naoki Hirata :nhirata from comment #0)
> I don't think anyone will actually do this, but it does crash and is
> reproducible.
See bug 763805.
OS: All → Android
Hardware: All → ARM
Summary: crash in [mozalloc_abort | __swrite] → crash with abort message: "ABORT: We need a context on Android: file /builds/slave/m-cen-andrd-ntly/build/gfx/layers/opengl/LayerManagerOGL.cpp, line 174"
Whiteboard: [native-crash][gfx]
Version: Firefox 14 → Trunk
Crash Signature: [@ mozalloc_abort | __swrite | libxul.so@0x10a9324] → [@ mozalloc_abort | __swrite | libxul.so@0x10a9324] [@ mozalloc_abort ]
Component: General → Graphics, Panning and Zooming
Component: Graphics, Panning and Zooming → Graphics
Product: Firefox for Android → Core
Note that since Firefox 16, you likely won't get this message anymore, you'll get bug 771774 instead. Should we dupe?
Maybe. But comment 0 says this is happening on the SGS2, which should be supported, right?
Oh, indeed, it is supported and I've seen Fennec running well on it. Nevermind  comment 2 then.
Crash Signature: [@ mozalloc_abort | __swrite | libxul.so@0x10a9324] [@ mozalloc_abort ] → [@ mozalloc_abort | __swrite | libxul.so@0x10a9324] [@ mozalloc_abort ] [@ TouchBadMemory | mozalloc_abort]
Bug 785163 also has STR to repro this; duped that to here.
I tried to debug this but didn't make much progress because the code in question is some combination of JS and C++ that gdb can't deal with. I used the STR from bug 785163 to repro this (i.e. run the following command twice):

adb shell am start -a android.intent.action.VIEW -n org.mozilla.fennec/.App -d "}"

I also did this using "http://foo.com" instead of "}" as a working baseline to see why the "}" case is different. What I found is that in toolkit/components/nsDefaultCLH.js, when the clh_handle function gets called, cmdLine.preventDefault is true with the foo.com URL and false with the } URL. I can't find the piece of code that's setting the preventDefault flag. When I set a breakpoint with GDB on the SetPreventDefault function it ends up triggering somewhere else which is just plain weird.

If anybody understands the command-line-handling code it would be good if they could take a look at this.
Component: Graphics → General
Product: Core → Firefox for Android
Summary: crash with abort message: "ABORT: We need a context on Android: file /builds/slave/m-cen-andrd-ntly/build/gfx/layers/opengl/LayerManagerOGL.cpp, line 174" → Starting fennec using am start and non-http URLs attempts to open a top-level window and blows up
There's places in the code other than nsDefaultCLH.js that open new XUL windows. For example amWebInstallListener.js does after installing an addon, and the same problem results. We need to do a better job in android of handling multiple XUL windows, probably by baking less assumptions into the code about how many XUL windows are going to be created.
(In reply to Kartikaya Gupta (:kats) from comment #9)
> There's places in the code other than nsDefaultCLH.js that open new XUL
> windows. For example amWebInstallListener.js does after installing an addon,
> and the same problem results. We need to do a better job in android of
> handling multiple XUL windows, probably by baking less assumptions into the
> code about how many XUL windows are going to be created.

amWebInstallListener attempts to avoid that by allowing a custom prompt:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/amWebInstallListener.js#158

We impl that here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/XPIDialogService.js#21

However, bug 789572 (fixed by bug 789721) was causing "confirm" to throw, which caused the amWebInstallListener code to do the default case, using the XUL dialog.

This issue should be fixed in a current nightly.
So then is that the general pattern we want to be following? A search for code that opens new XUL windows [1] turns up a bunch of hits. I'm not sure how many of those calls are actually hit in Fennec and under what conditions, but it seems like we can hit the one in nsDefaultCLH.js using the STR in comment #7. Do we need to go through all of those openWindow calls and make sure that we always have a hook that Fennec can override to display a native UI instead?

[1] http://dxr.mozilla.org/search.cgi?tree=mozilla-central&ext=js&string=openWindow
(In reply to Kartikaya Gupta (:kats) from comment #12)
> So then is that the general pattern we want to be following? A search for
> code that opens new XUL windows [1] turns up a bunch of hits. I'm not sure
> how many of those calls are actually hit in Fennec and under what
> conditions, but it seems like we can hit the one in nsDefaultCLH.js using
> the STR in comment #7. Do we need to go through all of those openWindow
> calls and make sure that we always have a hook that Fennec can override to
> display a native UI instead?

Yes. We did this once for XUL Fennec too. Even in XUL Fennec we did not want to use desktop UI/Dialogs.
Crash Signature: [@ mozalloc_abort | __swrite | libxul.so@0x10a9324] [@ mozalloc_abort ] [@ TouchBadMemory | mozalloc_abort] → [@ mozalloc_abort | __swrite | libxul.so@0x10a9324] [@ mozalloc_abort ] [@ TouchBadMemory | mozalloc_abort] [@ mozalloc_abort(char const*)]
Crash Signature: [@ mozalloc_abort | __swrite | libxul.so@0x10a9324] [@ mozalloc_abort ] [@ TouchBadMemory | mozalloc_abort] [@ mozalloc_abort(char const*)] → [@ mozalloc_abort | __swrite | libxul.so@0x10a9324] [@ mozalloc_abort ] [@ TouchBadMemory | mozalloc_abort] [@ mozalloc_abort(char const*)] [@ mozalloc_abort(char const*) | NS_DebugBreak_P | mozilla::layers::LayerManagerOGL::Initialize(nsRefPtr<mozill…
The patches from bug 826300 and bug 785597 prevent this crash, although it replaces the page with a white screen. The state seems to be recoverable if you close the tab.
Depends on: 785597
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #7)
> I used
> the STR from bug 785163 to repro this (i.e. run the following command twice):
> 
> adb shell am start -a android.intent.action.VIEW -n org.mozilla.fennec/.App
> -d "}"
> 

These STR work to crash fennec in the jan 11 nightly. After bug 785597 landed it is no longer reproducible. It does end up in a bad state (with a blank screen) but if that's a problem we should file another bug for it. (It's not really a user-facing problem though)
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Crash Signature: mozilla::layers::LayerManagerOGL::Initialize(nsRefPtr<mozilla::gl::GLContext>, bool)] → mozilla::layers::LayerManagerOGL::Initialize(nsRefPtr<mozilla::gl::GLContext>, bool)] [@ mozalloc_abort | PKIX_Initialize ]
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.